Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(preload): cssom assets #958

Merged
merged 60 commits into from
Aug 8, 2018
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
321f4f1
feat(rule): css-orientation-lock raw structure
jeeyyy Jun 5, 2018
33205ec
feat: adding preload config object.
jeeyyy Jun 6, 2018
5e4f463
fix: merge > from develop
jeeyyy Jun 6, 2018
a3ea7c1
chore: merge from > develop
jeeyyy Jun 7, 2018
7119f2d
fix: wip new rule - css orientation
jeeyyy Jun 7, 2018
3782006
chore: merge from > develop
jeeyyy Jun 7, 2018
508c795
fix: wip - cssom work
jeeyyy Jun 7, 2018
b78ac30
style: update eslint to allow spread operator for object spreading.
jeeyyy Jun 19, 2018
4e590a5
feat: cssom preloading async
jeeyyy Jun 19, 2018
18b87e3
refactor: remove css-orientation-lock work from cssom preloading.
jeeyyy Jun 19, 2018
771bd88
refactor: comments and clean-up
jeeyyy Jun 19, 2018
eac06ac
feat: audit run queue to await cssom fetching.
jeeyyy Jun 20, 2018
cad3bb6
refactor: change promise to q implementation.
jeeyyy Jun 20, 2018
d9ab58f
refactor: q plumbing from preloading to audit run.
jeeyyy Jun 20, 2018
858710f
feat: refactor data marshalling of preloadedAssets.
jeeyyy Jun 20, 2018
678da39
style: fix linting errors.
jeeyyy Jun 20, 2018
f763dec
refactor: seperate preload configuration method to utils for better c…
jeeyyy Jun 25, 2018
1b847ea
test: additional tests for cssom.
jeeyyy Jun 25, 2018
50229a0
style: formamtting updates.
jeeyyy Jun 25, 2018
8426fab
style: update tests to not have any es6 keywords
jeeyyy Jun 25, 2018
46ae47c
fix: valid-lang integration tests.
jeeyyy Jun 25, 2018
b03981b
docs: add preload configuration to api documentation
jeeyyy Jun 25, 2018
0ba0ef9
docs: fix markdown lint.
jeeyyy Jun 25, 2018
ba00d6b
refactor: lint, test & docs updates.
jeeyyy Jun 26, 2018
4fa9d6a
fix: merge from develop
jeeyyy Jul 1, 2018
2074cc9
docs: update api documentation for preload configuration
jeeyyy Jul 1, 2018
9f57cdb
chore: remove merge redundant files
jeeyyy Jul 1, 2018
f18db7f
refactor: revert preload changes to run/ audit/ checks
jeeyyy Jul 2, 2018
1434751
fix: preload changes based on review
jeeyyy Jul 2, 2018
b64309c
refactor: revert formatting changes
jeeyyy Jul 2, 2018
5ea88d5
fix: update shadown dom test to not pollute fixture
jeeyyy Jul 2, 2018
28bdcdd
fix: markdown lint
jeeyyy Jul 2, 2018
87820da
Merge branch 'develop' into preload-cssom
jeeyyy Jul 2, 2018
ae59586
fix: refactor based on review
jeeyyy Jul 4, 2018
15d71f1
fix: merge from origin
jeeyyy Jul 4, 2018
3a10d63
Merge branch 'develop' into preload-cssom
jeeyyy Jul 12, 2018
222f05b
fix: implement axios against xhrQ
jeeyyy Jul 12, 2018
7f061ae
fix: refactor based on comments/ review
jeeyyy Jul 12, 2018
0fca2d1
refactor: try catch block for stylesheet cssRules
jeeyyy Jul 12, 2018
2155df8
refactor: changes based on code review
jeeyyy Jul 17, 2018
0923c36
style: revert formatting changes
jeeyyy Jul 17, 2018
29e3c61
docs: update documentation for preload configuration
jeeyyy Jul 17, 2018
9ecba9e
test: add integration tests for preload-cssom
jeeyyy Jul 17, 2018
0b9d62d
docs: fix markdown lint
jeeyyy Jul 17, 2018
f06701b
Merge branch 'develop' into preload-cssom
jeeyyy Jul 17, 2018
20e6f7d
fix: code review based refactor
jeeyyy Jul 30, 2018
06f6dfc
Merge branch 'develop' into preload-cssom
jeeyyy Jul 30, 2018
9e28b8a
Merge branch 'develop' into preload-cssom
jeeyyy Jul 30, 2018
483bcd8
test: update tests based on code review
jeeyyy Jul 30, 2018
a3ecee7
Merge branch 'develop' into preload-cssom
jeeyyy Jul 30, 2018
3436cc0
fix: updates based on review.
jeeyyy Jul 31, 2018
12008e7
Merge branch 'develop' into preload-cssom
jeeyyy Jul 31, 2018
8da721e
fix: shadowDOM support and review updates
jeeyyy Jul 31, 2018
30a0819
Merge branch 'develop' into preload-cssom
jeeyyy Jul 31, 2018
027b3a1
chore: merge from develop
jeeyyy Jul 31, 2018
567ef3c
fix: updates based on review
jeeyyy Aug 6, 2018
b5c3f4f
Merge branch 'develop' into preload-cssom
jeeyyy Aug 6, 2018
a8c74ad
Merge branch 'develop' into preload-cssom
jeeyyy Aug 8, 2018
63cc5b4
fix: update preload function and tests
jeeyyy Aug 8, 2018
247a7cb
Merge branch 'develop' into preload-cssom
jeeyyy Aug 8, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ The options parameter is flexible way to configure how `axe.run` operates. The d
Additionally, there are a number or properties that allow configuration of different options:

