Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: CSSOM generation for shadowRoot in Safari #1113

Merged
merged 14 commits into from
Oct 9, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 35 additions & 7 deletions lib/core/utils/preload-cssom.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ function loadCssom({ root, shadowId }, timeout, convertTextToStylesheetFn) {
/**
* Make an axios get request to fetch a given resource and resolve
* @method getExternalStylesheet
* @private
* @param {Object} param an object with properties to configure the external XHR
* @property {Object} param.resolve resolve callback on queue
* @property {Object} param.reject reject callback on queue
* @property {String} param.url string representing the url of the resource to load
* @property {Number} param.timeout timeout to about network call
* @private
*/
function getExternalStylesheet({ resolve, reject, url }) {
axe.imports
Expand All @@ -38,11 +38,38 @@ function loadCssom({ root, shadowId }, timeout, convertTextToStylesheetFn) {

const q = axe.utils.queue();

// handle .styleSheets non existent on certain shadowDOM root
const rootStyleSheets = root.styleSheets
? Array.from(root.styleSheets)
: null;
if (!rootStyleSheets) {
let styleSheets = null;

if (!root.styleSheets) {
/**
* In Safari: (Issue: https://github.com/dequelabs/axe-core/issues/1082)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please report this as a bug to Apple.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reported, but do not have an issue/ bug reported link from them yet.

* As per the issue, shadowRoot does not return their styleShets,
* So the content of any `style` tag is parsed to construct shadowDOM cssom.
*
* Steps:
* - root(document).styleSheets does not return on `shadowContent/ shadowRoot`.
* - Hence lookup all `<style>` tag(s) with in the shadowRoot and convert to styleSheet dynamically
* - Pass these converted styleSheets to parse and construct cssom of shadowRoot(s)
*/
styleSheets = Array.from(root.children)
.filter(node => {
return node.nodeName.toUpperCase() === 'STYLE';
})
.map(node => {
const sheet = convertTextToStylesheetFn({
data: node.textContent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this solution work with link elements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works, just fine as shown here: https://github.com/dequelabs/axe-core/pull/1113/files#diff-b195c7e1a05c62e60347f579d8f7dddeR339

It was the style tag that needed special parsing as above.

// no need to pass shadowId, root and isExternal
// as only interested in converting given data to sheet
});
return sheet.sheet;
});
} else {
// retrieve stylesheets as an []
styleSheets = Array.from(root.styleSheets);
}

// if no root styleSheets then return
if (!styleSheets || !styleSheets.length) {
return q;
}

Expand All @@ -51,7 +78,7 @@ function loadCssom({ root, shadowId }, timeout, convertTextToStylesheetFn) {
let sheetHrefs = [];

// filter out sheets, that should not be accounted for...
const sheets = rootStyleSheets.filter(sheet => {
const sheets = styleSheets.filter(sheet => {
// FILTER > sheets with the same href (if exists)
let sheetAlreadyExists = false;
if (sheet.href) {
Expand Down Expand Up @@ -135,6 +162,7 @@ function loadCssom({ root, shadowId }, timeout, convertTextToStylesheetFn) {
});
}
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This empty array is passed into sheets.forEach((sheet, sheetIndex) => {...}, []).


// return
return q;
}
Expand Down
34 changes: 24 additions & 10 deletions test/integration/full/css-orientation-lock/violations.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ describe('css-orientation-lock violations test', function() {
}
});

function assertViolatedSelectors(relatedNodes, violatedSelectors) {
relatedNodes.forEach(function(node) {
var target = node.target[0];
var className = Array.isArray(target) ? target.reverse()[0] : target;
assert.isTrue(violatedSelectors.indexOf(className) !== -1);
});
}

it('returns VIOLATIONS if preload is set to TRUE', function(done) {
// the sheets included in the html, have styles for transform and rotate, hence the violation
axe.run(
Expand All @@ -57,18 +65,17 @@ describe('css-orientation-lock violations test', function() {
assert.property(res, 'violations');
assert.lengthOf(res.violations, 1);

// assert the node and related nodes
// assert the node
var checkedNode = res.violations[0].nodes[0];
assert.isTrue(/html/i.test(checkedNode.html));

// assert the relatedNodes
var checkResult = checkedNode.all[0];
assert.lengthOf(checkResult.relatedNodes, 2);
var violatedSelectors = ['.someDiv', '.thatDiv'];
checkResult.relatedNodes.forEach(function(node) {
var target = node.target[0];
var className = Array.isArray(target) ? target.reverse()[0] : target;
assert.isTrue(violatedSelectors.indexOf(className) !== -1);
});
assertViolatedSelectors(checkResult.relatedNodes, [
'.someDiv',
'.thatDiv'
]);

done();
}
Expand Down Expand Up @@ -101,12 +108,19 @@ describe('css-orientation-lock violations test', function() {
assert.property(res, 'violations');
assert.lengthOf(res.violations, 1);

// assert the node and related nodes
// assert the node
var checkedNode = res.violations[0].nodes[0];
assert.isTrue(/html/i.test(checkedNode.html));

// assert the relatedNodes
var checkResult = checkedNode.all[0];
assert.lengthOf(checkResult.relatedNodes, 3);
assertViolatedSelectors(checkResult.relatedNodes, [
'.someDiv',
'.thatDiv',
'.shadowDiv'
]);

// Issue - https://github.com/dequelabs/axe-core/issues/1082
assert.isAtLeast(checkResult.relatedNodes.length, 2);
done();
}
);
Expand Down
1 change: 1 addition & 0 deletions test/integration/full/preload-cssom/preload-cssom.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<iframe id="frame1" src="frames/level1.html"></iframe>
<div id="mocha"></div>
<div id="shadow-fixture"></div>
<div id="shadow-fixture2"></div>
<div class="blue">blue on parent page</div>
<script src="/test/testutils.js"></script>
<script src="preload-cssom.js"></script>
Expand Down
68 changes: 54 additions & 14 deletions test/integration/full/preload-cssom/preload-cssom.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ describe('preload cssom integration test', function() {
.then(function(results) {
var sheets = results[0];
// verify count
assert.isAtLeast(sheets.length, 4);
assert.lengthOf(sheets, 7);
// verify that the last non external sheet with shadowId has green selector
var nonExternalsheetsWithShadowId = sheets
.filter(function(s) {
Expand All @@ -246,20 +246,60 @@ describe('preload cssom integration test', function() {
.filter(function(s) {
return s.shadowId;
});
assertStylesheet(
nonExternalsheetsWithShadowId[
nonExternalsheetsWithShadowId.length - 1
].sheet,
'.green',
'.green{background-color:green;}'
);

// Issue - https://github.com/dequelabs/axe-core/issues/1082
if (
nonExternalsheetsWithShadowId &&
nonExternalsheetsWithShadowId.length
) {
assertStylesheet(
nonExternalsheetsWithShadowId[
nonExternalsheetsWithShadowId.length - 1
].sheet,
'.green',
'.green{background-color:green;}'
);
}
done();
})
.catch(done);
}
);

(shadowSupported ? it : xit)(
'should return styles from shadow dom',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to test link element stylesheets as well.

function(done) {
var fixture = document.getElementById('shadow-fixture2');
var shadow = fixture.attachShadow({ mode: 'open' });
shadow.innerHTML =
'<style>@import "https://cdnjs.cloudflare.com/ajax/libs/skeleton/2.0.4/skeleton.css"; @import "preload-cssom-shadow-blue.css"; .green { background-color: green; }</style>' +
'<div class="initialism">Some text</div>' +
'<style>.notGreen { background-color: orange; }</style>' +
'<div class="green">green</div>' +
'<div class="red">red</div>' +
'<div class="duplicateGreen">red</div>' +
'<h1>Heading</h1>';
getPreload(fixture)
.then(function(results) {
var sheets = results[0];
// verify count
assert.lengthOf(sheets, 8);
// verify that the last non external sheet with shadowId has green selector
var nonExternalsheetsWithShadowId = sheets
.filter(function(s) {
return !s.isExternal;
})
.filter(function(s) {
return s.shadowId;
});
assertStylesheet(
nonExternalsheetsWithShadowId[
nonExternalsheetsWithShadowId.length - 2
].sheet,
'.green',
'.green{background-color:green;}'
);
assertStylesheet(
nonExternalsheetsWithShadowId[
nonExternalsheetsWithShadowId.length - 1
].sheet,
'.notGreen',
'.notGreen{background-color:orange;}'
);
done();
})
.catch(done);
Expand Down