From 66651c98ff7f41fdaf886a643ef31a9b8740904a Mon Sep 17 00:00:00 2001 From: jkodu Date: Tue, 29 Jan 2019 19:20:34 +0000 Subject: [PATCH 01/28] refactor: update preload cssom to handle nested import style resolution --- lib/core/utils/preload-cssom.js | 429 +++++++++++++++++++------------- 1 file changed, 262 insertions(+), 167 deletions(-) diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index fb62f23817..5b9e372d78 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -1,191 +1,272 @@ /** - * Make an axios get request to fetch a given resource and resolve - * @method getExternalStylesheet - * @param {Object} options an object with properties to configure the external XHR - * @property {Object} options.resolve resolve callback on queue - * @property {Object} options.reject reject callback on queue - * @property {String} options.url string representing the url of the resource to load - * @property {Object} options.rootNode document or shadowDOM root document for which to process CSSOM - * @property {Number} options.timeout timeout to about network call - * @property {Function} options.getStyleSheet a utility function to generate a style sheet for a given text content - * @property {String} options.shadowId an id if undefined denotes that given root is a shadowRoot - * @property {Number} options.priority css applied priority - * @returns resolve with stylesheet object + * Filter to remove any duplicate `stylesheets`, that share the same `href` + * + * @method filterStylesheetsWithSameHref + * @private + * @param {Array} sheets stylesheets + * @returns {Array} + */ +function filterStylesheetsWithSameHref(sheets) { + let hrefs = []; + return sheets.filter(sheet => { + let exists = false; + if (sheet.href) { + if (!hrefs.includes(sheet.href)) { + hrefs.push(sheet.href); + } else { + exists = true; + } + } + return !exists; + }); +} + +/** + * Filter `media=print` + * + * @method filterMediaIsPrint + * @private + * @param {String} media media value eg: 'print' + * @returns {Boolean} + */ +function filterMediaIsPrint(media) { + if (media) { + return !media.toUpperCase().includes('PRINT'); + } + return true; +} + +/** + * Get stylesheet(s) for root + * + * @method getStylesheetsOfRootNode + * @private + * @param {Object} options configuration options + * @property {Object} options.rootNode document or document fragment + * @property {Function} options.convertTextToStylesheet a utility function to generate a style sheet from given data (text) + * @returns an array of stylesheet objects + */ +function getStylesheetsOfRootNode({ + rootNode, + convertTextToStylesheet, + shadowId +}) { + const sheets = + rootNode.nodeType === 11 && shadowId // nodeType === 11 -> DOCUMENT_FRAGMENT + ? Array.from(rootNode.children) + .filter(node => { + /** + * limit to only `style` or `link` attributes with `rel=stylesheet` and `media != print` + */ + const nodeName = node.nodeName.toUpperCase(); + const linkHref = node.getAttribute('href'); + const linkRel = node.getAttribute('rel'); + const isLink = + nodeName === 'LINK' && + linkHref && + linkRel && + node.rel.toUpperCase().includes('STYLESHEET'); + const isStyle = nodeName === 'STYLE'; + return isStyle || (isLink && filterMediaIsPrint(node.media)); + }) + .reduce((out, node) => { + const nodeName = node.nodeName.toUpperCase(); + const data = nodeName === 'STYLE' ? node.textContent : node; + const isLink = nodeName === 'LINK'; + const stylesheet = convertTextToStylesheet({ + data, + isLink, + root: rootNode + }); + out.push(stylesheet.sheet); + return out; + }, []) + : Array.from(rootNode.styleSheets).filter(sheet => + filterMediaIsPrint(sheet.media.mediaText) + ); + + return filterStylesheetsWithSameHref(sheets); +} + +/** + * Make an `axios` get request to fetch a given `resource` + * + * @method getStylesheetFromUrl * @private + * @param {Object} options an object with properties to configure the external request + * @property {String} options.url url of the resource to load + * @property {Number} options.timeout timeout + * @property {Function} options.convertTextToStylesheet a utility function to generate a style sheet from given data (text) + * @property {String} options.shadowId an `id` if `undefined` denotes that given root is a `shadowRoot` + * @property {Number} options.priority css applied priority + * @property {Object} options.rootNode document or document fragment + * @property {Object} options.queue `axe.utils.queue` to defer resolving stylesheets + * @property {Object} options.resolve resolve of the `axe.utils.queue` from which this method was involved + * @property {Object} options.reject reject of the `axe.utils.queue` from which this method was involved + * @returns resolve with stylesheet */ -function getExternalStylesheet(options) { +function getStylesheetFromUrl(options) { const { - resolve, - reject, url, - rootNode, timeout, - getStyleSheet, + convertTextToStylesheet, shadowId, - priority + priority, + rootNode, + queue, + resolve: resolvePreviousQueue, + reject: rejectPreviousQueue, + analyzeNested = false } = options; axe.imports - .axios({ - method: 'get', - url, - timeout - }) + .axios({ method: 'get', url, timeout }) .then(({ data }) => { - const sheet = getStyleSheet({ + const isExternal = true; + const stylesheet = convertTextToStylesheet({ data, - isExternal: true, shadowId, - root: rootNode, - priority + priority, + isExternal, + root: rootNode + }); + if (!analyzeNested) { + resolvePreviousQueue(stylesheet); + } + /** + * recursively check`cssRules` for any further `@import` rules + */ + parseRules({ + ...options, + queue, + sheet: stylesheet.sheet, + priority, + rootNode, + isExternal }); - resolve(sheet); + resolvePreviousQueue(undefined); }) - .catch(reject); + .catch(rejectPreviousQueue); } -/** - * Get stylesheet(s) from shadowDOM - * @param {Object} documentFragment document fragment node - * @param {Function} getStyleSheet helper function to get stylesheet object - * @returns an array of stylesheet objects - */ -function getSheetsFromShadowDom(documentFragment, getStyleSheet) { - return Array.from(documentFragment.children).reduce((out, node) => { - const nodeName = node.nodeName.toUpperCase(); - if (nodeName !== 'STYLE' && nodeName !== 'LINK') { - return out; - } - if (nodeName === 'STYLE') { - const dynamicSheet = getStyleSheet({ data: node.textContent }); - out.push(dynamicSheet.sheet); - } - if (nodeName === 'LINK' && !node.media.includes('print')) { - const dynamicSheet = getStyleSheet({ data: node, isLink: true }); - out.push(dynamicSheet.sheet); - } - return out; - }, []); -} +function parseRules(options) { + const { + shadowId, + rootNode, + priority, + isExternal = false, + sheet, + queue, + convertTextToStylesheet + } = options; + /** + * `sheet.cssRules` throws an error on `cross-origin` stylesheets + */ + const cssRules = sheet.cssRules; -/** - * Filter a given array of stylesheet objects - * @param {Array} styleSheets array of stylesheets - * @returns an filtered array of stylesheets - */ -function filterStyleSheets(styleSheets) { - let sheetHrefs = []; + const rules = Array.from(cssRules); + if (!rules || !rules.length) { + return; + } - return styleSheets.filter(sheet => { - // 1) FILTER > sheets with the same href - let sheetAlreadyExists = false; - if (sheet.href) { - if (!sheetHrefs.includes(sheet.href)) { - sheetHrefs.push(sheet.href); - } else { - sheetAlreadyExists = true; - } - } - // 2) FILTER > media='print' - const isPrintMedia = Array.from(sheet.media).includes('print'); + /** + * reference -> https://developer.mozilla.org/en-US/docs/Web/API/CSSRule#Type_constants + */ + const cssImportRules = rules.filter(r => r.type === 3); // type === 3 -> CSSRule.IMPORT_RULE - return !isPrintMedia && !sheetAlreadyExists; - }); + /** + * when no `@import` rules + * -> resolve the current `sheet` + */ + if (!cssImportRules.length) { + queue.defer(resolve => { + resolve({ sheet, shadowId, priority, isExternal, root: rootNode }); + }); + return; + } + + /** + * iterate `@import` rules and fetch styles + */ + cssImportRules.forEach((rule, cssRuleIndex) => + queue.defer((resolve, reject) => { + /** + * invoked with `analyzeNested` + */ + const newPriority = [...priority, cssRuleIndex]; + getStylesheetFromUrl({ + ...options, + url: rule.href, + resolve, + reject, + priority: newPriority, + queue, + analyzeNested: true + }); + }) + ); + + const nonImportCSSRules = rules.filter(r => r.type !== 3); + // no further rules to process in this sheet + if (!nonImportCSSRules.length) { + return; + } + + queue.defer(resolve => + resolve( + convertTextToStylesheet({ + // convert all `nonImportCSSRules` of the styles into `text` which will be converted to a new stylesheet + data: nonImportCSSRules + .reduce((out, rule) => { + out.push(rule.cssText); + return out; + }, []) + .join(), + shadowId, + root: rootNode, + isExternal, + priority + }) + ) + ); } /** * Returns a then(able) queue of CSSStyleSheet(s) * @method loadCssom * @private - * @param {Object} options an object with attributes essential to load CSSOM - * @property {Object} options.rootNode document or shadowDOM root document for which to process CSSOM - * @property {Number} options.rootIndex a number representing the index of the document or shadowDOM, used for priority - * @property {String} options.shadowId an id if undefined denotes that given root is a shadowRoot + * @param {Object} options configuration options + * @property {Object} options.rootNode document or document fragment + * @property {Number} options.rootIndex a number representing the index of the document or document fragment, used for priority computation + * @property {String} options.shadowId an id if undefined denotes that given root is a document fragment/ shadowDOM * @property {Number} options.timeout abort duration for network request - * @param {Function} options.getStyleSheet a utility function to generate a style sheet for a given text content + * @property {Function} options.convertTextToStylesheet a utility function to generate a style sheet from given data (text) * @return {Object} queue */ function loadCssom(options) { - const { rootNode, rootIndex, shadowId, getStyleSheet } = options; + const { rootIndex } = options; const q = axe.utils.queue(); - const styleSheets = - rootNode.nodeType === 11 && shadowId - ? getSheetsFromShadowDom(rootNode, getStyleSheet) - : Array.from(rootNode.styleSheets); - const sheets = filterStyleSheets(styleSheets); + const sheets = getStylesheetsOfRootNode(options); sheets.forEach((sheet, sheetIndex) => { - /* eslint max-statements: ["error", 20] */ const priority = [rootIndex, sheetIndex]; - try { - // The following line throws an error on cross-origin style sheets: - const cssRules = sheet.cssRules; - const rules = Array.from(cssRules); - if (!rules.length) { - return; - } - - // filter rules that are included by way of @import or nested link - const importRules = rules.filter(r => r.href); - if (!importRules.length) { - q.defer(resolve => - resolve({ - sheet, - isExternal: false, - shadowId, - root: rootNode, - priority - }) - ); - return; - } - - // for import rules, fetch via `href` - importRules.forEach(rule => { - q.defer((resolve, reject) => { - getExternalStylesheet({ - resolve, - reject, - url: rule.href, - priority, - ...options - }); - }); + parseRules({ + ...options, + sheet, + queue: q, + priority }); - - // in the same sheet - get inline rules in ' + - '
Some text
' + - '
green
' + - '
red
' + - '

Heading

'; + '' + + '
Some text
'; getPreload(shadowFixture) .then(function(results) { var sheets = results[0]; - // verify count - assert.lengthOf(sheets, 7); - // verify that the last non external sheet with shadowId has green selector - var nonExternalsheetsWithShadowId = sheets + assert.lengthOf(sheets, 2); + + var nonCrossOriginSheetsWithInShadowDOM = sheets .filter(function(s) { - return !s.isExternal; + return !s.isCrossOrigin; }) .filter(function(s) { return s.shadowId; }); assertStylesheet( - nonExternalsheetsWithShadowId[ - nonExternalsheetsWithShadowId.length - 1 + nonCrossOriginSheetsWithInShadowDOM[ + nonCrossOriginSheetsWithInShadowDOM.length - 1 ].sheet, '.green', '.green{background-color:green;}' ); - // verify priority of shadowId sheets is higher than base document - var anySheetFromBaseDocument = sheets.filter(function(s) { - return !s.shadowId; - })[0]; - var anySheetFromShadowDocument = sheets.filter(function(s) { - return s.shadowId; - })[0]; - // shadow dom priority is greater than base doc - assert.isAbove( - anySheetFromShadowDocument.priority[0], - anySheetFromBaseDocument.priority[0] - ); done(); }) .catch(done); @@ -268,124 +296,183 @@ describe('preload cssom integration test', function() { ); (shadowSupported ? it : xit)( - 'should return styles from shadow dom (handles multiple ' + - '
Some text
' + - '' + - '
green
' + - '
red
' + - '
red
' + + '' + '

Heading

'; - getPreload(shadowFixture) + + // sheet appended to root document + var stylesForPage = [styleSheets.styleTag]; + attachStylesheets({ styles: stylesForPage }, function(err) { + if (err) { + done(err); + } + getPreload(shadowFixture) + .then(function(results) { + var sheets = results[0]; + assert.lengthOf(sheets, 2); + + var shadowDomStyle = sheets.filter(function(s) { + return s.shadowId; + })[0]; + assertStylesheet( + shadowDomStyle.sheet, + '.base-style', + '.base-style { font-size: 100%; }' + ); + + var rootDocumentStyle = sheets.filter(function(s) { + return !s.shadowId; + })[0]; + assert.isAbove( + shadowDomStyle.priority[0], + rootDocumentStyle.priority[0] + ); + + axe.testUtils.removeStyleSheets(stylesForPage); + done(); + }) + .catch(done); + }); + } + ); + + it('returns styles from various @import(ed) styles from an @import(ed) stylesheet', function(done) { + var stylesForPage = [ + styleSheets.styleTagWithMultipleImports // this imports 2 other stylesheets + ]; + attachStylesheets({ styles: stylesForPage }, function(err) { + if (err) { + done(err); + } + getPreload() .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; - }); + assert.lengthOf(sheets, 2); + var nonCrossOriginSheets = sheets.filter(function(s) { + return !s.isCrossOrigin; + }); + assert.lengthOf(nonCrossOriginSheets, 2); assertStylesheet( - nonExternalsheetsWithShadowId[ - nonExternalsheetsWithShadowId.length - 2 - ].sheet, - '.green', - '.green{background-color:green;}' + nonCrossOriginSheets[0].sheet, + '.multiple-import-1', + '.multiple-import-1 { font-size: 100%; }' ); + axe.testUtils.removeStyleSheets(stylesForPage); + done(); + }) + .catch(done); + }); + }); + + it('returns style from nested @import (3 levels deep)', function(done) { + var stylesForPage = [styleSheets.styleTagWithNestedImports]; + attachStylesheets({ styles: stylesForPage }, function(err) { + if (err) { + done(err); + } + getPreload() + .then(function(results) { + var sheets = results[0]; + assert.lengthOf(sheets, 1); + var nonCrossOriginSheets = sheets.filter(function(s) { + return !s.isCrossOrigin; + }); + assert.lengthOf(nonCrossOriginSheets, 1); assertStylesheet( - nonExternalsheetsWithShadowId[ - nonExternalsheetsWithShadowId.length - 1 - ].sheet, - '.notGreen', - '.notGreen{background-color:orange;}' + nonCrossOriginSheets[0].sheet, + 'body::after', + 'body::after { content: "I-am-from-the-3rd-nested-import-stylesheet"; }' ); + axe.testUtils.removeStyleSheets(stylesForPage); done(); }) .catch(done); - } - ); + }); + }); - (shadowSupported ? it : xit)( - 'should return styles from shadow dom (handles mulitple ' + - '' + - '

Heading

'; - getPreload(shadowFixture) + it('returns style from cyclic @import (exits recursion successfully)', function(done) { + var stylesForPage = [styleSheets.styleTagWithCyclicImports]; + attachStylesheets({ styles: stylesForPage }, function(err) { + if (err) { + done(err); + } + getPreload() .then(function(results) { var sheets = results[0]; - // verify count - assert.lengthOf(sheets, 6); - - // 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; - }); - // there are no inline styles in shadowRoot - assert.lengthOf(nonExternalsheetsWithShadowId, 0); - - // ensure the output of shadowRoot sheet is that of expected external mocked response - var externalsheetsWithShadowId = sheets - .filter(function(s) { - return s.isExternal; - }) - .filter(function(s) { - return s.shadowId; - }); + assert.lengthOf(sheets, 1); assertStylesheet( - externalsheetsWithShadowId[0].sheet, - 'body', - 'body{overflow:auto;}' + sheets[0].sheet, + '.cycle-style', + '.cycle-style { font-family: inherit; }' ); + axe.testUtils.removeStyleSheets(stylesForPage); + done(); + }) + .catch(done); + }); + }); + it('returns style from cyclic @import which only imports one cross-origin stylesheet', function(done) { + var stylesForPage = [styleSheets.styleTagWithCyclicCrossOriginImports]; + attachStylesheets({ styles: stylesForPage }, function(err) { + if (err) { + done(err); + } + getPreload() + .then(function(results) { + var sheets = results[0]; + assert.lengthOf(sheets, 1); + assertStylesheet( + sheets[0].sheet, + '.container', + '.container { position: relative; width: 100%; max-width: 960px; margin: 0px auto; padding: 0px 20px; box-sizing: border-box; }' + ); + axe.testUtils.removeStyleSheets(stylesForPage); done(); }) .catch(done); - } - ); + }); + }); - commonTestsForRootAndFrame(); + commonTestsForRootNodeAndNestedFrame(); }); - describe('tests for nested iframe', function() { + describe('tests for nested document', function() { + var frame; + before(function() { if (isPhantom) { + // if `phantomjs` -> skip `suite` this.skip(); } - }); - - var frame; - - before(function() { frame = document.getElementById('frame1').contentDocument; }); - it('should return inline stylesheets defined using diff --git a/test/integration/full/preload-cssom/multiple-import-2.css b/test/integration/full/preload-cssom/multiple-import-2.css index a011355685..541d6910ba 100644 --- a/test/integration/full/preload-cssom/multiple-import-2.css +++ b/test/integration/full/preload-cssom/multiple-import-2.css @@ -1,5 +1,5 @@ /* this is referenced by multiple-import-1.css */ -.multiple-import-1 { +.style-from-multiple-import-2-css { font-size: 100%; } \ No newline at end of file diff --git a/test/integration/full/preload-cssom/multiple-import-3.css b/test/integration/full/preload-cssom/multiple-import-3.css index bb989201e0..d83324ad9c 100644 --- a/test/integration/full/preload-cssom/multiple-import-3.css +++ b/test/integration/full/preload-cssom/multiple-import-3.css @@ -1,5 +1,5 @@ -/* this is referenced by multiple-import-2.css */ +/* this is referenced by multiple-import-1.css */ -.multiple-import-2 { +.style-from-multiple-import-3-css { font-size: 100%; } \ No newline at end of file diff --git a/test/integration/full/preload-cssom/nested-import-3.css b/test/integration/full/preload-cssom/nested-import-3.css index 98c767c5ae..e0e2e60827 100644 --- a/test/integration/full/preload-cssom/nested-import-3.css +++ b/test/integration/full/preload-cssom/nested-import-3.css @@ -1,5 +1,5 @@ /* referred by nested-import-2.css */ -body::after { - content: 'I-am-from-the-3rd-nested-import-stylesheet' +.style-from-nested-import-3-css { + font-size: inherit } \ No newline at end of file diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index 389d4156a6..d3f5f7e71d 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -2,14 +2,6 @@ describe('preload cssom integration test', function() { 'use strict'; - before(function(done) { - if (isPhantom) { - // if `phantomjs` -> skip `suite` - this.skip(); - } - axe.testUtils.awaitNestedLoad(done); - }); - var shadowSupported = axe.testUtils.shadowSupport.v1; var isPhantom = window.PHANTOMJS ? true : false; var styleSheets = { @@ -49,20 +41,19 @@ describe('preload cssom integration test', function() { text: '@import "cyclic-cross-origin-import-1.css";' } }; + var stylesForPage; + var nestedFrame; - function assertStylesheet(sheet, selectorText, cssText, includes) { - assert.isDefined(sheet); - assert.property(sheet, 'cssRules'); - if (includes) { - assert.isTrue(cssText.includes(selectorText)); - } else { - assert.equal(sheet.cssRules[0].selectorText, selectorText); - assert.equal( - sheet.cssRules[0].cssText.replace(/\s/g, ''), - cssText.replace(/\s/g, '') - ); + before(function(done) { + if (isPhantom) { + // if `phantomjs` -> skip `suite` + this.skip(); } - } + axe.testUtils.awaitNestedLoad(function() { + nestedFrame = document.getElementById('frame1').contentDocument; + done(); + }); + }); function attachStylesheets(options, callback) { axe.testUtils @@ -75,10 +66,26 @@ describe('preload cssom integration test', function() { }); } - function getPreload(root) { + function detachStylesheets(done) { + if (!stylesForPage) { + done(); + } + axe.testUtils + .removeStyleSheets(stylesForPage) + .then(function() { + done(); + stylesForPage = undefined; + }) + .catch(function(err) { + done(err); + stylesForPage = undefined; + }); + } + + function getPreload(root, timeout) { var config = { asset: 'cssom', - timeout: 10000, + timeout: timeout ? timeout : 10000, treeRoot: axe.utils.getFlattenedTree(root ? root : document) }; return axe.utils.preloadCssom(config); @@ -86,7 +93,7 @@ describe('preload cssom integration test', function() { function commonTestsForRootNodeAndNestedFrame(root) { it('returns cross-origin stylesheet', function(done) { - var stylesForPage = [styleSheets.crossOriginLinkHref]; + stylesForPage = [styleSheets.crossOriginLinkHref]; attachStylesheets( { root, @@ -101,29 +108,24 @@ describe('preload cssom integration test', function() { var sheets = results[0]; assert.lengthOf(sheets, 1); var sheetData = sheets[0].sheet; - assertStylesheet( + axe.testUtils.assertStylesheet( sheetData, ':root', sheetData.cssRules[0].cssText, true ); - axe.testUtils - .removeStyleSheets(stylesForPage) - .then(function() { - done(); - }) - .catch(function(err) { - done(err); - }); + done(); }) - .catch(done); + .catch(function() { + done(new Error('Expected getPreload to resolve.')); + }); }, root ); }); it('returns no stylesheets when cross-origin stylesheets are of media=print', function(done) { - var stylesForPage = [styleSheets.crossOriginLinkHrefMediaPrint]; + stylesForPage = [styleSheets.crossOriginLinkHrefMediaPrint]; attachStylesheets( { root, @@ -137,44 +139,42 @@ describe('preload cssom integration test', function() { .then(function(results) { var sheets = results[0]; assert.lengthOf(sheets, 0); - axe.testUtils - .removeStyleSheets(stylesForPage) - .then(function() { - done(); - }) - .catch(function(err) { - done(err); - }); + done(); }) - .catch(done); + .catch(function() { + done(new Error('Expected getPreload to resolve.')); + }); }, root ); }); it('throws if cross-origin stylesheet request timeouts', function(done) { - var config = { - asset: 'cssom', - timeout: 1, - treeRoot: axe.utils.getFlattenedTree(root ? root : document) - }; - var doneCalled = false; - axe.utils - .preloadCssom(config) - .then(function() { - done(); - }) - .catch(function(error) { - assert.equal(error.message, 'timeout of 1ms exceeded'); // <-this message comes from axios - if (!doneCalled) { - doneCalled = true; - done(); + stylesForPage = [styleSheets.crossOriginLinkHref]; + attachStylesheets( + { + root, + styles: stylesForPage + }, + function(err) { + if (err) { + done(err); } - }); + getPreload(root, 1) + .then(() => { + done(new Error('Expected getPreload to reject.')); + }) + .catch(err => { + assert.isDefined(err); + done(); + }); + }, + root + ); }); it('throws if cross-origin stylesheet fail to load', function(done) { - var stylesForPage = [ + stylesForPage = [ { id: 'nonExistingStylesheet', text: '@import "import-non-existing-cross-origin.css";' @@ -191,22 +191,12 @@ describe('preload cssom integration test', function() { } var doneCalled = false; getPreload() - .then(function() { - done(); + .then(() => { + done(new Error('Expected getPreload to reject.')); }) - .catch(function(error) { - assert.equal(error.message, 'Network Error'); //<- message from `axios` - axe.testUtils - .removeStyleSheets(stylesForPage) - .then(function() { - if (!doneCalled) { - doneCalled = true; - done(); - } - }) - .catch(function(err) { - done(err); - }); + .catch(function(err) { + assert.isDefined(err); + done(); }); }, root @@ -231,44 +221,39 @@ describe('preload cssom integration test', function() { shadowFixture = document.getElementById('shadow-fixture'); }); - afterEach(function() { + afterEach(function(done) { if (shadowFixture) { document.body.removeChild(shadowFixture); } + detachStylesheets(done); }); it('returns stylesheets defined via ` and `` references to `CSSStyleSheet` object + .reduce((out, node) => { + const nodeName = node.nodeName.toUpperCase(); + const data = nodeName === 'STYLE' ? node.textContent : node; + const isLink = nodeName === 'LINK'; + const stylesheet = convertDataToStylesheet({ + data, + isLink, + root: rootNode + }); + out.push(stylesheet.sheet); + return out; + }, []) + ); +} + +/** + * Get stylesheets from `document` + * -> filter out stylesheet that are `media=print` + * + * @method getStylesheetsFromDocument + * @private + * @param {Object} rootNode `document` + * @returns {Array} + */ +function getStylesheetsFromDocument(rootNode) { + return Array.from(rootNode.styleSheets).filter(sheet => + filterMediaIsPrint(sheet.media.mediaText) + ); +} + +/** + * Get all `` and `` attributes + * -> limit to only `style` or `link` attributes with `rel=stylesheet` and `media != print` + * + * @method filerStyleAndLinkAttributesInDocumentFragment + * @private + * @param {Object} node HTMLElement + * @returns {Boolean} + */ +function filerStyleAndLinkAttributesInDocumentFragment(node) { + const nodeName = node.nodeName.toUpperCase(); + const linkHref = node.getAttribute('href'); + const linkRel = node.getAttribute('rel'); + const isLink = + nodeName === 'LINK' && + linkHref && + linkRel && + node.rel.toUpperCase().includes('STYLESHEET'); + const isStyle = nodeName === 'STYLE'; + return isStyle || (isLink && filterMediaIsPrint(node.media)); +} + +/** + * Exclude `link[rel='stylesheet]` attributes where `media=print` + * + * @method filterMediaIsPrint + * @private + * @param {String} media media value eg: 'print' + * @returns {Boolean} + */ +function filterMediaIsPrint(media) { + if (media) { + return !media.toUpperCase().includes('PRINT'); + } + return true; +} + +/** + * Exclude any duplicate `stylesheets`, that share the same `href` + * + * @method filterStylesheetsWithSameHref + * @private + * @param {Array} sheets stylesheets + * @returns {Array} + */ +function filterStylesheetsWithSameHref(sheets) { + let hrefs = []; + return sheets.filter(sheet => { + if (!sheet.href) { + // include sheets without `href` + return true; + } + // if `href` is present, ensure they are not duplicates + if (hrefs.includes(sheet.href)) { + return false; + } + hrefs.push(sheet.href); + return true; + }); +} From f8ba1605360e2e466d28f1db0edb81c29bb8c65c Mon Sep 17 00:00:00 2001 From: jkodu Date: Wed, 13 Feb 2019 11:44:41 +0000 Subject: [PATCH 07/28] refactor: updattes --- lib/core/utils/preload-cssom.js | 187 +++++++++++--------------------- 1 file changed, 61 insertions(+), 126 deletions(-) diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index 08c79d5b63..fec701265f 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -1,3 +1,5 @@ +/* eslint no-use-before-define: 0 */ + /** * Given a rootNode - construct CSSOM * -> get all source nodes (document & document fragments) within given root node @@ -27,73 +29,23 @@ axe.utils.preloadCssom = function preloadCssom({ } const dynamicDoc = document.implementation.createHTMLDocument(); - /** - * initialize factory function with dynamicDoc, which returns a function to resolve CSSStylesheet from given data - */ const convertDataToStylesheet = getStyleSheetFactory(dynamicDoc); - q.defer(getCssomForAllRootNodes); - - return q; - - /** - * Deferred function for CSSOM queue processing on all root nodes - * - * @method getCssomForAllRootNodes - * @private - * @param {Object} resolve resole callback - * @param {Object} reject reject callback - */ - function getCssomForAllRootNodes(resolve, reject) { - const rootNodesCssomQueue = axe.utils.queue(); - const rootNodesCssomQueueResults = rootNodes.reduce( - getCssomForRootNode, - rootNodesCssomQueue - ); - - rootNodesCssomQueueResults + q.defer((resolve, reject) => { + getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) .then(assets => { const cssom = processCssomAssets(assets); resolve(cssom); }) .catch(reject); - } + }); - /** - * Reducer function, that operated on a given root object - * - * @method getCssomForRootNode - * @private - * @param {Object} cssomQueue `axe.utils.queue` to defer cssom processing fro given node - * @param {Object} param Enhanced document or documentFragment object returned from `getAllRootNodesInTree` - * @param {Number} index given index of root - * @returns {Object} `axe.utils.queue` - */ - function getCssomForRootNode(cssomQueue, { rootNode, shadowId }, index) { - cssomQueue.defer((resolve, reject) => { - /** - * Load CSSOM assets for given root node - */ - const options = { - rootNode, - shadowId, - timeout, - convertDataToStylesheet, - rootIndex: index + 1 - }; - loadCssom(options) - .then(resolve) - .catch(reject); - }); - return cssomQueue; - } + return q; }; /** * Returns am array of source nodes containing `document` and `documentFragment` in a given `tree`. * - * @method getAllRootNodesInTree - * @private * @param {Object} treeRoot tree * @returns {Array} array of objects, which each object containing a root and an optional `shadowId` */ @@ -122,11 +74,7 @@ function getAllRootNodesInTree(tree) { * Convert text to CSSStyleSheet * Is a factory (closure) function, initialized with `document.implementation.createHTMLDocument()` which surfaces DOM API for creating `style` elements. * - * @method getStyleSheetFactory - * @private - * - * @param {Object} param `document.implementation.createHTMLDocument()` - * + * @param {Object} param `document.implementation.createHTMLDocument() * @param {Object} arg an object with properties to construct stylesheet * @property {String} arg.data text content of the stylesheet * @property {Boolean} arg.isExternal flag to notify if the resource was fetched from the network @@ -134,44 +82,66 @@ function getAllRootNodesInTree(tree) { * @property {Object} arg.root implementation document to create style elements * @property {String} arg.priority a number indicating the loaded priority of CSS, to denote specificity of styles contained in the sheet. */ -function getStyleSheetFactory(dynamicDoc) { - return function({ - data, +const getStyleSheetFactory = dynamicDoc => ({ + data, + isExternal, + shadowId, + root, + priority, + isLink = false +}) => { + const style = dynamicDoc.createElement('style'); + if (isLink) { + // as creating a stylesheet as link will need to be awaited + // till `onload`, it is wise to convert link href to @import statement + const text = dynamicDoc.createTextNode(`@import "${data.href}"`); + style.appendChild(text); + } else { + style.appendChild(dynamicDoc.createTextNode(data)); + } + dynamicDoc.head.appendChild(style); + return { + sheet: style.sheet, isExternal, shadowId, root, - priority, - isLink = false - }) { - const style = dynamicDoc.createElement('style'); - if (isLink) { - // as creating a stylesheet as link will need to be awaited - // till `onload`, it is wise to convert link href to @import statement - const text = dynamicDoc.createTextNode(`@import "${data.href}"`); - style.appendChild(text); - } else { - style.appendChild(dynamicDoc.createTextNode(data)); - } - dynamicDoc.head.appendChild(style); - return { - sheet: style.sheet, - isExternal, - shadowId, - root, - priority - }; + priority }; +}; + +/** + * Deferred function for CSSOM queue processing on all root nodes + * + * @param {Array} rootNodes array of root nodes, where node is an enhanced `document` or `documentFragment` object returned from `getAllRootNodesInTree` + * @param {Function} convertDataToStylesheet fn to convert given data to Stylesheet object + * @returns {Object} `axe.utils.queue` + */ +function getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) { + const q = axe.utils.queue(); + + rootNodes.forEach(({ rootNode, shadowId }, index) => + q.defer((resolve, reject) => + loadCssom({ + rootNode, + shadowId, + timeout, + convertDataToStylesheet, + rootIndex: index + 1 + }) + .then(resolve) + .catch(reject) + ) + ); + + return q; } /** * Process results from `loadCssom` queues of all root nodes * NOTE: - * using `axe.utils.queue` from various `loadCssom` paths, returns a nested array of arrays - * -> flatten arrays - * -> filter `undefined` results + * using `axe.utils.queue` from various `loadCssom` paths, returns a nested array of arrays at various depths, + * hence the need to flatten arrays * - * @method processCssomAssets - * @private * @param {Array} assets CSSOM assets for each root * @returns {Object} CSSOM object */ @@ -192,8 +162,6 @@ function processCssomAssets(nestedAssets) { /** * Returns `axe.utils.queue` of CSSStyleSheet(s) for a given root node * - * @method loadCssom - * @private * @param {Object} options configuration options * @property {Object} options.rootNode document or document fragment * @property {Number} options.rootIndex a number representing the index of the document or document fragment, used for priority computation @@ -234,8 +202,6 @@ function loadCssom(options) { /** * Parse non cross-origin stylesheets * - * @method parseNonCrossOriginStylesheet - * @private * @param {Object} sheet CSSStylesheet object * @param {Object} options `loadCssom` options * @param {Array} priority sheet priority @@ -316,7 +282,7 @@ function parseNonCrossOriginStylesheet(sheet, options, priority) { q.defer(resolve => resolve( options.convertDataToStylesheet({ - data: convertCssTextFromCssRulesToString(nonImportCSSRules), + data: nonImportCSSRules.map(rule => rule.cssText).join(), isExternal: false, priority, root: options.rootNode, @@ -326,30 +292,11 @@ function parseNonCrossOriginStylesheet(sheet, options, priority) { ); return q; - - /** - * Parse all CSSRules and interpret `cssText` and concatenate to string. - * - * @method convertCssTextFromCssRulesToString - * @private - * @param {Object} rules cssRules - * @returns {String} - */ - function convertCssTextFromCssRulesToString(rules) { - return rules - .reduce((out, rule) => { - out.push(rule.cssText); - return out; - }, []) - .join(); - } } /** * Parse cross-origin stylesheets * - * @method parseCrossOriginStylesheet - * @private * @param {String} url url from which to fetch stylesheet * @param {Object} options `loadCssom` options * @param {Array} priority sheet priority @@ -389,8 +336,6 @@ function parseCrossOriginStylesheet(url, options, priority) { /** * Get stylesheet(s) for root * - * @method getStylesheetsOfRootNode - * @private * @param {Object} options configuration options of `loadCssom` * @returns an array of stylesheets */ @@ -411,8 +356,6 @@ function getStylesheetsOfRootNode(options) { /** * Get stylesheets from `documentFragment` * - * @method getStylesheetsFromDocumentFragment - * @private * @param {Object} options configuration options of `loadCssom` * @returns {Array} */ @@ -441,8 +384,6 @@ function getStylesheetsFromDocumentFragment(options) { * Get stylesheets from `document` * -> filter out stylesheet that are `media=print` * - * @method getStylesheetsFromDocument - * @private * @param {Object} rootNode `document` * @returns {Array} */ @@ -456,8 +397,6 @@ function getStylesheetsFromDocument(rootNode) { * Get all `` and `` attributes * -> limit to only `style` or `link` attributes with `rel=stylesheet` and `media != print` * - * @method filerStyleAndLinkAttributesInDocumentFragment - * @private * @param {Object} node HTMLElement * @returns {Boolean} */ @@ -477,23 +416,19 @@ function filerStyleAndLinkAttributesInDocumentFragment(node) { /** * Exclude `link[rel='stylesheet]` attributes where `media=print` * - * @method filterMediaIsPrint - * @private * @param {String} media media value eg: 'print' * @returns {Boolean} */ function filterMediaIsPrint(media) { - if (media) { - return !media.toUpperCase().includes('PRINT'); + if (!media) { + return true; } - return true; + return !media.toUpperCase().includes('PRINT'); } /** * Exclude any duplicate `stylesheets`, that share the same `href` * - * @method filterStylesheetsWithSameHref - * @private * @param {Array} sheets stylesheets * @returns {Array} */ From 08f928e501757a55d7afda708bec5276d10ded95 Mon Sep 17 00:00:00 2001 From: jkodu Date: Wed, 13 Feb 2019 12:00:38 +0000 Subject: [PATCH 08/28] refactor: updattes --- lib/core/utils/preload-cssom.js | 42 +++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index fec701265f..3352b8de24 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -257,16 +257,17 @@ function parseNonCrossOriginStylesheet(sheet, options, priority) { }; axe.imports .axios(axiosOptions) - .then(({ data }) => { - const sheet = options.convertDataToStylesheet({ - data, - isExternal: true, - priority: newPriority, - root: options.rootNode, - shadowId: options.shadowId - }); - resolve(sheet); - }) + .then(({ data }) => + resolve( + options.convertDataToStylesheet({ + data, + isExternal: true, + priority: newPriority, + root: options.rootNode, + shadowId: options.shadowId + }) + ) + ) .catch(reject); }) ); @@ -317,16 +318,17 @@ function parseCrossOriginStylesheet(url, options, priority) { q.defer((resolve, reject) => { axe.imports .axios(axiosOptions) - .then(({ data }) => { - const sheet = options.convertDataToStylesheet({ - data, - isExternal: true, - priority, - root: options.rootNode, - shadowId: options.shadowId - }); - resolve(sheet); - }) + .then(({ data }) => + resolve( + options.convertDataToStylesheet({ + data, + isExternal: true, + priority, + root: options.rootNode, + shadowId: options.shadowId + }) + ) + ) .catch(reject); }); From 6d9e6aacf7ecd42545ba0d31b6b4c5811d7c08a9 Mon Sep 17 00:00:00 2001 From: jkodu Date: Thu, 21 Feb 2019 10:07:44 +0000 Subject: [PATCH 09/28] docs: remote @instance as method is static --- lib/core/utils/preload-cssom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index 3352b8de24..e12f49a7be 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -7,7 +7,7 @@ * * @method preloadCssom * @memberof `axe.utils` - * @instance + * * @param {Object} object argument which is a composite object, with attributes timeout, treeRoot(optional), resolve & reject * @property {Number} timeout timeout for any network calls made * @property {Object} treeRoot the DOM tree to be inspected From 213e569491669b8c3334d2735c9d0801de0ad1b5 Mon Sep 17 00:00:00 2001 From: jkodu Date: Mon, 11 Mar 2019 14:20:22 +0000 Subject: [PATCH 10/28] refactor: preload cssom for import url resolution --- lib/core/constants.js | 16 +- .../utils/parse-crossorigin-stylesheets.js | 56 ++++ .../utils/parse-sameorigin-stylesheets.js | 124 ++++++++ lib/core/utils/parse-stylesheets.js | 90 ++++++ lib/core/utils/preload-cssom.js | 275 ++++-------------- lib/core/utils/preload.js | 33 ++- test/core/utils/preload-cssom.js | 17 +- test/core/utils/preload.js | 38 +-- .../full/preload-cssom/preload-cssom.js | 2 +- test/integration/full/preload/preload.js | 2 +- 10 files changed, 369 insertions(+), 284 deletions(-) create mode 100644 lib/core/utils/parse-crossorigin-stylesheets.js create mode 100644 lib/core/utils/parse-sameorigin-stylesheets.js create mode 100644 lib/core/utils/parse-stylesheets.js diff --git a/lib/core/constants.js b/lib/core/constants.js index af9897ba86..ae22bebd10 100644 --- a/lib/core/constants.js +++ b/lib/core/constants.js @@ -32,8 +32,20 @@ resultGroups: [], resultGroupMap: {}, impact: Object.freeze(['minor', 'moderate', 'serious', 'critical']), - preloadAssets: Object.freeze(['cssom']), // overtime this array will grow with other preload asset types, this constant is to verify if a requested preload type by the user via the configuration is supported by axe. - preloadAssetsTimeout: 10000 + preload: Object.freeze({ + /** + * array of supported & preload(able) asset types. + */ + assets: ['cssom'], + /** + * timeout value when resolving preload(able) assets + */ + timeout: 10000, + /** + * maximum `@import` urls to resolve before exiting (safety catch for recursion) + */ + maxImportUrls: 10 + }) }; definitions.forEach(function(definition) { diff --git a/lib/core/utils/parse-crossorigin-stylesheets.js b/lib/core/utils/parse-crossorigin-stylesheets.js new file mode 100644 index 0000000000..6df5ecbe7e --- /dev/null +++ b/lib/core/utils/parse-crossorigin-stylesheets.js @@ -0,0 +1,56 @@ +/** + * Parse cross-origin stylesheets + * + * @param {String} url url from which to fetch stylesheet + * @param {Object} options `loadCssom` options + * @param {Array} priority sheet priority + * @param {Array} importedUrls urls of already imported stylesheets + * @param {Boolean} isCrossOrigin boolean denoting if a stylesheet is `cross-origin` + * @returns {Promise} + */ +axe.utils.parseCrossOriginStylesheets = function parseCrossOriginStylesheets( + url, + options, + priority, + importedUrls, + isCrossOrigin +) { + const axiosOptions = { + method: 'get', + url, + timeout: options.timeout + }; + + importedUrls.push(url); + return axe.imports + .axios(axiosOptions) + .then(({ data }) => { + const result = options.convertDataToStylesheet({ + data, + isCrossOrigin, + priority, + root: options.rootNode, + shadowId: options.shadowId + }); + + /** + * Note: + * Safety check to stop recursion, if there are numerous nested `@import` statements + */ + if (importedUrls.length > axe.constants.preload.maxImportUrls) { + return Promise.resolve(result); + } + + /** + * Parse resolved `cross-origin` stylesheet further for any `@import` styles + */ + return axe.utils.parseStylesheet( + result.sheet, + options, + priority, + importedUrls, + result.isCrossOrigin + ); + }) + .catch(e => Promise.reject(e)); +}; diff --git a/lib/core/utils/parse-sameorigin-stylesheets.js b/lib/core/utils/parse-sameorigin-stylesheets.js new file mode 100644 index 0000000000..9863f820c1 --- /dev/null +++ b/lib/core/utils/parse-sameorigin-stylesheets.js @@ -0,0 +1,124 @@ +/** + * Parse non cross-origin stylesheets + * + * @param {Object} sheet CSSStylesheet object + * @param {Object} options `loadCssom` options + * @param {Array} priority sheet priority + * @param {Array} importedUrls urls of already imported stylesheets + * @param {Boolean} isCrossOrigin boolean denoting if a stylesheet is `cross-origin` + * @returns {Promise} + */ +axe.utils.parseSameOriginStylesheets = function parseSameOriginStylesheets( + sheet, + options, + priority, + importedUrls, + isCrossOrigin = false +) { + const rules = Array.from(sheet.cssRules); + + if (!rules) { + return Promise.resolve(); + } + + /** + * reference -> https://developer.mozilla.org/en-US/docs/Web/API/CSSRule#Type_constants + */ + const cssImportRules = rules.filter(r => r.type === 3); // type === 3 -> CSSRule.IMPORT_RULE + + /** + * when no `@import` rules in given sheet -> resolve the current `sheet` & exit + */ + if (!cssImportRules.length) { + // exit + return Promise.resolve({ + isCrossOrigin, + priority, + root: options.rootNode, + shadowId: options.shadowId, + sheet + }); + } + + /** + * filter rules that are not already fetched + */ + const cssImportUrlsNotAlreadyImported = cssImportRules + // ensure rule has a href + .filter(rule => rule.href) + // extract href from object + .map(rule => rule.href) + // only href that are not already imported + .filter(url => !importedUrls.includes(url)); + + /** + * iterate `@import` rules and fetch styles + */ + const promises = cssImportUrlsNotAlreadyImported.map( + (importUrl, cssRuleIndex) => { + const newPriority = [...priority, cssRuleIndex]; + const axiosOptions = { + method: 'get', + url: importUrl, + timeout: options.timeout + }; + const isCrossOriginRequest = /^https?:\/\/|^\/\//i.test(importUrl); + + importedUrls.push(importUrl); + return axe.imports + .axios(axiosOptions) + .then(({ data }) => { + const result = options.convertDataToStylesheet({ + data, + isCrossOrigin: isCrossOriginRequest, + priority: newPriority, + root: options.rootNode, + shadowId: options.shadowId + }); + + /** + * Note: + * Safety check to stop recursion, if there are numerous nested `@import` statements + */ + if (importedUrls.length > axe.constants.preload.maxImportUrls) { + return Promise.resolve(result); + } + + /** + * Parse resolved `@import` stylesheet further for any `@import` styles + */ + return axe.utils.parseStylesheet( + result.sheet, + options, + newPriority, + importedUrls + ); + }) + .catch(e => Promise.reject(e)); + } + ); + + const nonImportCSSRules = rules.filter(r => r.type !== 3); + + // no further rules to process in this sheet + if (!nonImportCSSRules.length) { + return Promise.all(promises); + } + + // convert all `nonImportCSSRules` style rules into `text` and chain + promises.push( + new Promise(resolve => + resolve( + options.convertDataToStylesheet({ + data: nonImportCSSRules.map(rule => rule.cssText).join(), + isCrossOrigin, + priority, + root: options.rootNode, + shadowId: options.shadowId + }) + ) + ) + ); + + return Promise.all(promises); +}; diff --git a/lib/core/utils/parse-stylesheets.js b/lib/core/utils/parse-stylesheets.js new file mode 100644 index 0000000000..7c2d652f4b --- /dev/null +++ b/lib/core/utils/parse-stylesheets.js @@ -0,0 +1,90 @@ +/** + * Returns all CSS stylesheets for a given root node + * + * @param {Object} options configuration options + * @property {Object} options.rootNode document or document fragment + * @property {Number} options.rootIndex a number representing the index of the document or document fragment, used for priority computation + * @property {String} options.shadowId an id if undefined denotes that given root is a document fragment/ shadowDOM + * @property {Number} options.timeout abort duration for network request + * @property {Function} options.convertDataToStylesheet a utility function to generate a style sheet from given data (text) + * @returns {Promise} + */ +axe.utils.parseStylesheets = function parseStylesheets( + sheets, + options, + importedUrls = [] +) { + /** + * Note: + * `importedUrls` - keeps urls of already imported stylesheets, to prevent re-fetching + * eg: nested, cyclic or cross referenced `@import` urls + */ + const { rootIndex } = options; + return Promise.all( + sheets.map((sheet, sheetIndex) => { + const priority = [rootIndex, sheetIndex]; + return axe.utils.parseStylesheet(sheet, options, priority, importedUrls); + }) + ); +}; + +/** + * Parses a given stylesheet + * + * @param {Object} sheet stylesheet to parse + * @param {Object} options configuration options object from `axe.utils.parseStylesheets` + * @param {Array} priority priority of stylesheet + * @param {Array} importedUrls list of resolved `@import` urls + * @param {Boolean} isCrossOrigin boolean denoting if a stylesheet is `cross-origin`, passed for re-parsing `cross-origin` sheets + * @returns {Promise} + */ +axe.utils.parseStylesheet = function parseStylesheet( + sheet, + options, + priority, + importedUrls, + isCrossOrigin = false +) { + const isSameOrigin = isSameOriginStylesheet(sheet); + if (isSameOrigin) { + /** + * resolve `same-origin` stylesheet + */ + return axe.utils.parseSameOriginStylesheets( + sheet, + options, + priority, + importedUrls, + isCrossOrigin + ); + } + + /** + * resolve `cross-origin` stylesheet + */ + return axe.utils.parseCrossOriginStylesheets( + sheet.href, + options, + priority, + importedUrls, + true // -> isCrossOrigin + ); +}; + +/** + * Check if a given stylesheet is from the `same-origin` + * Note: + * `sheet.cssRules` throws an error on `cross-origin` stylesheets + * + * @param {Object} sheet CSS stylesheet + * @returns {Boolean} + */ +function isSameOriginStylesheet(sheet) { + try { + /*eslint no-unused-vars: 0*/ + const rules = sheet.cssRules; + return true; + } catch (e) { + return false; + } +} diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index 180a502360..cf8c4b0715 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -15,7 +15,7 @@ * @param {Object} object argument which is a composite object, with attributes timeout, treeRoot(optional), resolve & reject * @property {Number} timeout timeout for any network calls made * @property {Object} treeRoot the DOM tree to be inspected - * @returns {Object} `axe.utils.queue` with CSSOM assets + * @returns {Promise} */ axe.utils.preloadCssom = function preloadCssom({ timeout, @@ -26,25 +26,26 @@ axe.utils.preloadCssom = function preloadCssom({ */ const rootNodes = getAllRootNodesInTree(treeRoot); - const q = axe.utils.queue(); + const promises = []; if (!rootNodes.length) { - return q; + return Promise.all(promises); } const dynamicDoc = document.implementation.createHTMLDocument(); const convertDataToStylesheet = getStyleSheetFactory(dynamicDoc); - q.defer((resolve, reject) => { + const p = new Promise((resolve, reject) => { getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) .then(assets => { - const cssom = processCssomAssets(assets); + const cssom = flattenAssets(assets); resolve(cssom); }) .catch(reject); }); + promises.push(p); - return q; + return Promise.all(promises); }; /** @@ -81,14 +82,15 @@ function getAllRootNodesInTree(tree) { * @param {Object} param `document.implementation.createHTMLDocument() * @param {Object} arg an object with properties to construct stylesheet * @property {String} arg.data text content of the stylesheet - * @property {Boolean} arg.isExternal flag to notify if the resource was fetched from the network + * @property {Boolean} arg.isCrossOrigin flag to notify if the resource was fetched from the network * @property {String} arg.shadowId (Optional) shadowId if shadowDOM * @property {Object} arg.root implementation document to create style elements * @property {String} arg.priority a number indicating the loaded priority of CSS, to denote specificity of styles contained in the sheet. + * @returns {Function} */ const getStyleSheetFactory = dynamicDoc => ({ data, - isExternal, + isCrossOrigin, shadowId, root, priority, @@ -106,7 +108,7 @@ const getStyleSheetFactory = dynamicDoc => ({ dynamicDoc.head.appendChild(style); return { sheet: style.sheet, - isExternal, + isCrossOrigin, shadowId, root, priority @@ -114,244 +116,71 @@ const getStyleSheetFactory = dynamicDoc => ({ }; /** - * Deferred function for CSSOM queue processing on all root nodes + * Process CSSOM on all root nodes * * @param {Array} rootNodes array of root nodes, where node is an enhanced `document` or `documentFragment` object returned from `getAllRootNodesInTree` * @param {Function} convertDataToStylesheet fn to convert given data to Stylesheet object - * @returns {Object} `axe.utils.queue` + * @returns {Promise} */ function getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) { - const q = axe.utils.queue(); + const promises = []; - rootNodes.forEach(({ rootNode, shadowId }, index) => - q.defer((resolve, reject) => - loadCssom({ - rootNode, - shadowId, - timeout, - convertDataToStylesheet, - rootIndex: index + 1 - }) - .then(resolve) - .catch(reject) - ) - ); - - return q; -} - -/** - * Process results from `loadCssom` queues of all root nodes - * NOTE: - * using `axe.utils.queue` from various `loadCssom` paths, returns a nested array of arrays at various depths, - * hence the need to flatten arrays - * - * @param {Array} assets CSSOM assets for each root - * @returns {Object} CSSOM object - */ -function processCssomAssets(nestedAssets) { - const result = []; - - nestedAssets.forEach(item => { - if (Array.isArray(item)) { - result.push(...processCssomAssets(item)); - } else { - result.push(item); + rootNodes.forEach(({ rootNode, shadowId }, index) => { + const sheets = getStylesheetsOfRootNode( + rootNode, + shadowId, + convertDataToStylesheet + ); + if (!sheets) { + return Promise.all(promises); } - }); - - return result; -} - -/** - * Returns `axe.utils.queue` of CSSStyleSheet(s) for a given root node - * - * @param {Object} options configuration options - * @property {Object} options.rootNode document or document fragment - * @property {Number} options.rootIndex a number representing the index of the document or document fragment, used for priority computation - * @property {String} options.shadowId an id if undefined denotes that given root is a document fragment/ shadowDOM - * @property {Number} options.timeout abort duration for network request - * @property {Function} options.convertDataToStylesheet a utility function to generate a style sheet from given data (text) - * @return {Object} queue - */ -function loadCssom(options) { - const { rootIndex } = options; - const q = axe.utils.queue(); - - const sheets = getStylesheetsOfRootNode(options); - if (!sheets) { - return q; - } - - sheets.forEach((sheet, sheetIndex) => { - const priority = [rootIndex, sheetIndex]; - try { - const deferredQ = parseNonCrossOriginStylesheet(sheet, options, priority); - q.defer(deferredQ); - } catch (e) { - // cross-origin stylesheet -> make an XHR and q the response - const deferredQ = parseCrossOriginStylesheet( - sheet.href, - options, - priority - ); - q.defer(deferredQ); - } + const parseOptions = { + rootNode, + shadowId, + timeout, + convertDataToStylesheet, + rootIndex: index + 1 + }; + const p = axe.utils.parseStylesheets(sheets, parseOptions); + promises.push(p); }); - return q; + return Promise.all(promises); } /** - * Parse non cross-origin stylesheets + * Flatten CSSOM assets * - * @param {Object} sheet CSSStylesheet object - * @param {Object} options `loadCssom` options - * @param {Array} priority sheet priority + * @param {[Array]} assets nested assets (varying depth) + * @returns {Array} Array of CSSOM object */ -function parseNonCrossOriginStylesheet(sheet, options, priority) { - const q = axe.utils.queue(); - - /** - * `sheet.cssRules` throws an error on `cross-origin` stylesheets - */ - const cssRules = sheet.cssRules; - - const rules = Array.from(cssRules); - if (!rules) { - return q; - } - - /** - * reference -> https://developer.mozilla.org/en-US/docs/Web/API/CSSRule#Type_constants - */ - const cssImportRules = rules.filter(r => r.type === 3); // type === 3 -> CSSRule.IMPORT_RULE - - /** - * when no `@import` rules in given sheet - * -> resolve the current `sheet` & exit - */ - if (!cssImportRules.length) { - q.defer(resolve => - resolve({ - isExternal: false, - priority, - root: options.rootNode, - shadowId: options.shadowId, - sheet - }) - ); - - // exit - return q; - } - - /** - * iterate `@import` rules and fetch styles - */ - cssImportRules.forEach((importRule, cssRuleIndex) => - q.defer((resolve, reject) => { - const importUrl = importRule.href; - const newPriority = [...priority, cssRuleIndex]; - const axiosOptions = { - method: 'get', - url: importUrl, - timeout: options.timeout - }; - axe.imports - .axios(axiosOptions) - .then(({ data }) => - resolve( - options.convertDataToStylesheet({ - data, - isExternal: true, - priority: newPriority, - root: options.rootNode, - shadowId: options.shadowId - }) - ) - ) - .catch(reject); - }) - ); - - const nonImportCSSRules = rules.filter(r => r.type !== 3); - - // no further rules to process in this sheet - if (!nonImportCSSRules.length) { - return q; - } - - // convert all `nonImportCSSRules` style rules into `text` and defer into queue - q.defer(resolve => - resolve( - options.convertDataToStylesheet({ - data: nonImportCSSRules.map(rule => rule.cssText).join(), - isExternal: false, - priority, - root: options.rootNode, - shadowId: options.shadowId - }) - ) +function flattenAssets(assets) { + return assets.reduce( + (acc, val) => + Array.isArray(val) ? acc.concat(flattenAssets(val)) : acc.concat(val), + [] ); - - return q; -} - -/** - * Parse cross-origin stylesheets - * - * @param {String} url url from which to fetch stylesheet - * @param {Object} options `loadCssom` options - * @param {Array} priority sheet priority - */ -function parseCrossOriginStylesheet(url, options, priority) { - const q = axe.utils.queue(); - - if (!url) { - return q; - } - - const axiosOptions = { - method: 'get', - url, - timeout: options.timeout - }; - - q.defer((resolve, reject) => { - axe.imports - .axios(axiosOptions) - .then(({ data }) => - resolve( - options.convertDataToStylesheet({ - data, - isExternal: true, - priority, - root: options.rootNode, - shadowId: options.shadowId - }) - ) - ) - .catch(reject); - }); - - return q; } /** * Get stylesheet(s) for root * * @param {Object} options configuration options of `loadCssom` - * @returns an array of stylesheets + * @property {Object} options.rootNode `document` or `documentFragment` + * @property {String} options.shadowId an id if undefined denotes that given root is a document fragment/ shadowDOM + * @property {Function} options.convertDataToStylesheet a utility function to generate a style sheet from given data (text) + * @returns {Array} an array of stylesheets */ -function getStylesheetsOfRootNode(options) { - const { rootNode, shadowId } = options; +function getStylesheetsOfRootNode(rootNode, shadowId, convertDataToStylesheet) { let sheets; // nodeType === 11 -> DOCUMENT_FRAGMENT if (rootNode.nodeType === 11 && shadowId) { - sheets = getStylesheetsFromDocumentFragment(options); + sheets = getStylesheetsFromDocumentFragment( + rootNode, + convertDataToStylesheet + ); } else { sheets = getStylesheetsFromDocument(rootNode); } @@ -362,11 +191,11 @@ function getStylesheetsOfRootNode(options) { /** * Get stylesheets from `documentFragment` * - * @param {Object} options configuration options of `loadCssom` + * @property {Object} options.rootNode `documentFragment` + * @property {Function} options.convertDataToStylesheet a utility function to generate a stylesheet from given data * @returns {Array} */ -function getStylesheetsFromDocumentFragment(options) { - const { rootNode, convertDataToStylesheet } = options; +function getStylesheetsFromDocumentFragment(rootNode, convertDataToStylesheet) { return ( Array.from(rootNode.children) .filter(filerStyleAndLinkAttributesInDocumentFragment) diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index 38cee08077..f80bf773bc 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -26,13 +26,13 @@ axe.utils.shouldPreload = function shouldPreload(options) { /** * Constructs a configuration object representing the preload requested assets & timeout * @param {Object} options run configuration options (or defaults) passed via axe.run - * @return {Object} + * @return {Object} configuration */ axe.utils.getPreloadConfig = function getPreloadConfig(options) { - // default fallback configuration + const { defaultAssets, defaultTimeout } = axe.constants.preload; const config = { - assets: axe.constants.preloadAssets, - timeout: axe.constants.preloadAssetsTimeout + assets: defaultAssets, + timeout: defaultTimeout }; // if no `preload` is configured via `options` - return default config @@ -47,13 +47,13 @@ axe.utils.getPreloadConfig = function getPreloadConfig(options) { // check if requested assets to preload are valid items const areRequestedAssetsValid = options.preload.assets.every(a => - axe.constants.preloadAssets.includes(a.toLowerCase()) + defaultAssets.includes(a.toLowerCase()) ); if (!areRequestedAssetsValid) { throw new Error( `Requested assets, not supported. ` + - `Supported assets are: ${axe.constants.preloadAssets.join(', ')}.` + `Supported assets are: ${defaultAssets.join(', ')}.` ); } @@ -74,36 +74,37 @@ axe.utils.getPreloadConfig = function getPreloadConfig(options) { }; /** - * Returns a then(able) queue with results of all requested preload(able) assets. Eg: ['cssom']. - * If preload is set to false, returns an empty queue. + * Returns a Promise with results of all requested preload(able) assets. eg: ['cssom']. + * * @param {Object} options run configuration options (or defaults) passed via axe.run - * @return {Object} queue + * @return {Object} Promise */ axe.utils.preload = function preload(options) { const preloadFunctionsMap = { cssom: axe.utils.preloadCssom }; - const q = axe.utils.queue(); + const promises = []; const shouldPreload = axe.utils.shouldPreload(options); if (!shouldPreload) { - return q; + return Promise.all(promises); } const preloadConfig = axe.utils.getPreloadConfig(options); preloadConfig.assets.forEach(asset => { - q.defer((resolve, reject) => { + const p = new Promise((resolve, reject) => { preloadFunctionsMap[asset](preloadConfig) - .then(results => { + .then(results => resolve({ [asset]: results[0] - }); - }) + }) + ) .catch(reject); }); + promises.push(p); }); - return q; + return Promise.all(promises); }; diff --git a/test/core/utils/preload-cssom.js b/test/core/utils/preload-cssom.js index fb51d17477..8bcfdb66ba 100644 --- a/test/core/utils/preload-cssom.js +++ b/test/core/utils/preload-cssom.js @@ -1,3 +1,10 @@ +/** + * NOTE: + * `document.styleSheets` does not recognize dynamically injected stylesheets after `load` via `beforeEach`/ `before`, + * so tests for disabled and external stylesheets are done in `integration` tests + * Refer Directory: `./test/full/preload-cssom/**.*` + */ + describe('axe.utils.preloadCssom unit tests', function() { 'use strict'; @@ -39,8 +46,9 @@ describe('axe.utils.preloadCssom unit tests', function() { it('should return a queue', function() { var actual = axe.utils.preloadCssom(args); - assert.isObject(actual); - assert.containsAllKeys(actual, ['then', 'defer', 'catch']); + assert.isTrue( + Object.prototype.toString.call(actual) === '[object Promise]' + ); }); it('should ensure result of cssom is an array of sheets', function(done) { @@ -107,9 +115,4 @@ describe('axe.utils.preloadCssom unit tests', function() { done(error); }); }); - - /** - * NOTE: document.styleSheets does not recognise dynamically injected stylesheets after load via beforeEach/ before, so tests for disabled and external stylesheets are done in integration - * Refer Directory: ./test/full/preload-cssom/**.* - */ }); diff --git a/test/core/utils/preload.js b/test/core/utils/preload.js index 2a26e8a7b2..5b217383d1 100644 --- a/test/core/utils/preload.js +++ b/test/core/utils/preload.js @@ -1,41 +1,11 @@ describe('axe.utils.preload', function() { 'use strict'; - it('should return a queue', function() { + it('should return a Promise', function() { var actual = axe.utils.preload({}); - assert.isObject(actual); - assert.containsAllKeys(actual, ['then', 'defer', 'catch']); - }); - - it('should ensure queue is defer(able)', function(done) { - var options = { - preload: false - }; - var actual = axe.utils.preload(options); - actual - .defer(function(res, rej) { - assert.isFunction(rej); - res(true); - done(); - }) - .catch(function(error) { - done(error); - }); - }); - - it('should ensure queue is then(able)', function(done) { - var options = { - preload: false - }; - var actual = axe.utils.preload(options); - actual - .then(function(results) { - assert.isDefined(results); - done(); - }) - .catch(function(error) { - done(error); - }); + assert.isTrue( + Object.prototype.toString.call(actual) === '[object Promise]' + ); }); it('should return empty array as result', function(done) { diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index baa9913005..f7ae01c4db 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -1,4 +1,4 @@ -/* global axe, Promise */ +/* global axe */ describe('preload cssom integration test', function() { 'use strict'; diff --git a/test/integration/full/preload/preload.js b/test/integration/full/preload/preload.js index e8ccf063c2..352de395fc 100644 --- a/test/integration/full/preload/preload.js +++ b/test/integration/full/preload/preload.js @@ -1,4 +1,4 @@ -/* global axe, Promise */ +/* global axe */ describe('preload integration test', function() { 'use strict'; From 40699b6ec16e08b8cdf581af138ecfaee23334cc Mon Sep 17 00:00:00 2001 From: jkodu Date: Mon, 11 Mar 2019 15:15:27 +0000 Subject: [PATCH 11/28] fix: tests and update preload config computation --- .../utils/parse-crossorigin-stylesheets.js | 2 +- .../utils/parse-sameorigin-stylesheets.js | 2 +- lib/core/utils/preload-cssom.js | 9 +- lib/core/utils/preload.js | 10 +- test/core/base/audit.js | 125 +++++++++--------- test/core/utils/preload-cssom.js | 10 +- test/core/utils/preload.js | 8 ++ .../full/css-orientation-lock/passes.js | 1 - .../full/css-orientation-lock/violations.js | 1 - .../full/preload-cssom/preload-cssom.js | 4 - test/integration/full/preload/preload.js | 1 - 11 files changed, 92 insertions(+), 81 deletions(-) diff --git a/lib/core/utils/parse-crossorigin-stylesheets.js b/lib/core/utils/parse-crossorigin-stylesheets.js index 6df5ecbe7e..06d8ea19aa 100644 --- a/lib/core/utils/parse-crossorigin-stylesheets.js +++ b/lib/core/utils/parse-crossorigin-stylesheets.js @@ -2,7 +2,7 @@ * Parse cross-origin stylesheets * * @param {String} url url from which to fetch stylesheet - * @param {Object} options `loadCssom` options + * @param {Object} options options object from `axe.utils.parseStylesheets` * @param {Array} priority sheet priority * @param {Array} importedUrls urls of already imported stylesheets * @param {Boolean} isCrossOrigin boolean denoting if a stylesheet is `cross-origin` diff --git a/lib/core/utils/parse-sameorigin-stylesheets.js b/lib/core/utils/parse-sameorigin-stylesheets.js index 9863f820c1..8e6283eed2 100644 --- a/lib/core/utils/parse-sameorigin-stylesheets.js +++ b/lib/core/utils/parse-sameorigin-stylesheets.js @@ -2,7 +2,7 @@ * Parse non cross-origin stylesheets * * @param {Object} sheet CSSStylesheet object - * @param {Object} options `loadCssom` options + * @param {Object} options options object from `axe.utils.parseStylesheets` * @param {Array} priority sheet priority * @param {Array} importedUrls urls of already imported stylesheets * @param {Boolean} isCrossOrigin boolean denoting if a stylesheet is `cross-origin` diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index cf8c4b0715..383c50e87d 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -7,7 +7,7 @@ /** * Given a rootNode - construct CSSOM * -> get all source nodes (document & document fragments) within given root node - * -> recursively call `loadCssom` to resolve styles + * -> recursively call `axe.utils.parseStylesheets` to resolve styles for each node * * @method preloadCssom * @memberof `axe.utils` @@ -166,10 +166,9 @@ function flattenAssets(assets) { /** * Get stylesheet(s) for root * - * @param {Object} options configuration options of `loadCssom` - * @property {Object} options.rootNode `document` or `documentFragment` - * @property {String} options.shadowId an id if undefined denotes that given root is a document fragment/ shadowDOM - * @property {Function} options.convertDataToStylesheet a utility function to generate a style sheet from given data (text) + * @param {Object} options.rootNode `document` or `documentFragment` + * @param {String} options.shadowId an id if undefined denotes that given root is a document fragment/ shadowDOM + * @param {Function} options.convertDataToStylesheet a utility function to generate a style sheet from given data (text) * @returns {Array} an array of stylesheets */ function getStylesheetsOfRootNode(rootNode, shadowId, convertDataToStylesheet) { diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index f80bf773bc..ce8c711782 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -29,10 +29,10 @@ axe.utils.shouldPreload = function shouldPreload(options) { * @return {Object} configuration */ axe.utils.getPreloadConfig = function getPreloadConfig(options) { - const { defaultAssets, defaultTimeout } = axe.constants.preload; + const { assets, timeout } = axe.constants.preload; const config = { - assets: defaultAssets, - timeout: defaultTimeout + assets, + timeout }; // if no `preload` is configured via `options` - return default config @@ -47,13 +47,13 @@ axe.utils.getPreloadConfig = function getPreloadConfig(options) { // check if requested assets to preload are valid items const areRequestedAssetsValid = options.preload.assets.every(a => - defaultAssets.includes(a.toLowerCase()) + assets.includes(a.toLowerCase()) ); if (!areRequestedAssetsValid) { throw new Error( `Requested assets, not supported. ` + - `Supported assets are: ${defaultAssets.join(', ')}.` + `Supported assets are: ${assets.join(', ')}.` ); } diff --git a/test/core/base/audit.js b/test/core/base/audit.js index 4f31701fc6..56775c5859 100644 --- a/test/core/base/audit.js +++ b/test/core/base/audit.js @@ -747,7 +747,7 @@ describe('Audit', function() { ); }); - it('should continue to run rules and return result when preload is rejected', function(done) { + it.skip('should continue to run rules and return result when preload is rejected', function(done) { fixture.innerHTML = '
'; var preloadOverrideInvoked = false; @@ -821,67 +821,70 @@ describe('Audit', function() { ); }); - it('should continue to run rules and return result when axios time(s)out and rejects preload', function(done) { - fixture.innerHTML = '
'; - - // there is no stubbing here, - // the actual axios call is invoked, and timedout immediately as timeout is set to 0.1 - - var preloadNeededCheckInvoked = false; - var audit = new Audit(); - // add a rule and check that does not need preload - audit.addRule({ - id: 'no-preload', - selector: 'div#div1', - preload: false - }); - // add a rule which needs preload - audit.addRule({ - id: 'yes-preload', - selector: 'div#div2', - preload: true, - any: ['yes-preload-check'] - }); - audit.addCheck({ - id: 'yes-preload-check', - evaluate: function(node, options, vNode, context) { - preloadNeededCheckInvoked = true; - this.data(context); - return true; - } - }); - - var preloadOptions = { - preload: { - assets: ['cssom'], - timeout: 0.1 - } - }; - audit.run( - { include: [axe.utils.getFlattenedTree(fixture)[0]] }, - { - preload: preloadOptions - }, - function(results) { - assert.isDefined(results); - // assert that both rules ran, although preload failed - assert.lengthOf(results, 2); + (window.PHANTOMJS ? xit : it)( + 'should continue to run rules and return result when axios time(s)out and rejects preload', + function(done) { + fixture.innerHTML = '
'; + + // there is no stubbing here, + // the actual axios call is invoked, and timedout immediately as timeout is set to 0.1 + + var preloadNeededCheckInvoked = false; + var audit = new Audit(); + // add a rule and check that does not need preload + audit.addRule({ + id: 'no-preload', + selector: 'div#div1', + preload: false + }); + // add a rule which needs preload + audit.addRule({ + id: 'yes-preload', + selector: 'div#div2', + preload: true, + any: ['yes-preload-check'] + }); + audit.addCheck({ + id: 'yes-preload-check', + evaluate: function(node, options, vNode, context) { + preloadNeededCheckInvoked = true; + this.data(context); + return true; + } + }); - // assert that because preload failed - // cssom was not populated on context of repective check - assert.isTrue(preloadNeededCheckInvoked); - var ruleResult = results.filter(function(r) { - return (r.id = 'yes-preload' && r.nodes.length > 0); - })[0]; - var checkResult = ruleResult.nodes[0].any[0]; - assert.isDefined(checkResult.data); - assert.notProperty(checkResult.data, ['cssom']); - // done - done(); - }, - noop - ); - }); + var preloadOptions = { + preload: { + assets: ['cssom'], + timeout: 0.1 + } + }; + audit.run( + { include: [axe.utils.getFlattenedTree(fixture)[0]] }, + { + preload: preloadOptions + }, + function(results) { + assert.isDefined(results); + // assert that both rules ran, although preload failed + assert.lengthOf(results, 2); + + // assert that because preload failed + // cssom was not populated on context of repective check + assert.isTrue(preloadNeededCheckInvoked); + var ruleResult = results.filter(function(r) { + return (r.id = 'yes-preload' && r.nodes.length > 0); + })[0]; + var checkResult = ruleResult.nodes[0].any[0]; + assert.isDefined(checkResult.data); + assert.notProperty(checkResult.data, ['cssom']); + // done + done(); + }, + noop + ); + } + ); it('should assign an empty array to axe._selectCache', function(done) { var saved = axe.utils.ruleShouldRun; diff --git a/test/core/utils/preload-cssom.js b/test/core/utils/preload-cssom.js index 8bcfdb66ba..25b1b32c43 100644 --- a/test/core/utils/preload-cssom.js +++ b/test/core/utils/preload-cssom.js @@ -8,6 +8,14 @@ describe('axe.utils.preloadCssom unit tests', function() { 'use strict'; + var isPhantom = window.PHANTOMJS ? true : false; + + before(function() { + if (isPhantom) { + this.skip(); // if `phantomjs` -> skip `suite` + } + }); + var args; function addStyleToHead() { @@ -44,7 +52,7 @@ describe('axe.utils.preloadCssom unit tests', function() { assert.isFunction(axe.utils.preloadCssom); }); - it('should return a queue', function() { + it('should return a Promise', function() { var actual = axe.utils.preloadCssom(args); assert.isTrue( Object.prototype.toString.call(actual) === '[object Promise]' diff --git a/test/core/utils/preload.js b/test/core/utils/preload.js index 5b217383d1..75d40ad051 100644 --- a/test/core/utils/preload.js +++ b/test/core/utils/preload.js @@ -1,6 +1,14 @@ describe('axe.utils.preload', function() { 'use strict'; + var isPhantom = window.PHANTOMJS ? true : false; + + before(function() { + if (isPhantom) { + this.skip(); // if `phantomjs` -> skip `suite` + } + }); + it('should return a Promise', function() { var actual = axe.utils.preload({}); assert.isTrue( diff --git a/test/integration/full/css-orientation-lock/passes.js b/test/integration/full/css-orientation-lock/passes.js index 1b67802b56..40e97f06f4 100644 --- a/test/integration/full/css-orientation-lock/passes.js +++ b/test/integration/full/css-orientation-lock/passes.js @@ -18,7 +18,6 @@ describe('css-orientation-lock passes test', function() { before(function(done) { if (isPhantom) { this.skip(); - done(); } else { axe.testUtils .addStyleSheets(styleSheets) diff --git a/test/integration/full/css-orientation-lock/violations.js b/test/integration/full/css-orientation-lock/violations.js index 4d28a9a516..b4e0526792 100644 --- a/test/integration/full/css-orientation-lock/violations.js +++ b/test/integration/full/css-orientation-lock/violations.js @@ -17,7 +17,6 @@ describe('css-orientation-lock violations test', function() { before(function(done) { if (isPhantom) { this.skip(); - done(); } else { axe.testUtils .addStyleSheets(styleSheets) diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index f7ae01c4db..3861b5e804 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -45,10 +45,6 @@ describe('preload cssom integration test', function() { var nestedFrame; before(function(done) { - if (isPhantom) { - // if `phantomjs` -> skip `suite` - this.skip(); - } axe.testUtils.awaitNestedLoad(function() { nestedFrame = document.getElementById('frame1').contentDocument; done(); diff --git a/test/integration/full/preload/preload.js b/test/integration/full/preload/preload.js index 352de395fc..34ef5ce7ef 100644 --- a/test/integration/full/preload/preload.js +++ b/test/integration/full/preload/preload.js @@ -21,7 +21,6 @@ describe('preload integration test', function() { before(function(done) { if (isPhantom) { this.skip(); - done(); } else { // load custom rule // load custom check From a0ab521fda5a4d0380bb50a73ade6c848dd12ef3 Mon Sep 17 00:00:00 2001 From: jkodu Date: Mon, 11 Mar 2019 15:36:46 +0000 Subject: [PATCH 12/28] fix: tests and refacttor --- .../utils/parse-crossorigin-stylesheets.js | 55 +++++++------- .../utils/parse-sameorigin-stylesheets.js | 72 +++++++++---------- lib/core/utils/preload-cssom.js | 5 +- lib/core/utils/preload.js | 27 +++---- test/integration/full/preload/preload.js | 2 +- 5 files changed, 73 insertions(+), 88 deletions(-) diff --git a/lib/core/utils/parse-crossorigin-stylesheets.js b/lib/core/utils/parse-crossorigin-stylesheets.js index 06d8ea19aa..73e80674a9 100644 --- a/lib/core/utils/parse-crossorigin-stylesheets.js +++ b/lib/core/utils/parse-crossorigin-stylesheets.js @@ -22,35 +22,32 @@ axe.utils.parseCrossOriginStylesheets = function parseCrossOriginStylesheets( }; importedUrls.push(url); - return axe.imports - .axios(axiosOptions) - .then(({ data }) => { - const result = options.convertDataToStylesheet({ - data, - isCrossOrigin, - priority, - root: options.rootNode, - shadowId: options.shadowId - }); + return axe.imports.axios(axiosOptions).then(({ data }) => { + const result = options.convertDataToStylesheet({ + data, + isCrossOrigin, + priority, + root: options.rootNode, + shadowId: options.shadowId + }); - /** - * Note: - * Safety check to stop recursion, if there are numerous nested `@import` statements - */ - if (importedUrls.length > axe.constants.preload.maxImportUrls) { - return Promise.resolve(result); - } + /** + * Note: + * Safety check to stop recursion, if there are numerous nested `@import` statements + */ + if (importedUrls.length > axe.constants.preload.maxImportUrls) { + return result; + } - /** - * Parse resolved `cross-origin` stylesheet further for any `@import` styles - */ - return axe.utils.parseStylesheet( - result.sheet, - options, - priority, - importedUrls, - result.isCrossOrigin - ); - }) - .catch(e => Promise.reject(e)); + /** + * Parse resolved `cross-origin` stylesheet further for any `@import` styles + */ + return axe.utils.parseStylesheet( + result.sheet, + options, + priority, + importedUrls, + result.isCrossOrigin + ); + }); }; diff --git a/lib/core/utils/parse-sameorigin-stylesheets.js b/lib/core/utils/parse-sameorigin-stylesheets.js index 8e6283eed2..d54db8ed95 100644 --- a/lib/core/utils/parse-sameorigin-stylesheets.js +++ b/lib/core/utils/parse-sameorigin-stylesheets.js @@ -65,36 +65,33 @@ axe.utils.parseSameOriginStylesheets = function parseSameOriginStylesheets( const isCrossOriginRequest = /^https?:\/\/|^\/\//i.test(importUrl); importedUrls.push(importUrl); - return axe.imports - .axios(axiosOptions) - .then(({ data }) => { - const result = options.convertDataToStylesheet({ - data, - isCrossOrigin: isCrossOriginRequest, - priority: newPriority, - root: options.rootNode, - shadowId: options.shadowId - }); + return axe.imports.axios(axiosOptions).then(({ data }) => { + const result = options.convertDataToStylesheet({ + data, + isCrossOrigin: isCrossOriginRequest, + priority: newPriority, + root: options.rootNode, + shadowId: options.shadowId + }); - /** - * Note: - * Safety check to stop recursion, if there are numerous nested `@import` statements - */ - if (importedUrls.length > axe.constants.preload.maxImportUrls) { - return Promise.resolve(result); - } + /** + * Note: + * Safety check to stop recursion, if there are numerous nested `@import` statements + */ + if (importedUrls.length > axe.constants.preload.maxImportUrls) { + return result; + } - /** - * Parse resolved `@import` stylesheet further for any `@import` styles - */ - return axe.utils.parseStylesheet( - result.sheet, - options, - newPriority, - importedUrls - ); - }) - .catch(e => Promise.reject(e)); + /** + * Parse resolved `@import` stylesheet further for any `@import` styles + */ + return axe.utils.parseStylesheet( + result.sheet, + options, + newPriority, + importedUrls + ); + }); } ); @@ -106,17 +103,16 @@ axe.utils.parseSameOriginStylesheets = function parseSameOriginStylesheets( } // convert all `nonImportCSSRules` style rules into `text` and chain + promises.push( - new Promise(resolve => - resolve( - options.convertDataToStylesheet({ - data: nonImportCSSRules.map(rule => rule.cssText).join(), - isCrossOrigin, - priority, - root: options.rootNode, - shadowId: options.shadowId - }) - ) + Promise.resolve( + options.convertDataToStylesheet({ + data: nonImportCSSRules.map(rule => rule.cssText).join(), + isCrossOrigin, + priority, + root: options.rootNode, + shadowId: options.shadowId + }) ) ); diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index 383c50e87d..d660398d77 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -26,15 +26,14 @@ axe.utils.preloadCssom = function preloadCssom({ */ const rootNodes = getAllRootNodesInTree(treeRoot); - const promises = []; - if (!rootNodes.length) { - return Promise.all(promises); + return Promise.resolve(); } const dynamicDoc = document.implementation.createHTMLDocument(); const convertDataToStylesheet = getStyleSheetFactory(dynamicDoc); + const promises = []; const p = new Promise((resolve, reject) => { getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) .then(assets => { diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index ce8c711782..ef448afa19 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -84,27 +84,20 @@ axe.utils.preload = function preload(options) { cssom: axe.utils.preloadCssom }; - const promises = []; - const shouldPreload = axe.utils.shouldPreload(options); if (!shouldPreload) { - return Promise.all(promises); + return Promise.resolve(); } const preloadConfig = axe.utils.getPreloadConfig(options); - preloadConfig.assets.forEach(asset => { - const p = new Promise((resolve, reject) => { - preloadFunctionsMap[asset](preloadConfig) - .then(results => - resolve({ - [asset]: results[0] - }) - ) - .catch(reject); - }); - promises.push(p); - }); - - return Promise.all(promises); + return Promise.all( + preloadConfig.assets.map(asset => { + return preloadFunctionsMap[asset](preloadConfig).then(results => + Promise.resolve({ + [asset]: results[0] + }) + ); + }) + ); }; diff --git a/test/integration/full/preload/preload.js b/test/integration/full/preload/preload.js index 34ef5ce7ef..d7d6214af7 100644 --- a/test/integration/full/preload/preload.js +++ b/test/integration/full/preload/preload.js @@ -116,7 +116,7 @@ describe('preload integration test', function() { var crossOriginSheet = cssom.filter(function(s) { return s.isCrossOrigin; }); - assert.lengthOf(crossOriginSheet, 0); + assert.lengthOf(crossOriginSheet, 1); var inlineStylesheet = cssom.filter(function(s) { return s.sheet.cssRules.length === 1 && !s.isCrossOrigin; From 0424d03c61467d66d8c371c44d774025965140a6 Mon Sep 17 00:00:00 2001 From: jkodu Date: Mon, 11 Mar 2019 16:03:23 +0000 Subject: [PATCH 13/28] fix: tests and refacttor --- lib/core/utils/preload-cssom.js | 18 ++++------ test/core/base/audit.js | 2 +- test/core/utils/preload-cssom.js | 17 ++++------ test/core/utils/preload.js | 6 ++-- .../full/preload-cssom/preload-cssom.js | 33 +++++++------------ test/integration/full/preload/preload.js | 1 - 6 files changed, 27 insertions(+), 50 deletions(-) diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index d660398d77..28789bc080 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -33,18 +33,14 @@ axe.utils.preloadCssom = function preloadCssom({ const dynamicDoc = document.implementation.createHTMLDocument(); const convertDataToStylesheet = getStyleSheetFactory(dynamicDoc); - const promises = []; - const p = new Promise((resolve, reject) => { - getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) - .then(assets => { - const cssom = flattenAssets(assets); - resolve(cssom); - }) - .catch(reject); + return getCssomForAllRootNodes( + rootNodes, + convertDataToStylesheet, + timeout + ).then(assets => { + const cssom = flattenAssets(assets); + return cssom; }); - promises.push(p); - - return Promise.all(promises); }; /** diff --git a/test/core/base/audit.js b/test/core/base/audit.js index 56775c5859..c87f96eb71 100644 --- a/test/core/base/audit.js +++ b/test/core/base/audit.js @@ -747,7 +747,7 @@ describe('Audit', function() { ); }); - it.skip('should continue to run rules and return result when preload is rejected', function(done) { + it('should continue to run rules and return result when preload is rejected', function(done) { fixture.innerHTML = '
'; var preloadOverrideInvoked = false; diff --git a/test/core/utils/preload-cssom.js b/test/core/utils/preload-cssom.js index 25b1b32c43..f99176495d 100644 --- a/test/core/utils/preload-cssom.js +++ b/test/core/utils/preload-cssom.js @@ -62,9 +62,7 @@ describe('axe.utils.preloadCssom unit tests', function() { it('should ensure result of cssom is an array of sheets', function(done) { var actual = axe.utils.preloadCssom(args); actual - .then(function(results) { - // returned from queue, hence the index look up - var cssom = results[0]; + .then(function(cssom) { assert.isAtLeast(cssom.length, 2); done(); }) @@ -76,9 +74,7 @@ describe('axe.utils.preloadCssom unit tests', function() { it('ensure that each of the cssom object have defined properties', function(done) { var actual = axe.utils.preloadCssom(args); actual - .then(function(results) { - // returned from queue, hence the index look up - var cssom = results[0]; + .then(function(cssom) { assert.isAtLeast(cssom.length, 2); cssom.forEach(function(o) { assert.hasAllKeys(o, [ @@ -99,8 +95,8 @@ describe('axe.utils.preloadCssom unit tests', function() { it('should fail if number of sheets returned does not match stylesheets defined in document', function(done) { var actual = axe.utils.preloadCssom(args); actual - .then(function(results) { - assert.isFalse(results[0].length <= 1); // returned from queue, hence the index look up + .then(function(cssom) { + assert.isFalse(cssom.length <= 1); done(); }) .catch(function(error) { @@ -111,9 +107,8 @@ describe('axe.utils.preloadCssom unit tests', function() { it('should ensure all returned stylesheet is defined and has property cssRules', function(done) { var actual = axe.utils.preloadCssom(args); actual - .then(function(results) { - var sheets = results[0]; - sheets.forEach(function(s) { + .then(function(cssom) { + cssom.forEach(function(s) { assert.isDefined(s.sheet); assert.property(s.sheet, 'cssRules'); }); diff --git a/test/core/utils/preload.js b/test/core/utils/preload.js index 75d40ad051..5f6bfd2c5a 100644 --- a/test/core/utils/preload.js +++ b/test/core/utils/preload.js @@ -16,16 +16,14 @@ describe('axe.utils.preload', function() { ); }); - it('should return empty array as result', function(done) { + it('should return `undefined` as result', function(done) { var options = { preload: false }; var actual = axe.utils.preload(options); actual .then(function(results) { - assert.isDefined(results); - assert.isArray(results); - assert.lengthOf(results, 0); + assert.isUndefined(results); done(); }) .catch(function(error) { diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index 3861b5e804..7ebab45bf4 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -100,8 +100,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload(root) - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 1); var sheetData = sheets[0].sheet; axe.testUtils.assertStylesheet( @@ -132,8 +131,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 0); done(); }) @@ -230,8 +228,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 1); var sheetData = sheets[0].sheet; axe.testUtils.assertStylesheet( @@ -254,8 +251,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 1); var nonCrossOriginSheets = sheets.filter(function(s) { return !s.isCrossOrigin; @@ -287,8 +283,7 @@ describe('preload cssom integration test', function() { '' + '
Some text
'; getPreload(shadowFixture) - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 2); var nonCrossOriginSheetsWithInShadowDOM = sheets .filter(function(s) { @@ -330,8 +325,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload(shadowFixture) - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 2); var shadowDomStyle = sheets.filter(function(s) { @@ -368,8 +362,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 2); var nonCrossOriginSheets = sheets.filter(function(s) { return !s.isCrossOrigin; @@ -400,8 +393,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 1); var nonCrossOriginSheets = sheets.filter(function(s) { return !s.isCrossOrigin; @@ -427,8 +419,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 1); axe.testUtils.assertStylesheet( sheets[0].sheet, @@ -450,8 +441,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload() - .then(function(results) { - var sheets = results[0]; + .then(function(sheets) { assert.lengthOf(sheets, 1); axe.testUtils.assertStylesheet( sheets[0].sheet, @@ -482,8 +472,7 @@ describe('preload cssom integration test', function() { it('returns styles defined using From ba6d4c3719e78da38e51a15e0de2c240679745ae Mon Sep 17 00:00:00 2001 From: jkodu Date: Thu, 25 Apr 2019 14:09:20 +0100 Subject: [PATCH 23/28] update tests --- test/integration/full/preload-cssom/preload-cssom.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index 2c401cef86..e619cc3851 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -161,7 +161,7 @@ describe('preload cssom integration test', function() { done(err); } getPreload(root) - .then(() => { + .then(function() { done(new Error('Expected getPreload to reject.')); }) .catch(function(err) { @@ -432,17 +432,17 @@ describe('preload cssom integration test', function() { }); }); - it('throws if cross-origin stylesheet request timeouts', function(done) { + it.only('throws if cross-origin stylesheet request timeouts', function(done) { stylesForPage = [styleSheets.crossOriginLinkHref]; attachStylesheets({ styles: stylesForPage }, function(err) { if (err) { done(err); } getPreload(undefined, 1) - .then(() => { + .then(function() { done(new Error('Expected getPreload to reject.')); }) - .catch(err => { + .catch(function(err) { assert.isDefined(err); done(); }); From c395e87e182bd0a4ebe435366a3bf05ccd449af7 Mon Sep 17 00:00:00 2001 From: jkodu Date: Thu, 25 Apr 2019 14:53:17 +0100 Subject: [PATCH 24/28] update test --- test/integration/full/preload-cssom/preload-cssom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index e619cc3851..94e3fb11bd 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -432,7 +432,7 @@ describe('preload cssom integration test', function() { }); }); - it.only('throws if cross-origin stylesheet request timeouts', function(done) { + it('throws if cross-origin stylesheet request timeouts', function(done) { stylesForPage = [styleSheets.crossOriginLinkHref]; attachStylesheets({ styles: stylesForPage }, function(err) { if (err) { From f228798ddedb025133f6b08e1842988fb498051a Mon Sep 17 00:00:00 2001 From: jkodu Date: Tue, 30 Apr 2019 15:23:25 +0100 Subject: [PATCH 25/28] respect preload timeout on a higher level --- lib/core/constants.js | 6 +- .../utils/parse-crossorigin-stylesheets.js | 12 +--- lib/core/utils/preload.js | 66 +++++++++++-------- 3 files changed, 42 insertions(+), 42 deletions(-) diff --git a/lib/core/constants.js b/lib/core/constants.js index ae22bebd10..02d40619b8 100644 --- a/lib/core/constants.js +++ b/lib/core/constants.js @@ -40,11 +40,7 @@ /** * timeout value when resolving preload(able) assets */ - timeout: 10000, - /** - * maximum `@import` urls to resolve before exiting (safety catch for recursion) - */ - maxImportUrls: 10 + timeout: 10000 }) }; diff --git a/lib/core/utils/parse-crossorigin-stylesheets.js b/lib/core/utils/parse-crossorigin-stylesheets.js index a29706de39..e0c4ddfc99 100644 --- a/lib/core/utils/parse-crossorigin-stylesheets.js +++ b/lib/core/utils/parse-crossorigin-stylesheets.js @@ -17,8 +17,7 @@ axe.utils.parseCrossOriginStylesheets = function parseCrossOriginStylesheets( ) { const axiosOptions = { method: 'get', - url, - timeout: options.timeout + url }; importedUrls.push(url); @@ -31,15 +30,6 @@ axe.utils.parseCrossOriginStylesheets = function parseCrossOriginStylesheets( shadowId: options.shadowId }); - /** - * Note: - * Safety check to stop recursion, - * if there are numerous nested `@import` statements - */ - if (importedUrls.length > axe.constants.preload.maxImportUrls) { - return result; - } - /** * Parse resolved stylesheet further for any `@import` styles */ diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index a72be7ddb7..69673acde9 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -89,34 +89,48 @@ axe.utils.preload = function preload(options) { return Promise.resolve(); } - const preloadConfig = axe.utils.getPreloadConfig(options); + return new Promise((resolve, reject) => { + /** + * Start `timeout` timer for preloading assets + * -> reject if allowed time expires. + */ + setTimeout( + () => reject(`Preload assets timed out.`), + axe.constants.preload.timeout + ); - return Promise.all( - preloadConfig.assets.map(asset => - preloadFunctionsMap[asset](preloadConfig).then(results => { - return { - [asset]: results - }; - }) - ) - ).then(results => { /** - * Note: - * Combine array of results into an object map - * - * From -> - * [{cssom: [...], aom: [...]}] - * To -> - * { - * cssom: [...] - * aom: [...] - * } + * Fetch requested `assets` */ - return results.reduce((out, result) => { - return { - ...out, - ...result - }; - }, {}); + const preloadConfig = axe.utils.getPreloadConfig(options); + Promise.all( + preloadConfig.assets.map(asset => + preloadFunctionsMap[asset](preloadConfig).then(results => { + return { + [asset]: results + }; + }) + ) + ).then(results => { + /** + * Combine array of results into an object map + * + * From -> + * [{cssom: [...], aom: [...]}] + * To -> + * { + * cssom: [...] + * aom: [...] + * } + */ + const preloadAssets = results.reduce((out, result) => { + return { + ...out, + ...result + }; + }, {}); + + resolve(preloadAssets); + }); }); }; From 5ed7b4d69169634e4f0a713e209d88e802d126cc Mon Sep 17 00:00:00 2001 From: jkodu Date: Wed, 1 May 2019 10:10:25 +0100 Subject: [PATCH 26/28] update tests for ttimeout --- lib/core/utils/preload-cssom.js | 24 ++++++-------- lib/core/utils/preload.js | 6 ++-- test/core/utils/preload-cssom.js | 32 +++++++------------ test/core/utils/preload.js | 26 +++++++-------- .../full/preload-cssom/preload-cssom.js | 27 ++-------------- test/integration/full/preload/preload.js | 31 ++++++++---------- 6 files changed, 52 insertions(+), 94 deletions(-) diff --git a/lib/core/utils/preload-cssom.js b/lib/core/utils/preload-cssom.js index ca44e2b7fe..df2c7ecfa1 100644 --- a/lib/core/utils/preload-cssom.js +++ b/lib/core/utils/preload-cssom.js @@ -11,16 +11,13 @@ * * @method preloadCssom * @memberof `axe.utils` - * - * @param {Object} object argument which is a composite object, with attributes timeout, treeRoot(optional), resolve & reject - * @property {Number} timeout timeout for any network calls made - * @property {Object} treeRoot the DOM tree to be inspected + * @param {Object} options composite options object + * @property {Array} options.assets array of preloaded assets requested, eg: [`cssom`] + * @property {Number} options.timeout timeout + * @property {Object} options.treeRoot (optional) the DOM tree to be inspected * @returns {Promise} */ -axe.utils.preloadCssom = function preloadCssom({ - timeout, - treeRoot = axe._tree[0] -}) { +axe.utils.preloadCssom = function preloadCssom({ treeRoot = axe._tree[0] }) { /** * get all `document` and `documentFragment` with in given `tree` */ @@ -36,11 +33,9 @@ axe.utils.preloadCssom = function preloadCssom({ const convertDataToStylesheet = getStyleSheetFactory(dynamicDoc); - return getCssomForAllRootNodes( - rootNodes, - convertDataToStylesheet, - timeout - ).then(assets => flattenAssets(assets)); + return getCssomForAllRootNodes(rootNodes, convertDataToStylesheet).then( + assets => flattenAssets(assets) + ); }; /** @@ -117,7 +112,7 @@ const getStyleSheetFactory = dynamicDoc => ({ * @param {Function} convertDataToStylesheet fn to convert given data to Stylesheet object * @returns {Promise} */ -function getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) { +function getCssomForAllRootNodes(rootNodes, convertDataToStylesheet) { const promises = []; rootNodes.forEach(({ rootNode, shadowId }, index) => { @@ -133,7 +128,6 @@ function getCssomForAllRootNodes(rootNodes, convertDataToStylesheet, timeout) { const parseOptions = { rootNode, shadowId, - timeout, convertDataToStylesheet, rootIndex: index + 1 }; diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index 69673acde9..2ce19ada61 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -90,19 +90,21 @@ axe.utils.preload = function preload(options) { } return new Promise((resolve, reject) => { + const preloadConfig = axe.utils.getPreloadConfig(options); + /** * Start `timeout` timer for preloading assets * -> reject if allowed time expires. */ setTimeout( () => reject(`Preload assets timed out.`), - axe.constants.preload.timeout + preloadConfig.timeout ); /** * Fetch requested `assets` */ - const preloadConfig = axe.utils.getPreloadConfig(options); + Promise.all( preloadConfig.assets.map(asset => preloadFunctionsMap[asset](preloadConfig).then(results => { diff --git a/test/core/utils/preload-cssom.js b/test/core/utils/preload-cssom.js index f99176495d..7a7c924ea3 100644 --- a/test/core/utils/preload-cssom.js +++ b/test/core/utils/preload-cssom.js @@ -16,7 +16,7 @@ describe('axe.utils.preloadCssom unit tests', function() { } }); - var args; + var treeRoot; function addStyleToHead() { var css = 'html {font-size: inherit;}'; @@ -37,30 +37,22 @@ describe('axe.utils.preloadCssom unit tests', function() { beforeEach(function() { addStyleToHead(); - args = { - asset: 'cssom', - timeout: 10000, - treeRoot: (axe._tree = axe.utils.getFlattenedTree(document)) - }; + treeRoot = axe._tree = axe.utils.getFlattenedTree(document); }); afterEach(function() { removeStyleFromHead(); }); - it('should be a function', function() { - assert.isFunction(axe.utils.preloadCssom); - }); - - it('should return a Promise', function() { - var actual = axe.utils.preloadCssom(args); + it('returns a Promise', function() { + var actual = axe.utils.preloadCssom({ treeRoot }); assert.isTrue( Object.prototype.toString.call(actual) === '[object Promise]' ); }); - it('should ensure result of cssom is an array of sheets', function(done) { - var actual = axe.utils.preloadCssom(args); + it('returns CSSOM object containing an array of sheets', function(done) { + var actual = axe.utils.preloadCssom({ treeRoot }); actual .then(function(cssom) { assert.isAtLeast(cssom.length, 2); @@ -71,8 +63,8 @@ describe('axe.utils.preloadCssom unit tests', function() { }); }); - it('ensure that each of the cssom object have defined properties', function(done) { - var actual = axe.utils.preloadCssom(args); + it('returns CSSOM and ensure that each object have defined properties', function(done) { + var actual = axe.utils.preloadCssom({ treeRoot }); actual .then(function(cssom) { assert.isAtLeast(cssom.length, 2); @@ -92,8 +84,8 @@ describe('axe.utils.preloadCssom unit tests', function() { }); }); - it('should fail if number of sheets returned does not match stylesheets defined in document', function(done) { - var actual = axe.utils.preloadCssom(args); + it('returns false if number of sheets returned does not match stylesheets defined in document', function(done) { + var actual = axe.utils.preloadCssom({ treeRoot }); actual .then(function(cssom) { assert.isFalse(cssom.length <= 1); @@ -104,8 +96,8 @@ describe('axe.utils.preloadCssom unit tests', function() { }); }); - it('should ensure all returned stylesheet is defined and has property cssRules', function(done) { - var actual = axe.utils.preloadCssom(args); + it('returns all stylesheets and ensure each sheet has property cssRules', function(done) { + var actual = axe.utils.preloadCssom({ treeRoot }); actual .then(function(cssom) { cssom.forEach(function(s) { diff --git a/test/core/utils/preload.js b/test/core/utils/preload.js index 63dd769398..1737dc041c 100644 --- a/test/core/utils/preload.js +++ b/test/core/utils/preload.js @@ -11,14 +11,14 @@ describe('axe.utils.preload', function() { axe._tree = axe.utils.getFlattenedTree(fixture); }); - it('should return a Promise', function() { + it('returns a Promise', function() { var actual = axe.utils.preload({}); assert.isTrue( Object.prototype.toString.call(actual) === '[object Promise]' ); }); - it('should return `undefined` as result', function(done) { + it('returns `undefined` as result', function(done) { var options = { preload: false }; @@ -33,26 +33,22 @@ describe('axe.utils.preload', function() { }); }); - it('should return an object with property cssom and verify result is same output from preloadCssom', function(done) { + it('returns assets with `cssom`, cross-verify result is same output from `preloadCssom` fn', function(done) { var options = { preload: { assets: ['cssom'] } }; var actual = axe.utils.preload(options); - actual - .then(function(results) { - assert.isDefined(results); - assert.property(results, 'cssom'); - // also verify that result from css matches that of preloadCssom - axe.utils.preloadCssom(options).then(function(resultFromPreloadCssom) { - assert.deepEqual(results.cssom, resultFromPreloadCssom); - done(); - }); - }) - .catch(function(error) { - done(error); + actual.then(function(results) { + assert.isDefined(results); + assert.property(results, 'cssom'); + + axe.utils.preloadCssom(options).then(function(resultFromPreloadCssom) { + assert.deepEqual(results.cssom, resultFromPreloadCssom); + done(); }); + }); }); describe('axe.utils.shouldPreload', function() { diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index 94e3fb11bd..d15f795fac 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -79,13 +79,9 @@ describe('preload cssom integration test', function() { }); } - function getPreload(root, timeout) { - var config = { - asset: 'cssom', - timeout: timeout ? timeout : 10000, - treeRoot: axe.utils.getFlattenedTree(root ? root : document) - }; - return axe.utils.preloadCssom(config); + function getPreload(root) { + const treeRoot = axe.utils.getFlattenedTree(root ? root : document); + return axe.utils.preloadCssom({ treeRoot }); } function commonTestsForRootNodeAndNestedFrame(root) { @@ -432,23 +428,6 @@ describe('preload cssom integration test', function() { }); }); - it('throws if cross-origin stylesheet request timeouts', function(done) { - stylesForPage = [styleSheets.crossOriginLinkHref]; - attachStylesheets({ styles: stylesForPage }, function(err) { - if (err) { - done(err); - } - getPreload(undefined, 1) - .then(function() { - done(new Error('Expected getPreload to reject.')); - }) - .catch(function(err) { - assert.isDefined(err); - done(); - }); - }); - }); - commonTestsForRootNodeAndNestedFrame(); }); diff --git a/test/integration/full/preload/preload.js b/test/integration/full/preload/preload.js index 165932813f..fed88acfae 100644 --- a/test/integration/full/preload/preload.js +++ b/test/integration/full/preload/preload.js @@ -87,20 +87,16 @@ describe('preload integration test', function() { }); } - it('returns CSSOM data to checks evaluate function for the custom rule which required preloaded assets', function(done) { + it("returns preloaded assets to the check's evaluate fn for the rule which has `preload:true`", function(done) { axe.run( { runOnly: { type: 'rule', values: ['run-later-rule'] }, - // run config asks to preload, and the rule requires a preload as well, context will be mutated with 'cssom' asset - preload: { - assets: ['cssom'] - } + preload: true }, function(err, res) { - // we ensure preload was skipped by checking context does not have cssom in checks evaluate function assert.isNull(err); assert.isDefined(res); assert.property(res, 'passes'); @@ -110,15 +106,17 @@ describe('preload integration test', function() { assert.property(checkData, 'cssom'); var cssom = checkData.cssom; - assert.lengthOf(cssom, 3); // ignore all media='print' styleSheets + + // ignores all media='print' styleSheets + assert.lengthOf(cssom, 3); // there should be no external sheet returned - // as everything is earmarked as media print var crossOriginSheet = cssom.filter(function(s) { return s.isCrossOrigin; }); assert.lengthOf(crossOriginSheet, 1); + // verify content of stylesheet var inlineStylesheet = cssom.filter(function(s) { return s.sheet.cssRules.length === 1 && !s.isCrossOrigin; })[0].sheet; @@ -133,20 +131,16 @@ describe('preload integration test', function() { ); }); - it('retuns no CSSOM data to checks which does not require preload(true)', function(done) { + it("returns NO preloaded assets to the check which does not require preload'", function(done) { axe.run( { runOnly: { type: 'rule', values: ['run-now-rule'] }, - // run config asks to preload, but no rule mandates preload, so preload is skipped - preload: { - assets: ['cssom'] - } + preload: true }, function(err, res) { - // we ensure preload was skipped by checking context does not have cssom in checks evaluate function assert.isNull(err); assert.isDefined(res); assert.property(res, 'passes'); @@ -159,21 +153,19 @@ describe('preload integration test', function() { ); }); - it('returns results for rule (executes the rule & check) although preloading of assets is timed out', function(done) { + it('returns results for rule (that requires preloaded assets) although preload timed out', function(done) { axe.run( { runOnly: { type: 'rule', values: ['run-later-rule'] }, - // run config asks to preload, and the rule requires a preload as well, context will be mutated with 'cssom' asset preload: { assets: ['cssom'], timeout: 1 } }, function(err, res) { - // we ensure preload was skipped by checking context does not have cssom in checks evaluate function assert.isNull(err); assert.isDefined(res); assert.property(res, 'passes'); @@ -187,7 +179,10 @@ describe('preload integration test', function() { ); }); - it('returns no CSSOM for rule when preload assets is rejected (due to attempting to load non existing resource)', function(done) { + it('returns no preloaded assets for rule when preload assets is rejected', function(done) { + /** + * Note: Attempting to load a non-existing stylesheet will reject the preload function + */ var stylesForPage = [ { id: 'nonExistingStylesheet', From 5ca679fb34071be93369f65662b0234786f784ed Mon Sep 17 00:00:00 2001 From: jkodu Date: Wed, 1 May 2019 11:31:19 +0100 Subject: [PATCH 27/28] update tests --- lib/core/utils/preload.js | 11 +- .../full/preload-cssom/preload-cssom.js | 26 +- test/integration/full/preload/preload.html | 10 +- test/integration/full/preload/preload.js | 387 +++++++++--------- 4 files changed, 220 insertions(+), 214 deletions(-) diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index 2ce19ada61..945e40f788 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -90,24 +90,21 @@ axe.utils.preload = function preload(options) { } return new Promise((resolve, reject) => { - const preloadConfig = axe.utils.getPreloadConfig(options); + const { assets, timeout } = axe.utils.getPreloadConfig(options); /** * Start `timeout` timer for preloading assets * -> reject if allowed time expires. */ - setTimeout( - () => reject(`Preload assets timed out.`), - preloadConfig.timeout - ); + setTimeout(() => reject(`Preload assets timed out.`), timeout); /** * Fetch requested `assets` */ Promise.all( - preloadConfig.assets.map(asset => - preloadFunctionsMap[asset](preloadConfig).then(results => { + assets.map(asset => + preloadFunctionsMap[asset](options).then(results => { return { [asset]: results }; diff --git a/test/integration/full/preload-cssom/preload-cssom.js b/test/integration/full/preload-cssom/preload-cssom.js index d15f795fac..b592a913f8 100644 --- a/test/integration/full/preload-cssom/preload-cssom.js +++ b/test/integration/full/preload-cssom/preload-cssom.js @@ -79,7 +79,7 @@ describe('preload cssom integration test', function() { }); } - function getPreload(root) { + function getPreloadCssom(root) { const treeRoot = axe.utils.getFlattenedTree(root ? root : document); return axe.utils.preloadCssom({ treeRoot }); } @@ -96,7 +96,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload(root) + getPreloadCssom(root) .then(function(sheets) { assert.lengthOf(sheets, 1); var sheetData = sheets[0].sheet; @@ -127,7 +127,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload(root) + getPreloadCssom(root) .then(function(sheets) { assert.lengthOf(sheets, 0); done(); @@ -156,7 +156,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload(root) + getPreloadCssom(root) .then(function() { done(new Error('Expected getPreload to reject.')); }) @@ -199,7 +199,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload() + getPreloadCssom() .then(function(sheets) { assert.lengthOf(sheets, 1); var sheetData = sheets[0].sheet; @@ -222,7 +222,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload() + getPreloadCssom() .then(function(sheets) { assert.lengthOf(sheets, 1); var nonCrossOriginSheets = sheets.filter(function(s) { @@ -254,7 +254,7 @@ describe('preload cssom integration test', function() { '.green { background-color: green; } ' + '' + '
Some text
'; - getPreload(shadowFixture) + getPreloadCssom(shadowFixture) .then(function(sheets) { assert.lengthOf(sheets, 2); var nonCrossOriginSheetsWithInShadowDOM = sheets @@ -296,7 +296,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload(shadowFixture) + getPreloadCssom(shadowFixture) .then(function(sheets) { assert.lengthOf(sheets, 2); @@ -333,7 +333,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload() + getPreloadCssom() .then(function(sheets) { assert.lengthOf(sheets, 2); var nonCrossOriginSheets = sheets.filter(function(s) { @@ -364,7 +364,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload() + getPreloadCssom() .then(function(sheets) { assert.lengthOf(sheets, 1); var nonCrossOriginSheets = sheets.filter(function(s) { @@ -390,7 +390,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload() + getPreloadCssom() .then(function(sheets) { assert.lengthOf(sheets, 1); axe.testUtils.assertStylesheet( @@ -412,7 +412,7 @@ describe('preload cssom integration test', function() { if (err) { done(err); } - getPreload() + getPreloadCssom() .then(function(sheets) { assert.lengthOf(sheets, 1); axe.testUtils.assertStylesheet( @@ -443,7 +443,7 @@ describe('preload cssom integration test', function() { }); it('returns styles defined using