| Property | Default | Description |
|-----------------|:-------:|:----------------------------:|
|-----------------|:-------|:----------------------------|
| `runOnly` | n/a | Limit which rules are executed, based on names or tags
| `rules` | n/a | Allow customizing a rule's properties (including { enable: false })
| `reporter` | `v1` | Which reporter to use (see [Configuration](#api-name-axeconfigure))
Expand All @@ -339,6 +339,7 @@ Additionally, there are a number or properties that allow configuration of diffe
| `elementRef` | `false` | Return element references in addition to the target
| `restoreScroll` | `false` | Scrolls elements back to before axe started
| `frameWaitTime` | `60000` | How long (in milliseconds) axe waits for a response from embedded frames before timing out
| `preload` | `false` | Any additional assets (eg: cssom) to preload before running rules. [See here for configuration details](#preload-configuration-details)

###### Options Parameter Examples

Expand Down Expand Up @@ -460,6 +461,28 @@ Additionally, there are a number or properties that allow configuration of diffe
```
This example will process all of the "violations", "incomplete", and "inapplicable" result types. Since "passes" was not specified, it will only process the first pass for each rule, if one exists. As a result, the results object's `passes` array will have a length of either `0` or `1`. On a series of extremely large pages, this would improve performance considerably.

###### <a id='preload-configuration-details'></a> Preload Configuration in Options Parameter

The preload attribute in options parameter accepts a `boolean` or an `object` where an array of assets can be specified.

1. Specifying a `boolean`

```js
preload: true
```

2. Specifying an `object`
```js
preload: { assets: ['cssom'], timeout: 50000 }
```
The `assets` attribute expects an array of preload(able) constraints to be fetched. The current set of values supported for `assets` is listed in the following table:

| Asset Type | Description |
|:-----------|:------------|
| `cssom` | This asset type preloads all CSS Stylesheets rulesets specified in the page. The stylessheets can be an external cross-domain resource, a relative stylesheet or an inline style with in the head tag of the document. If the stylesheet is an external cross-domain a network request is made. An object representing the CSS Rules from each stylesheet is made available to the checks evaluate function as `preloadedAssets` at run-time |

The `timeout` attribute in the object configuration is `optional` and has a fallback default value (10000ms). The `timeout` is essential for any network dependent assets that are preloaded, where-in if a given request takes longer than the specified/ default value, the operation is aborted.

##### Callback Parameter

The callback parameter is a function that will be called when the asynchronous `axe.run` function completes. The callback function is passed two parameters. The first parameter will be an error thrown inside of aXe if axe.run could not complete. If axe completed correctly the first parameter will be null, and the second parameter will be the results object.
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/aria/aria-hidden-body.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
"fail": "aria-hidden=true should not be present on the document body"
}
}
}
}
4 changes: 3 additions & 1 deletion lib/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
results: [],
resultGroups: [],
resultGroupMap: {},
impact: Object.freeze(['minor', 'moderate', 'serious', 'critical'])
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
};

definitions.forEach(function(definition) {
Expand Down
108 changes: 108 additions & 0 deletions lib/core/utils/preload-cssom.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/**
* Returns a then(able) queue of CSSStyleSheet(s)
* @param {Object} ownerDocument document object to be inspected for stylesheets
* @param {number} timeout on network request for stylesheet that need to be externally fetched
* @param {Function} getSheetFromTextFn a utility function to generate a style sheet from text
* @return {Object} queue
* @private
*/
function loadCssom(ownerDocument, timeout, getSheetFromTextFn) {
const q = axe.utils.queue();

Array.from(ownerDocument.styleSheets).forEach(sheet => {
if (sheet.disabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add && sheet.cssRules.length <= 0, rather than add an if statement in the try/catch block. Currently the code looks like it's missing an "else".

Copy link
Contributor Author

@jeeyyy jeeyyy Jul 31, 2018

Choose a reason for hiding this comment

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

We cannot move .cssRules to here, as we need the catch to trigger for external stylesheets. Trying to read a .cssRules on external resource throws a SecurityError, which flows into the catch block. This is documented below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although I have refactored slightly

return;
}

try {
if (sheet.cssRules) {
// accessing .cssRules throws for external (cross-domain) sheets, which is handled in the catch
sheet.isExternal = false; // isExternal is a flag to ascertain if a network request is necessary for cssom building
q.defer(resolve => resolve(sheet));
}
} catch (e) {
// external sheet -> make an xhr and q the response
q.defer((resolve, reject) => {
axe.imports
.axios({
method: 'get',
url: sheet.href,
timeout
})
.then(({ data }) => {
const sheet = getSheetFromTextFn(data, true); //second argument acts as > isExternal - true
Copy link
Contributor

Choose a reason for hiding this comment

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

That second argument is useful. It's always external. More importantly, isExternal shouldn't be set in getSheetFromTextFn at all. It belongs in the same function that you set isExternal = false (this one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved!

resolve(sheet);
})
.catch(reject);
});
}
});

return q;
}

/**
* Returns an array of documents with a given root node/ tree
* @param {Object} treeRoot - the DOM tree to be inspected
* @return {Array} documents
* @private
*/
function getDocumentsFromTreeRoot(treeRoot) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make this available on axe.utils?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leaving it here for now, can migrate if other cssom rules need this, I don't think so, because preload should be the only place to construct this data.

let ids = [];
const documents = axe.utils
.querySelectorAllFilter(treeRoot, '*', node => {
if (ids.includes(node.shadowId)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

since shadowId can be undefined, might be safer if we test that explicitly, so node.shadowId && ids.includes(.... We've been burned by bad polyfills before.

Copy link
Contributor Author

@jeeyyy jeeyyy Jul 17, 2018

Choose a reason for hiding this comment

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

I understand the thinking here, but node.shadowId is undefined in many cases, and this returns false correctly as expected. Adding a checked if null or undefined, is not essential here in my opinion. Can chat about this if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine, but you owe me a beer when we get this reported. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is cool by me 🍺

return false;
}
ids.push(node.shadowId);
return true;
})
.map(node => {
return node.actualNode.ownerDocument;
});
return documents;
}

/**
* @method preloadCssom
* @memberof axe.utils
* @instance
* @param {Object} object argument which is a composite object, with attributes asset, timeout, treeRoot(optional), resolve & reject
* asset - type of asset being loaded, in this case cssom
* timeout - timeout for any network calls made
* treeRoot - the DOM tree to be inspected
* resolve/ reject - promise chainable methods
* @return {Object}
*/
function preloadCssom({ timeout, treeRoot = axe._tree[0] }) {
const documents = getDocumentsFromTreeRoot(treeRoot);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're not doing shadow DOM in this PR, than we need to replace this expression. Otherwise we're just committing buggy shadow DOM code to develop, rather than not having it supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have just solved for shadowDOM, and it would make sense to fold all of that into the same PR. So yes, shadowDOM is in the same PR.

const q = axe.utils.queue();

if (!documents.length) {
return q;
}

const dynamicDoc = document.implementation.createHTMLDocument();
function getSheetFromTextFn(cssText, isExternal) {
// create style node with css text
const style = dynamicDoc.createElement('style');
style.type = 'text/css';
style.appendChild(dynamicDoc.createTextNode(cssText));
// added style to temporary document
dynamicDoc.head.appendChild(style);
// add attribute to differentiate if it is a fetched sheet
style.sheet.isExternal = isExternal;
return style.sheet;
}

documents.forEach(doc => {
q.defer((resolve, reject) => {
loadCssom(doc, timeout, getSheetFromTextFn)
.then(resolve)
.catch(reject);
});
});

return q;
}
axe.utils.preloadCssom = preloadCssom;
116 changes: 116 additions & 0 deletions lib/core/utils/preload.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/**
* Validated the preload object
* @param {Object | boolean} preload configuration object or boolean passed via the options parameter to axe.run
* @return {boolean}
* @private
*/
function isValidPreloadObject(preload) {
if (typeof preload !== 'object') {
return preload;
Copy link
Contributor

Choose a reason for hiding this comment

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

preload=false shouldn't return false in isValidPreloadObject. This is misleading.

}
return (
typeof preload === 'object' &&
preload.hasOwnProperty('assets') &&
Copy link
Member

Choose a reason for hiding this comment

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

Can remove the .hasOwnProperty check, as Array.isArray(undefined) returns false.

Array.isArray(preload.assets)
);
}

/**
* Returns a boolean which decides if preload is configured
* @param {Object} options run configuration options (or defaults) passed via axe.run
* @return {boolean}
*/
axe.utils.shouldPreload = function shouldPreload(options) {
Copy link
Member

Choose a reason for hiding this comment

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

Could simplify this:

axe.utils.shouldPreload = options => options && isValidPreloadObject(options.preload)

Copy link
Member

Choose a reason for hiding this comment

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

Why was my comment marked as resolved? You didn't change anything or respond.....?

Copy link
Contributor Author

@jeeyyy jeeyyy Aug 8, 2018

Choose a reason for hiding this comment

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

Simply because, I do not want to check if preload is a valid object until I am sure that it is an object. Hence a boolean check and an undefined check earlier.

Preload can take 3 values: true, false or { assets: ['assetKey'] } as defined in the docs.

Doing this options && isValidPreloadObject(options.preload) ignores and overrides the boolean values due to &&.

Scenarios (with your suggestion):

Preload Value options isValidPreloadObject output expected
options: undefined or null false false false false
options: { preload : true } true false false true (BOOM!)
options: { preload : false } true false false false
options: { preload : { assets: ['cssom'] } } true true true true

I am sure there is a clever way to shorten things further, but this keeps it easy to follow.
Prefer to keep this for that reason.

Appreciate the comments.

Copy link
Member

Choose a reason for hiding this comment

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

Discussed on Slack. Disregard my comment. This is good as-is!

return isValidPreloadObject(options.preload);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest we run this first and throw if its invalid, rather than do it in getPreloadConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what we are doing.

const shouldPreload = axe.utils.shouldPreload(options);
if (!shouldPreload) {
	return q;
}

const preloadConfig = axe.utils.getPreloadConfig(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}
*/
axe.utils.getPreloadConfig = function getPreloadConfig(options) {
// default fallback configuration
const config = {
assets: axe.constants.preloadAssets,
timeout: axe.constants.preloadAssetsTimeout
};

// if type is boolean
if (typeof options.preload === 'boolean') {
return config;
}

// if type is object - ensure an array of assets to load is specified
if (!isValidPreloadObject(options.preload)) {
throw new Error(
Copy link
Contributor

Choose a reason for hiding this comment

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

This code will never run because if it isn't, your shouldPreload function causes utils.preload to exit early.

'No assets configured for preload in aXe run configuration'
);
}

// check if requested assets to preload are valid items
const areRequestedAssetsValid = options.preload.assets.every(a =>
axe.constants.preloadAssets.includes(a.toLowerCase())
);

if (!areRequestedAssetsValid) {
throw new Error(
`Requested assets, not supported by aXe.` +
Copy link
Member

Choose a reason for hiding this comment

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

IIRC we're not capitalizing the "x" anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid i18n concerns removed aXe.

`Supported assets are: ${axe.constants.preloadAssets.join(', ')}.`
);
}

// unique assets to load, in case user had requested same asset type many times.
config.assets = axe.utils.uniqueArray(
options.preload.assets.map(a => a.toLowerCase()),
[]
);

if (
options.preload.timeout &&
typeof options.preload.timeout === 'number' &&
!Number.isNaN(options.preload.timeout)
) {
config.timeout = options.preload.timeout;
}
return config;
};

/**
* 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.
* @param {Object} options run configuration options (or defaults) passed via axe.run
* @return {Object} queue
*/
axe.utils.preload = function preload(options) {
const preloadFunctionsMap = {
cssom: axe.utils.preloadCssom
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest you expose this so that it can be extended and you can test that this function is actually called when you call preload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just an object map of various preload options/ types and the respective functions to execute.

The tests for cssom, already test the execution of the respective function & any further additions to this hash, will have their own tests. So do not see the need to abstract any more.

Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment about testing that preloadCssom is called and returned correctly. We'll need to do something to prove that this works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignored as per discussion.

};

const q = axe.utils.queue();

const shouldPreload = axe.utils.shouldPreload(options);
if (!shouldPreload) {
return q;
}

const preloadConfig = axe.utils.getPreloadConfig(options);

preloadConfig.assets.forEach(asset => {
q.defer((resolve, reject) => {
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this top-level .defer()? It looks like this could be rewritten as:

prelaodedConfig.assets.forEach(asset => {
  preloadFunctionsMap[asset]({ asset, timeout: preloadConfig.timeout })
    .then(results => results[0])
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prefer to keep the top level q, then all the preloaded asset types lives under one q, when is then(ed) in the run setup.

Copy link
Contributor

@WilcoFiers WilcoFiers Jul 13, 2018

Choose a reason for hiding this comment

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

The queue isn't chainable, so that wouldn't work. It only gives you one "then".

Copy link
Member

Choose a reason for hiding this comment

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

Oh, weird. I figured it worked the same way as a native Promise.

Copy link
Contributor Author

@jeeyyy jeeyyy Jul 17, 2018

Choose a reason for hiding this comment

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

Still keeping this one, as is. Appreciate the comments.

preloadFunctionsMap[asset]({
asset,
timeout: preloadConfig.timeout
})
.then(results => {
const sheets = results[0];
resolve({
[asset]: sheets
});
})
.catch(reject);
});
});

return q;
};
2 changes: 1 addition & 1 deletion lib/rules/aria-hidden-body.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@
"aria-hidden-body"
],
"none": []
}
}
90 changes: 90 additions & 0 deletions test/core/utils/preload-cssom.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
describe('axe.utils.preloadCssom unit tests', function() {
'use strict';

var args;

function addStyleToHead() {
var css = 'html {font-size: inherit;}';
var head = document.head || document.getElementsByTagName('head')[0];
var style = document.createElement('style');
style.id = 'preloadCssomTestHeadSheet';
style.type = 'text/css';
style.appendChild(document.createTextNode(css));
head.appendChild(style);
}

function removeStyleFromHead() {
var s = document.getElementById('preloadCssomTestHeadSheet');
if (s) {
s.parentNode.removeChild(s);
}
}

beforeEach(function() {
addStyleToHead();
args = {
asset: 'cssom',
timeout: 10000,
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 queue', function() {
var actual = axe.utils.preloadCssom(args);
assert.isObject(actual);
assert.containsAllKeys(actual, ['then', 'defer', 'catch']);
});

it('should ensure result of cssom is an array of sheets', function(done) {
var actual = axe.utils.preloadCssom(args);
actual
.then(function(results) {
assert.lengthOf(results[0], 2); // returned from queue, hence the index look up
done();
})
.catch(function(error) {
done(error);
});
});

it('should fail if number of sheets returned does not match stylesheets defined in document', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't what's being tested. It's not true either, this should return styles in shadow DOM too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorted!

var actual = axe.utils.preloadCssom(args);
actual
.then(function(results) {
assert.isFalse(results[0].length <= 1); // returned from queue, hence the index look up
done();
})
.catch(function(error) {
done(error);
});
});

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you loop over an array with 1 item?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not one item, there could be multiple sheets for the document & in this case, there is the mocha.css & anything added via before or to the fixture.

assert.isDefined(s);
assert.property(s, 'cssRules');
});
done();
})
.catch(function(error) {
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/**.*
*/
});
Loading