From 369f7987c548cd514c3a2c990d8057c1d1351c88 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Tue, 30 May 2017 16:24:55 +0200 Subject: [PATCH 01/41] getSelector v2 alpha --- lib/core/utils/get-selector.js | 285 ++++++++++++++++++++++----------- 1 file changed, 195 insertions(+), 90 deletions(-) diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index 7d339b6ff1..0a79dadbb2 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -1,104 +1,209 @@ -/** - * Gets the index of element siblings that have the same nodeName - * Intended for use with the CSS psuedo-class `:nth-of-type()` and xpath node index - * @param {HTMLElement} element The element to test - * @return {Number} The number of preceeding siblings with the same nodeName - * @private - */ -function nthOfType(element) { - 'use strict'; - - var index = 1, - type = element.nodeName.toUpperCase(); +const escapeSelector = axe.utils.escapeSelector; + +function isUncommonClassName (className) { + return ![ + 'focus', 'hover', + 'hidden', 'visible', + 'dirty', 'touched', 'valid', 'disable', + 'disable', 'enable', 'active', 'col-' + ].find(str => className.includes(str)); +} - /*jshint boss:true */ - element = element.previousElementSibling; - while (element) { - if (element.nodeName.toUpperCase() === type) { - index++; - } - element = element.previousElementSibling; - } +function getDistinctClassList (elm) { + if (!elm.classList || elm.classList.length === 0) { + return []; + } + + return Array.from( + elm.parentNode && elm.parentNode.children || [] + ).reduce((classList, childElm) => { + if (elm === childElm) { + return classList; + } else { + return classList.filter(classItem => { + return !childElm.classList.contains(classItem); + }); + } + }, Array.from(elm.classList).filter(isUncommonClassName)); +} - return index; +const commonNodes = [ + 'div', 'span', 'p', + 'b', 'i', 'u', 'strong', 'em', + 'h2', 'h3' +]; + +function getNthChildString (elm, selector) { + // TODO: Use the current polyfill + const hasMatchingSiblings = Array.from(elm.parentNode.children).find( + sibling => sibling !== elm && sibling.matches(selector) + ); + if (hasMatchingSiblings) { + const nthChild = 1 + Array.from(elm.parentNode.children).indexOf(elm); + return ':nth-child(' + nthChild + ')'; + } else { + return ''; + } } +const createSelector = { + // Get ID properties + getElmId (elm) { + if (elm.id && !elm.id.match(/player_uid_/)) { + return '#' + escapeSelector(elm.id); + } + }, + // Get custom element name + getCustomElm (elm, { isCustomElm, nodeName }) { + if (isCustomElm) { + return nodeName; + } + }, + + // Get ARIA role + getElmRoleProp (elm) { + if (elm.hasAttribute('role')) { + return '[role="' + escapeSelector(elm.getAttribute('role')) +'"]'; + } + }, + // Get uncommon node names + getUncommonElm (elm, { isCommonElm, isCustomElm, nodeName }) { + if (!isCommonElm && !isCustomElm) { + // Add [type] if nodeName is an input element + if (nodeName === 'input' && elm.hasAttribute('type')) { + nodeName += '[type=' + elm.type + ']'; + } + return nodeName; + } + }, + // Has a name property, but no ID (Think input fields) + getElmNameProp (elm) { + if (!elm.id && elm.name) { + return '[name="' + escapeSelector(elm.name) + '"]'; + } + }, + // Get any distinct classes (as long as there aren't more then 3 of them) + getDistinctClass (elm, { distinctClassList }) { + if (distinctClassList.length > 0 && distinctClassList.length < 3) { + return '.' + distinctClassList.map(escapeSelector).join('.'); + } + }, + // Get a selector that uses src/href props + getFileRefProp (elm) { + if (elm.id) { + return; + } + // TODO: find a better way to either show a brief name, or exit empty + let attr; + if (elm.hasAttribute('href')) { + attr = 'href'; + } else if (elm.hasAttribute('src')) { + attr = 'src'; + } + + if (attr) { + const filePath = elm.getAttribute(attr).replace(/\/$/, '').split(/\//g); + if (filePath) { + return '[' + attr + '$="' + encodeURI(filePath.pop()) + '"]'; + } + } + }, + // Get common node names + getCommonName (elm, { nodeName, isCommonElm }) { + if (isCommonElm) { + return nodeName; + } + }, + // Get common classes + getCommonClass (elm, { isCommonElm, classList, distinctClassList }) { + if ( + isCommonElm && classList.length > 0 && + (distinctClassList.length === 0 || distinctClassList.length >= 3) + ) { + return '.' + classList.map(escapeSelector).join('.'); + } + } +}; + /** - * Checks if an element has siblings with the same selector - * @param {HTMLElement} node The element to test - * @param {String} selector The CSS selector to test - * @return {Boolean} Whether any of element's siblings matches selector - * @private + * Get an array of features (as CSS selectors) that describe an element + * + * By going down the list of most to least prominent element features, + * we attempt to find those features that a dev is most likely to + * recognize the element by (IDs, aria roles, custom element names, etc.) */ -function siblingsHaveSameSelector(node, selector) { - 'use strict'; - - var index, sibling, - siblings = node.parentNode.children; - - if (!siblings) { - return false; - } - - var length = siblings.length; - - for (index = 0; index < length; index++) { - sibling = siblings[index]; - if (sibling !== node && axe.utils.matchesSelector(sibling, selector)) { - return true; - } - } - return false; +function getElmFeatures (elm, featureCount) { + const nodeName = elm.nodeName.toLowerCase(); + const classList = Array.from(elm.classList) || []; + // Collect some props we need to build the selector + const props = { + nodeName, + classList, + isCustomElm: nodeName.includes('-'), + isCommonElm: commonNodes.includes(nodeName), + distinctClassList: getDistinctClassList(elm) + }; + + return [ + // go through feature selectors in order of priority + createSelector.getElmId, + createSelector.getCustomElm, + createSelector.getElmRoleProp, + createSelector.getUncommonElm, + createSelector.getElmNameProp, + createSelector.getDistinctClass, + createSelector.getCommonName, + createSelector.getCommonClass + ].reduce((features, func) => { + // As long as we haven't met our count, keep looking for features + if (features.length === featureCount) { + return features; + } + + const feature = func(elm, props); + if (feature) { + if (!feature[0].match(/[a-z]/)) { + features.push(feature); + } else { + features.unshift(feature); + } + } + return features; + }, []); } - /** * Gets a unique CSS selector * @param {HTMLElement} node The element to get the selector for * @return {String} Unique CSS selector for the node */ -axe.utils.getSelector = function getSelector(node) { - //jshint maxstatements: 21 - 'use strict'; - - function escape(p) { - return axe.utils.escapeSelector(p); - } - - var parts = [], part; - - while (node.parentNode) { - part = ''; - - if (node.id && document.querySelectorAll('#' + axe.utils.escapeSelector(node.id)).length === 1) { - parts.unshift('#' + axe.utils.escapeSelector(node.id)); - break; - } - - if (node.className && typeof node.className === 'string') { - part = '.' + node.className.trim().split(/\s+/).map(escape).join('.'); - if (part === '.' || siblingsHaveSameSelector(node, part)) { - part = ''; - } - } - - if (!part) { - part = axe.utils.escapeSelector(node.nodeName).toLowerCase(); - if (part === 'html' || part === 'body') { - parts.unshift(part); - break; - } - if (siblingsHaveSameSelector(node, part)) { - part += ':nth-of-type(' + nthOfType(node) + ')'; - } - - } - - parts.unshift(part); - - node = node.parentNode; - } - - return parts.join(' > '); - +axe.utils.getSelector = function createUniqueSelector (elm, options = {}) { + if (!elm) { + return ''; + } + const { + featureCount = 2, + minDepth = 1, + toRoot = false, + childSelectors = [] + } = options; + + const elmFeatures = getElmFeatures(elm, featureCount); + const selector = elmFeatures.join(''); + const nthChildPath = getNthChildString(elm, selector); + + const isUnique = options.isUnique || document.querySelectorAll(selector).length === 1; + const addParent = (toRoot || minDepth > 0 || !isUnique); + const selectorParts = [selector + nthChildPath, ...childSelectors]; + + if (elm.parentElement && addParent) { + return createUniqueSelector(elm.parentNode, { + toRoot, isUnique, + childSelectors: selectorParts, + featureCount: 1, + minDepth: minDepth -1 + }); + } else { + return selectorParts.join(' > '); + } }; From 838740f49bb4bb7bce0f893573cc09f7c558ba7d Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Thu, 1 Jun 2017 13:47:02 +0200 Subject: [PATCH 02/41] have getSelector v2 pass (most) previous tests --- lib/core/utils/get-selector.js | 57 +++++++++++-------- lib/core/utils/pollyfills.js | 15 ++++- test/core/utils/get-selector.js | 46 +++------------ .../rules/duplicate-id/duplicate-id.json | 2 +- 4 files changed, 57 insertions(+), 63 deletions(-) diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index 0a79dadbb2..3d3617779a 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -14,9 +14,8 @@ function getDistinctClassList (elm) { return []; } - return Array.from( - elm.parentNode && elm.parentNode.children || [] - ).reduce((classList, childElm) => { + const siblings = elm.parentNode && Array.from(elm.parentNode.children || '') || []; + return siblings.reduce((classList, childElm) => { if (elm === childElm) { return classList; } else { @@ -34,12 +33,14 @@ const commonNodes = [ ]; function getNthChildString (elm, selector) { + const siblings = elm.parentNode && Array.from(elm.parentNode.children || '') || []; // TODO: Use the current polyfill - const hasMatchingSiblings = Array.from(elm.parentNode.children).find( - sibling => sibling !== elm && sibling.matches(selector) - ); + const hasMatchingSiblings = siblings.find(sibling => ( + sibling !== elm && + axe.utils.matchesSelector(sibling, selector) + )); if (hasMatchingSiblings) { - const nthChild = 1 + Array.from(elm.parentNode.children).indexOf(elm); + const nthChild = 1 + siblings.indexOf(elm); return ':nth-child(' + nthChild + ')'; } else { return ''; @@ -48,9 +49,18 @@ function getNthChildString (elm, selector) { const createSelector = { // Get ID properties - getElmId (elm) { - if (elm.id && !elm.id.match(/player_uid_/)) { - return '#' + escapeSelector(elm.id); + getElmId (elm) { + if (!elm.id) { + return; + } + const id = '#' + escapeSelector(elm.id || ''); + if ( + // Don't include youtube's uid values, they change on reload + !id.match(/player_uid_/) && + // Don't include IDs that occur more then once on the page + document.querySelectorAll(id).length === 1 + ) { + return id; } }, // Get custom element name @@ -69,6 +79,7 @@ const createSelector = { // Get uncommon node names getUncommonElm (elm, { isCommonElm, isCustomElm, nodeName }) { if (!isCommonElm && !isCustomElm) { + nodeName = escapeSelector(nodeName); // Add [type] if nodeName is an input element if (nodeName === 'input' && elm.hasAttribute('type')) { nodeName += '[type=' + elm.type + ']'; @@ -113,15 +124,6 @@ const createSelector = { if (isCommonElm) { return nodeName; } - }, - // Get common classes - getCommonClass (elm, { isCommonElm, classList, distinctClassList }) { - if ( - isCommonElm && classList.length > 0 && - (distinctClassList.length === 0 || distinctClassList.length >= 3) - ) { - return '.' + classList.map(escapeSelector).join('.'); - } } }; @@ -146,14 +148,12 @@ function getElmFeatures (elm, featureCount) { return [ // go through feature selectors in order of priority - createSelector.getElmId, createSelector.getCustomElm, createSelector.getElmRoleProp, createSelector.getUncommonElm, createSelector.getElmNameProp, createSelector.getDistinctClass, - createSelector.getCommonName, - createSelector.getCommonClass + createSelector.getCommonName ].reduce((features, func) => { // As long as we haven't met our count, keep looking for features if (features.length === featureCount) { @@ -178,6 +178,7 @@ function getElmFeatures (elm, featureCount) { * @return {String} Unique CSS selector for the node */ axe.utils.getSelector = function createUniqueSelector (elm, options = {}) { + //jshint maxstatements: 17 if (!elm) { return ''; } @@ -188,11 +189,21 @@ axe.utils.getSelector = function createUniqueSelector (elm, options = {}) { childSelectors = [] } = options; + const idSelector = createSelector.getElmId(elm); + if (idSelector) { + return [idSelector, ...childSelectors].join(' > '); + } + const elmFeatures = getElmFeatures(elm, featureCount); - const selector = elmFeatures.join(''); + let selector = elmFeatures.join(''); const nthChildPath = getNthChildString(elm, selector); const isUnique = options.isUnique || document.querySelectorAll(selector).length === 1; + // For the odd case that document doesn't have a unique selector + if (!isUnique && elm === document.documentElement) { + selector += ':root'; + } + const addParent = (toRoot || minDepth > 0 || !isUnique); const selectorParts = [selector + nthChildPath, ...childSelectors]; diff --git a/lib/core/utils/pollyfills.js b/lib/core/utils/pollyfills.js index a1a5f9d393..9f63aa8f9e 100644 --- a/lib/core/utils/pollyfills.js +++ b/lib/core/utils/pollyfills.js @@ -241,4 +241,17 @@ if (!Array.from) { return A; }; }()); -} \ No newline at end of file +} + +if (!String.prototype.includes) { + String.prototype.includes = function(search, start) { + if (typeof start !== 'number') { + start = 0; + } + if (start + search.length > this.length) { + return false; + } else { + return this.indexOf(search, start) !== -1; + } + }; +} diff --git a/test/core/utils/get-selector.js b/test/core/utils/get-selector.js index 8a0676bff6..61d5e271e8 100644 --- a/test/core/utils/get-selector.js +++ b/test/core/utils/get-selector.js @@ -99,12 +99,11 @@ describe('axe.utils.getSelector', function () { var sel = axe.utils.getSelector(node); - assert.equal(sel, '#fixture > div:nth-of-type(2)'); + assert.equal(sel, '#fixture > div:nth-child(2)'); var result = document.querySelectorAll(sel); assert.lengthOf(result, 1); assert.equal(result[0], node); - }); it('should use classes if available and unique', function () { @@ -118,7 +117,7 @@ describe('axe.utils.getSelector', function () { var sel = axe.utils.getSelector(node); - assert.equal(sel, '#fixture > .dogs.cats'); + assert.equal(sel, '#fixture > div.dogs.cats'); var result = document.querySelectorAll(sel); assert.lengthOf(result, 1); @@ -137,7 +136,7 @@ describe('axe.utils.getSelector', function () { var sel = axe.utils.getSelector(node); - assert.equal(sel, '#fixture > div:nth-of-type(2)'); + assert.equal(sel, '#fixture > div:nth-child(2)'); var result = document.querySelectorAll(sel); assert.lengthOf(result, 1); @@ -145,39 +144,6 @@ describe('axe.utils.getSelector', function () { }); - it('should properly calculate nth-of-type when siblings are of different type', function () { - var node, target; - node = document.createElement('span'); - fixture.appendChild(node); - - node = document.createElement('span'); - fixture.appendChild(node); - - node = document.createElement('div'); - fixture.appendChild(node); - - node = document.createElement('div'); - target = node; - fixture.appendChild(node); - - node = document.createElement('div'); - fixture.appendChild(node); - - node = document.createElement('span'); - fixture.appendChild(node); - - - - var sel = axe.utils.getSelector(target); - - assert.equal(sel, '#fixture > div:nth-of-type(2)'); - - var result = document.querySelectorAll(sel); - assert.lengthOf(result, 1); - assert.equal(result[0], target); - - }); - it('should work on the documentElement', function () { var sel = axe.utils.getSelector(document.documentElement); var result = document.querySelectorAll(sel); @@ -214,8 +180,12 @@ describe('axe.utils.getSelector', function () { it('shouldn\'t fail if the node\'s parentNode doesnt have children, somehow (Firefox bug)', function () { var sel = axe.utils.getSelector({ nodeName: 'a', + classList: [], + hasAttribute: function () { return false; }, parentNode: { - nodeName: 'b' + nodeName: 'b', + hasAttribute: function () { return false; }, + classList: [] } }); assert.equal(sel, 'a'); diff --git a/test/integration/rules/duplicate-id/duplicate-id.json b/test/integration/rules/duplicate-id/duplicate-id.json index f11c770ebb..baa508d0e1 100644 --- a/test/integration/rules/duplicate-id/duplicate-id.json +++ b/test/integration/rules/duplicate-id/duplicate-id.json @@ -1,6 +1,6 @@ { "description": "duplicate-id test", "rule": "duplicate-id", - "violations": [["#fixture > p:nth-of-type(1)"]], + "violations": [["#fixture > p:nth-child(1)"]], "passes": [["#fixture"], ["#single"]] } From 83e91629ef891fcb2f0c0f3128cd224d4b1c7083 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Thu, 1 Jun 2017 15:20:44 +0200 Subject: [PATCH 03/41] test: get-selector v2 tests --- lib/core/utils/get-selector.js | 2 +- test/core/utils/get-selector.js | 77 +++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index 3d3617779a..398ae00a5c 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -82,7 +82,7 @@ const createSelector = { nodeName = escapeSelector(nodeName); // Add [type] if nodeName is an input element if (nodeName === 'input' && elm.hasAttribute('type')) { - nodeName += '[type=' + elm.type + ']'; + nodeName += '[type="' + elm.type + '"]'; } return nodeName; } diff --git a/test/core/utils/get-selector.js b/test/core/utils/get-selector.js index 61d5e271e8..6beec682aa 100644 --- a/test/core/utils/get-selector.js +++ b/test/core/utils/get-selector.js @@ -190,4 +190,81 @@ describe('axe.utils.getSelector', function () { }); assert.equal(sel, 'a'); }); + + it('should use role attributes', function () { + var node = document.createElement('div'); + node.setAttribute('role', 'menuitem'); + fixture.appendChild(node); + + assert.equal( + axe.utils.getSelector(node), + '#fixture > div[role="menuitem"]' + ); + }); + + it('should give use two features on the first element', function () { + var node = document.createElement('div'); + node.setAttribute('role', 'menuitem'); + fixture.appendChild(node); + + assert.equal( + axe.utils.getSelector(node), + '#fixture > div[role="menuitem"]' + ); + + node.className = 'dqpl-btn-primary'; + assert.equal( + axe.utils.getSelector(node), + '#fixture > [role="menuitem"].dqpl-btn-primary' + ); + }); + + it('should give use one features on the subsequent elements', function () { + var span = document.createElement('span'); + var node = document.createElement('div'); + node.setAttribute('role', 'menuitem'); + span.className = 'expand-icon'; + node.appendChild(span); + fixture.appendChild(node); + + assert.equal( + axe.utils.getSelector(span), + '[role="menuitem"] > span.expand-icon' + ); + }); + + it('should prioritize uncommon tagNames', function () { + var node = document.createElement('button'); + node.setAttribute('role', 'menuitem'); + node.className = 'dqpl-btn-primary'; + fixture.appendChild(node); + assert.equal( + axe.utils.getSelector(node), + '#fixture > button[role="menuitem"]' + ); + }); + + it('should add [type] to input elements', function () { + var node = document.createElement('input'); + node.type = 'password'; + node.className = 'dqpl-textfield'; + fixture.appendChild(node); + assert.equal( + axe.utils.getSelector(node), + '#fixture > input[type="password"].dqpl-textfield' + ); + }); + + it('should use the name property', function () { + var node = document.createElement('input'); + node.type = 'text'; + node.name = 'username'; + node.className = 'dqpl-textfield'; + fixture.appendChild(node); + assert.equal( + axe.utils.getSelector(node), + '#fixture > input[type="text"][name="username"]' + ); + }); + }); From 3b4b2f10a290955cc3f48580bc702f0005098e13 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Jun 2017 13:55:31 +0200 Subject: [PATCH 04/41] feat: Add absolutePaths option --- lib/core/base/check.js | 2 +- lib/core/base/rule.js | 2 +- lib/core/utils/check-helper.js | 4 +- lib/core/utils/collect-results-from-frames.js | 2 +- lib/core/utils/dq-element.js | 23 +++++----- lib/core/utils/get-check-option.js | 4 +- lib/core/utils/get-selector.js | 42 ++++++++++--------- lib/core/utils/merge-results.js | 10 ++--- test/core/base/check.js | 2 +- test/core/base/rule.js | 6 ++- test/core/public/run.js | 39 +++++++++++++++++ test/core/utils/check-helper.js | 12 +++--- test/core/utils/dq-element.js | 20 +++++++-- test/core/utils/get-check-option.js | 26 ++++++++++-- 14 files changed, 135 insertions(+), 59 deletions(-) diff --git a/lib/core/base/check.js b/lib/core/base/check.js index 9e94adcdf0..6289ac70c1 100644 --- a/lib/core/base/check.js +++ b/lib/core/base/check.js @@ -66,7 +66,7 @@ Check.prototype.run = function (node, options, resolve, reject) { if (enabled) { var checkResult = new CheckResult(this); - var checkHelper = axe.utils.checkHelper(checkResult, resolve, reject); + var checkHelper = axe.utils.checkHelper(checkResult, options, resolve, reject); var result; try { diff --git a/lib/core/base/rule.js b/lib/core/base/rule.js index 4c6aaf7a3c..2206a468a9 100644 --- a/lib/core/base/rule.js +++ b/lib/core/base/rule.js @@ -171,7 +171,7 @@ Rule.prototype.run = function (context, options, resolve, reject) { } }); if (hasResults) { - result.node = new axe.utils.DqElement(node); + result.node = new axe.utils.DqElement(node, options); ruleResult.nodes.push(result); } } diff --git a/lib/core/utils/check-helper.js b/lib/core/utils/check-helper.js index b9ed513db7..cac9b5d3fe 100644 --- a/lib/core/utils/check-helper.js +++ b/lib/core/utils/check-helper.js @@ -4,7 +4,7 @@ * @param {Function} callback The callback to expose when `this.async()` is called * @return {Object} Bound to `this` for a check's fn */ -axe.utils.checkHelper = function checkHelper(checkResult, resolve, reject) { +axe.utils.checkHelper = function checkHelper(checkResult, options, resolve, reject) { 'use strict'; return { @@ -26,7 +26,7 @@ axe.utils.checkHelper = function checkHelper(checkResult, resolve, reject) { relatedNodes: function (nodes) { nodes = nodes instanceof Node ? [nodes] : axe.utils.toArray(nodes); checkResult.relatedNodes = nodes.map(function (element) { - return new axe.utils.DqElement(element); + return new axe.utils.DqElement(element, options); }); } }; diff --git a/lib/core/utils/collect-results-from-frames.js b/lib/core/utils/collect-results-from-frames.js index 4a665d31da..e0e219d4ce 100644 --- a/lib/core/utils/collect-results-from-frames.js +++ b/lib/core/utils/collect-results-from-frames.js @@ -104,7 +104,7 @@ function collectResultsFromFrames(context, options, command, parameter, resolve, // Combine results from all frames and give it back q.then(function(data) { - resolve(axe.utils.mergeResults(data)); + resolve(axe.utils.mergeResults(data, options)); }).catch(reject); } diff --git a/lib/core/utils/dq-element.js b/lib/core/utils/dq-element.js index ac0518a436..e3fa051464 100644 --- a/lib/core/utils/dq-element.js +++ b/lib/core/utils/dq-element.js @@ -1,8 +1,6 @@ /*exported DqElement */ function truncate(str, maxLength) { - 'use strict'; - maxLength = maxLength || 300; if (str.length > maxLength) { @@ -14,8 +12,6 @@ function truncate(str, maxLength) { } function getSource (element) { - 'use strict'; - var source = element.outerHTML; if (!source && typeof XMLSerializer === 'function') { source = new XMLSerializer().serializeToString(element); @@ -29,12 +25,13 @@ function getSource (element) { * @param {HTMLElement} element The element to serialize * @param {Object} spec Properties to use in place of the element when instantiated on Elements from other frames */ -function DqElement(element, spec) { - 'use strict'; - +function DqElement(element, options, spec) { this._fromFrame = !!spec; this.spec = spec || {}; + if (options && options.absolutePaths) { + this._options = { toRoot: true }; + } /** * The generated HTML source code of the element @@ -56,7 +53,9 @@ DqElement.prototype = { * @return {String} */ get selector() { - return this.spec.selector || [axe.utils.getSelector(this.element)]; + return this.spec.selector || [ + axe.utils.getSelector(this.element, this._options) + ]; }, /** @@ -64,7 +63,9 @@ DqElement.prototype = { * @return {String} */ get xpath() { - return this.spec.xpath || [axe.utils.getXpath(this.element)]; + return this.spec.xpath || [ + axe.utils.getXpath(this.element) + ]; }, /** @@ -88,10 +89,10 @@ DqElement.prototype = { } }; -DqElement.fromFrame = function (node, frame) { +DqElement.fromFrame = function (node, options, frame) { node.selector.unshift(frame.selector); node.xpath.unshift(frame.xpath); - return new axe.utils.DqElement(frame.element, node); + return new axe.utils.DqElement(frame.element, options, node); }; axe.utils.DqElement = DqElement; diff --git a/lib/core/utils/get-check-option.js b/lib/core/utils/get-check-option.js index 1f471fa519..06e33f1058 100644 --- a/lib/core/utils/get-check-option.js +++ b/lib/core/utils/get-check-option.js @@ -7,7 +7,6 @@ * @return {Object} The resolved object with `options` and `enabled` keys */ axe.utils.getCheckOption = function (check, ruleID, options) { - 'use strict'; var ruleCheckOption = ((options.rules && options.rules[ruleID] || {}).checks || {})[check.id]; var checkOption = (options.checks || {})[check.id]; @@ -35,6 +34,7 @@ axe.utils.getCheckOption = function (check, ruleID, options) { return { enabled: enabled, - options: opts + options: opts, + absolutePaths: options.absolutePaths }; }; \ No newline at end of file diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index 398ae00a5c..76d9b4419a 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -34,7 +34,6 @@ const commonNodes = [ function getNthChildString (elm, selector) { const siblings = elm.parentNode && Array.from(elm.parentNode.children || '') || []; - // TODO: Use the current polyfill const hasMatchingSiblings = siblings.find(sibling => ( sibling !== elm && axe.utils.matchesSelector(sibling, selector) @@ -178,36 +177,39 @@ function getElmFeatures (elm, featureCount) { * @return {String} Unique CSS selector for the node */ axe.utils.getSelector = function createUniqueSelector (elm, options = {}) { - //jshint maxstatements: 17 + //jshint maxstatements: 19 if (!elm) { return ''; } + let selector, addParent; + let { isUnique = false } = options; + const idSelector = createSelector.getElmId(elm); const { - featureCount = 2, - minDepth = 1, - toRoot = false, - childSelectors = [] - } = options; + featureCount = 2, + minDepth = 1, + toRoot = false, + childSelectors = [] + } = options; - const idSelector = createSelector.getElmId(elm); if (idSelector) { - return [idSelector, ...childSelectors].join(' > '); - } + selector = idSelector; + isUnique = true; - const elmFeatures = getElmFeatures(elm, featureCount); - let selector = elmFeatures.join(''); - const nthChildPath = getNthChildString(elm, selector); + } else { + selector = getElmFeatures(elm, featureCount).join(''); + selector += getNthChildString(elm, selector); + isUnique = options.isUnique || document.querySelectorAll(selector).length === 1; - const isUnique = options.isUnique || document.querySelectorAll(selector).length === 1; - // For the odd case that document doesn't have a unique selector - if (!isUnique && elm === document.documentElement) { - selector += ':root'; + // For the odd case that document doesn't have a unique selector + if (!isUnique && elm === document.documentElement) { + selector += ':root'; + } + addParent = (minDepth !== 0 || !isUnique); } - const addParent = (toRoot || minDepth > 0 || !isUnique); - const selectorParts = [selector + nthChildPath, ...childSelectors]; + const selectorParts = [selector, ...childSelectors]; - if (elm.parentElement && addParent) { + if (elm.parentElement && (toRoot || addParent)) { return createUniqueSelector(elm.parentNode, { toRoot, isUnique, childSelectors: selectorParts, diff --git a/lib/core/utils/merge-results.js b/lib/core/utils/merge-results.js index 3651e6a4ed..d7bdcc4491 100644 --- a/lib/core/utils/merge-results.js +++ b/lib/core/utils/merge-results.js @@ -6,7 +6,7 @@ * @param {HTMLElement} frameElement The frame element * @param {String} frameSelector Unique CSS selector for the frame */ -function pushFrame(resultSet, frameElement, frameSelector) { +function pushFrame(resultSet, options, frameElement, frameSelector) { 'use strict'; var frameXpath = axe.utils.getXpath(frameElement); var frameSpec = { @@ -16,13 +16,13 @@ function pushFrame(resultSet, frameElement, frameSelector) { }; resultSet.forEach(function (res) { - res.node = axe.utils.DqElement.fromFrame(res.node, frameSpec); + res.node = axe.utils.DqElement.fromFrame(res.node, options, frameSpec); var checks = axe.utils.getAllChecks(res); if (checks.length) { checks.forEach(function (check) { check.relatedNodes = check.relatedNodes.map( - (node) => axe.utils.DqElement.fromFrame(node, frameSpec) + (node) => axe.utils.DqElement.fromFrame(node, options, frameSpec) ); }); } @@ -78,7 +78,7 @@ function normalizeResult(result) { * @param {Array} frameResults Array of objects including the RuleResults as `results` and frame as `frame` * @return {Array} The merged RuleResults; should only have one result per rule */ -axe.utils.mergeResults = function mergeResults(frameResults) { +axe.utils.mergeResults = function mergeResults(frameResults, options) { 'use strict'; var result = []; frameResults.forEach(function (frameResult) { @@ -89,7 +89,7 @@ axe.utils.mergeResults = function mergeResults(frameResults) { results.forEach(function (ruleResult) { if (ruleResult.nodes && frameResult.frame) { - pushFrame(ruleResult.nodes, frameResult.frameElement, frameResult.frame); + pushFrame(ruleResult.nodes, options, frameResult.frameElement, frameResult.frame); } var res = axe.utils.findBy(result, 'id', ruleResult.id); diff --git a/test/core/base/check.js b/test/core/base/check.js index 916fa181a8..2026a371a2 100644 --- a/test/core/base/check.js +++ b/test/core/base/check.js @@ -164,7 +164,7 @@ describe('Check', function () { cb = function () { return true; }, result = { monkeys: 'bananas' }; - axe.utils.checkHelper = function (checkResult, callback) { + axe.utils.checkHelper = function (checkResult, options, callback) { assert.instanceOf(checkResult, window.CheckResult); assert.equal(callback, cb); diff --git a/test/core/base/rule.js b/test/core/base/rule.js index a4e1d6c828..d47e436594 100644 --- a/test/core/base/rule.js +++ b/test/core/base/rule.js @@ -542,7 +542,11 @@ describe('Rule', function() { id: 'cats', enabled: true, after: function(results, options) { - assert.deepEqual(options, { enabled: true, options: { dogs: true }}); + assert.deepEqual(options, { + enabled: true, + options: { dogs: true }, + absolutePaths: undefined + }); success = true; return results; } diff --git a/test/core/public/run.js b/test/core/public/run.js index d3f825b050..4581a21394 100644 --- a/test/core/public/run.js +++ b/test/core/public/run.js @@ -276,6 +276,45 @@ describe('axe.run', function () { }); }); }); + + describe('option absolutePaths', function () { + + it('returns relative paths when falsy', function (done) { + axe.run('#fixture', { + absolutePaths: 0 + }, function (err, result) { + assert.deepEqual( + result.violations[0].nodes[0].target, + ['#fixture'] + ); + done(); + }); + }); + + it('returns absolute paths when truthy', function (done) { + axe.run('#fixture', { + absolutePaths: 'yes please' + }, function (err, result) { + assert.deepEqual( + result.violations[0].nodes[0].target, + ['html > body > #fixture'] + ); + done(); + }); + }); + + it('returns absolute paths on related nodes', function (done) { + axe.run('#fixture', { + absolutePaths: true + }, function (err, result) { + assert.deepEqual( + result.violations[0].nodes[0].none[0].relatedNodes[0].target, + ['html > body > #fixture'] + ); + done(); + }); + }); + }); }); describe('axe.run iframes', function () { diff --git a/test/core/utils/check-helper.js b/test/core/utils/check-helper.js index 43f250da64..ef5baad70f 100644 --- a/test/core/utils/check-helper.js +++ b/test/core/utils/check-helper.js @@ -8,8 +8,8 @@ describe('axe.utils.checkHelper', function () { assert.isFunction(axe.utils.checkHelper); }); - it('should accept 3 named parameters', function () { - assert.lengthOf(axe.utils.checkHelper, 3); + it('should accept 4 named parameters', function () { + assert.lengthOf(axe.utils.checkHelper, 4); }); it('should return an object', function () { @@ -25,10 +25,10 @@ describe('axe.utils.checkHelper', function () { helper.async(); assert.isTrue(helper.isAsync); }); - it('should call the second parameter of `axe.utils.checkHelper` when invoked', function () { + it('should call the third parameter of `axe.utils.checkHelper` when invoked', function () { function fn() { success = true; } var success = false, - helper = axe.utils.checkHelper({}, fn); + helper = axe.utils.checkHelper({}, {}, fn); var done = helper.async(); done(); @@ -36,14 +36,14 @@ describe('axe.utils.checkHelper', function () { assert.isTrue(success); }); - it('should call the third parameter of `axe.utils.checkHelper` when returning an error', function () { + it('should call the fourth parameter of `axe.utils.checkHelper` when returning an error', function () { var success = false; function reject(e) { success = true; assert.equal(e.message, 'Concrete donkey!'); } - var helper = axe.utils.checkHelper({}, noop, reject); + var helper = axe.utils.checkHelper({}, {}, noop, reject); var done = helper.async(); done( new Error('Concrete donkey!')); diff --git a/test/core/utils/dq-element.js b/test/core/utils/dq-element.js index 085fd74824..b0fedf2716 100644 --- a/test/core/utils/dq-element.js +++ b/test/core/utils/dq-element.js @@ -72,7 +72,7 @@ describe('DqElement', function () { it('should use spec object over passed element', function () { fixture.innerHTML = '
Hello!
'; - var result = new DqElement(fixture.firstChild, { + var result = new DqElement(fixture.firstChild, {}, { source: 'woot' }); assert.equal(result.source, 'woot'); @@ -100,7 +100,7 @@ describe('DqElement', function () { it('should prefer selector from spec object', function () { fixture.innerHTML = '
Hello!
'; - var result = new DqElement(fixture.firstChild, { + var result = new DqElement(fixture.firstChild, {}, { selector: 'woot' }); assert.equal(result.selector, 'woot'); @@ -126,7 +126,7 @@ describe('DqElement', function () { it('should prefer selector from spec object', function () { fixture.innerHTML = '
Hello!
'; - var result = new DqElement(fixture.firstChild, { + var result = new DqElement(fixture.firstChild, {}, { xpath: 'woot' }); assert.equal(result.xpath, 'woot'); @@ -134,6 +134,18 @@ describe('DqElement', function () { }); + describe('absolutePaths', function () { + it('creates a path all the way to root', function () { + fixture.innerHTML = '
Hello!
'; + var result = new DqElement(fixture.firstChild, { + absolutePaths: true + }); + assert.include(result.selector[0], 'html > '); + assert.include(result.selector[0], '#fixture > '); + assert.include(result.selector[0], '#foo'); + }); + }); + describe('toJSON', function () { it('should only stringify selector and source', function () { var expected = { @@ -141,7 +153,7 @@ describe('DqElement', function () { source: '', xpath: '/foo/bar/joe' }; - var result = new DqElement('joe', expected); + var result = new DqElement('joe', {}, expected); assert.deepEqual(JSON.stringify(result), JSON.stringify(expected)); }); diff --git a/test/core/utils/get-check-option.js b/test/core/utils/get-check-option.js index 73e7729425..0ac9be7151 100644 --- a/test/core/utils/get-check-option.js +++ b/test/core/utils/get-check-option.js @@ -26,7 +26,8 @@ describe('axe.utils.getCheckOption', function () { } }), { enabled: 'yes', - options: 'please' + options: 'please', + absolutePaths: undefined }); }); it('should fallback to global check options if not defined on the rule', function () { @@ -52,7 +53,8 @@ describe('axe.utils.getCheckOption', function () { } }), { enabled: 'yes', - options: 'please' + options: 'please', + absolutePaths: undefined }); }); @@ -70,7 +72,8 @@ describe('axe.utils.getCheckOption', function () { } }), { enabled: 'yes', - options: 'please' + options: 'please', + absolutePaths: undefined }); }); @@ -81,8 +84,23 @@ describe('axe.utils.getCheckOption', function () { options: 'please' }, 'monkeys', {}), { enabled: 'yes', - options: 'please' + options: 'please', + absolutePaths: undefined }); }); + it('passes absolutePaths option along', function () { + assert.deepEqual(axe.utils.getCheckOption({ + id: 'bananas', + enabled: 'on', + options: 'many' + }, 'monkeys', { + absolutePaths: 'yep' + }), { + enabled: 'on', + options: 'many', + absolutePaths: 'yep' + }); + }); + }); \ No newline at end of file From 68e294402ca7c0e639fa5e42069a17ab5879193b Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Jun 2017 13:56:02 +0200 Subject: [PATCH 05/41] choir: Describe available options in API.md --- doc/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/API.md b/doc/API.md index 4bee0fc006..be76d43bbb 100644 --- a/doc/API.md +++ b/doc/API.md @@ -300,6 +300,19 @@ The options parameter is flexible way to configure how `axe.run` operates. The d * Run all rules defined in the system, except for the list of rules specified * Run a specific set of rules provided as a list of rule ids +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 [Configutration](#api-name-axeconfigure)) +| `xpath` | `false` | Return xpath selectors for elements +| `absolutePaths` | `false` | Use absolute paths when creating element selectors +| `iframes` | `true` | Tell axe to run inside iframes +| `elementRef` | `false` | Return element references in addition to the target + + ###### Options Parameter Examples 1. Run only Rules for an accessibility standard From 0b77cc8e5f64d9154751d73d928884202be8aa62 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Mon, 5 Jun 2017 18:43:32 +0200 Subject: [PATCH 06/41] feat: Add get-friendly-url to get-selector --- lib/core/utils/get-friendly-uri-end.js | 125 ++++++++++++++++++++++++ lib/core/utils/get-selector.js | 16 ++- test/core/utils/get-friendly-uri-end.js | 52 ++++++++++ test/core/utils/get-selector.js | 19 ++++ 4 files changed, 202 insertions(+), 10 deletions(-) create mode 100644 lib/core/utils/get-friendly-uri-end.js create mode 100644 test/core/utils/get-friendly-uri-end.js diff --git a/lib/core/utils/get-friendly-uri-end.js b/lib/core/utils/get-friendly-uri-end.js new file mode 100644 index 0000000000..e80dc97d88 --- /dev/null +++ b/lib/core/utils/get-friendly-uri-end.js @@ -0,0 +1,125 @@ +/** + * Check if a string contains mostly numbers + */ +function isMostlyNumbers (str = '') { + return ( + str.length !== 0 && + (str.match(/[0-9]/g) || '').length >= str.length / 2 + ); +} + +/** + * Spit a string into an array with two pieces, at a given index + * @param String string to split + * @param Number index at which to split + * @return Array + */ +function splitString (str, splitIndex) { + return [str.substring(0, splitIndex), str.substring(splitIndex)]; +} + +/** + * Take a relative or absolute URL and pull it into it's indivisual pieces + * + * @param url (string) + * @return urlPieces + * .protocol The protocol used, e.g. 'https://' + * .domain Domain name including sub domains and TLD, e.g. 'docs.deque.com' + * .port The port number, e.g. ':8080' + * .path Path after the domain, e.g. '/home.html' + * .query Query string, e.g. '?user=admin&password=pass' + * .hash Hash / internal reference, e.g. '#footer' + */ +function uriParser (url) { + // jshint maxstatements:19 + let original = url; + let protocol = '', domain = '', port = '', path = '', query = '', hash = ''; + if (url.includes('#')) { + [url, hash] = splitString(url, url.indexOf('#')); + } + + if (url.includes('?')) { + [url, query] = splitString(url, url.indexOf('?')); + } + + if (url.includes('://')) { + [protocol, url] = url.split('://'); + [domain, url] = splitString(url, url.indexOf('/')); + } else if (url.substr(0,2) === '//') { + url = url.substr(2); + [domain, url] = splitString(url, url.indexOf('/')); + } + + if (domain.substr(0,4) === 'www.') { + domain = domain.substr(4); + } + + if (domain && domain.includes(':')) { + [domain, port] = splitString(domain, domain.indexOf(':')); + } + + path = url; // Whatever is left, must be the path + return { original, protocol, domain, port, path, query, hash }; +} + +/** + * Try to, at the end of the URI, find a string that a user can identify the URI by + * + * @param uri The URI to use + * @param options + * .currentDomain The current domain name (optional) + * .maxLength Max length of the returned string (default: 25) + * @return string A portion at the end of the uri, no longer than maxLength + */ +axe.utils.getFriendlyUriEnd = function getFriendlyUriEnd (uri = '', options = {}) { + // jshint maxstatements: 16, maxcomplexity: 13, scripturl: true + if (// Skip certain URIs: + uri.length <= 1 || // very short + uri.substr(0, 5) === 'data:' || // data URIs are unreadable + uri.substr(0, 11) === 'javascript:' || // JS isn't a URL + uri.includes('?') // query strings aren't very readable either + ) { + return; + } + + const { currentDomain, maxLength = 25 } = options; + const { path, domain, hash } = uriParser(uri); + // Split the path at the last / that has text after it + const pathEnd = path.substr( + path.substr(0, path.length-2).lastIndexOf('/') + 1 + ); + + if (hash) { + if (pathEnd && (pathEnd + hash).length <= maxLength) { + return pathEnd + hash; + } else if (pathEnd.length < 2 && hash.length > 2 && hash.length <= maxLength) { + return hash; + } else { + return; + } + } else if (domain && domain.length < maxLength && path.length <= 1) {// '' or '/' + return domain + path; + } + + // See if the domain should be returned + if (path === '/' + pathEnd && + domain && currentDomain && + domain !== currentDomain && + (domain + path).length <= maxLength + ) { + return domain + path; + } + + const lastDotIndex = pathEnd.lastIndexOf('.'); + if (// Exclude very short or very long string + (lastDotIndex === -1 || lastDotIndex > 1) && + (lastDotIndex !== -1 || pathEnd.length > 2) && + pathEnd.length <= maxLength && + // Exclude index files + !pathEnd.match(/index(\.[a-zA-Z]{2-4})?/) && + // Exclude files that are likely to be database IDs + !isMostlyNumbers(pathEnd) + ) { + return pathEnd; + } +}; diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index 76d9b4419a..f9ab4f7822 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -100,22 +100,17 @@ const createSelector = { }, // Get a selector that uses src/href props getFileRefProp (elm) { - if (elm.id) { - return; - } - // TODO: find a better way to either show a brief name, or exit empty let attr; if (elm.hasAttribute('href')) { attr = 'href'; } else if (elm.hasAttribute('src')) { attr = 'src'; + } else { + return; } - - if (attr) { - const filePath = elm.getAttribute(attr).replace(/\/$/, '').split(/\//g); - if (filePath) { - return '[' + attr + '$="' + encodeURI(filePath.pop()) + '"]'; - } + const friendlyUriEnd = axe.utils.getFriendlyUriEnd(elm.getAttribute(attr)); + if (friendlyUriEnd) { + return '[' + attr + '$="' + encodeURI(friendlyUriEnd) + '"]'; } }, // Get common node names @@ -152,6 +147,7 @@ function getElmFeatures (elm, featureCount) { createSelector.getUncommonElm, createSelector.getElmNameProp, createSelector.getDistinctClass, + createSelector.getFileRefProp, createSelector.getCommonName ].reduce((features, func) => { // As long as we haven't met our count, keep looking for features diff --git a/test/core/utils/get-friendly-uri-end.js b/test/core/utils/get-friendly-uri-end.js new file mode 100644 index 0000000000..ddb220b670 --- /dev/null +++ b/test/core/utils/get-friendly-uri-end.js @@ -0,0 +1,52 @@ +describe('axe.utils.getFriendlyUriEnd', function () { + 'use strict'; + var getFriendlyUriEnd = axe.utils.getFriendlyUriEnd; + + it('returns a domain name', function () { + assert.equal('deque.com', getFriendlyUriEnd('http://deque.com')); + assert.equal('deque.com/', getFriendlyUriEnd('https://www.deque.com/')); + assert.equal('docs.deque.com/', getFriendlyUriEnd('//docs.deque.com/')); + }); + + it('returns a filename', function () { + assert.equal('contact/', getFriendlyUriEnd('../../contact/')); + assert.equal('contact/', getFriendlyUriEnd('http://deque.com/contact/')); + assert.equal('contact', getFriendlyUriEnd('/contact')); + assert.equal('contact.html', getFriendlyUriEnd('/contact.html')); + }); + + it('returns a hash URI', function () { + assert.equal('#footer', getFriendlyUriEnd('#footer')); + assert.equal('contact.html#footer', getFriendlyUriEnd('/contact.html#footer')); + }); + + it('returns undef when there is a query', function () { + assert.isUndefined(getFriendlyUriEnd('/contact?')); + assert.isUndefined(getFriendlyUriEnd('/contact?foo=bar')); + }); + + it('returns undef for index files', function () { + assert.isUndefined(getFriendlyUriEnd('/index.cfs')); + assert.isUndefined(getFriendlyUriEnd('/index')); + }); + + it('returns undef when the result is too short', function () { + assert.isUndefined(getFriendlyUriEnd('/i.html')); + assert.isUndefined(getFriendlyUriEnd('/dq')); + }); + + it('returns undef when the result is too long', function () { + assert.isDefined(getFriendlyUriEnd('/abcd.html', { maxLength: 50 })); + assert.isDefined(getFriendlyUriEnd('#foo-bar-baz', { maxLength: 50 })); + assert.isDefined(getFriendlyUriEnd('//deque.com', { maxLength: 50 })); + + assert.isUndefined(getFriendlyUriEnd('/abcd.html', { maxLength: 5 })); + assert.isUndefined(getFriendlyUriEnd('#foo-bar-baz', { maxLength: 5 })); + assert.isUndefined(getFriendlyUriEnd('//deque.com', { maxLength: 5 })); + }); + + it('returns undef when the result has too many numbers', function () { + assert.isUndefined(getFriendlyUriEnd('123456.html')); + }); + +}); diff --git a/test/core/utils/get-selector.js b/test/core/utils/get-selector.js index 6beec682aa..36f6805290 100644 --- a/test/core/utils/get-selector.js +++ b/test/core/utils/get-selector.js @@ -202,6 +202,25 @@ describe('axe.utils.getSelector', function () { ); }); + it('should use href and src attributes', function () { + var link = document.createElement('a'); + link.setAttribute('href', '//deque.com/about/'); + fixture.appendChild(link); + + var img = document.createElement('img'); + img.setAttribute('src', '//deque.com/logo.png'); + fixture.appendChild(img); + + assert.equal( + axe.utils.getSelector(link), + '#fixture > a[href$="about/"]' + ); + assert.equal( + axe.utils.getSelector(img), + '#fixture > img[src$="logo.png"]' + ); + }); + it('should give use two features on the first element', function () { var node = document.createElement('div'); node.setAttribute('role', 'menuitem'); From 879018aec4b71d6a5c794d5f4038377e26f3a3ff Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Wed, 17 May 2017 12:29:28 -0400 Subject: [PATCH 07/41] feat: added new rule/check for aria-hidden on body Checks for aria-hidden on body element #669 --- lib/checks/aria/aria-hidden-body.js | 11 +++++++++++ lib/checks/aria/aria-hidden-body.json | 11 +++++++++++ lib/rules/aria-hidden-body.js | 11 +++++++++++ lib/rules/aria-hidden-body.json | 19 +++++++++++++++++++ test/checks/aria/aria-hidden-body.js | 22 ++++++++++++++++++++++ 5 files changed, 74 insertions(+) create mode 100644 lib/checks/aria/aria-hidden-body.js create mode 100644 lib/checks/aria/aria-hidden-body.json create mode 100644 lib/rules/aria-hidden-body.js create mode 100644 lib/rules/aria-hidden-body.json create mode 100644 test/checks/aria/aria-hidden-body.js diff --git a/lib/checks/aria/aria-hidden-body.js b/lib/checks/aria/aria-hidden-body.js new file mode 100644 index 0000000000..5150f5fceb --- /dev/null +++ b/lib/checks/aria/aria-hidden-body.js @@ -0,0 +1,11 @@ +var aria = /^aria-hidden/; +if (node && node.hasAttributes()) { + var attrs = node.attributes; + for (var i = 0, l = attrs.length; i < l; i++) { + if (aria.test(attrs[i].name)) { + return false; + } + } +} + +return true; diff --git a/lib/checks/aria/aria-hidden-body.json b/lib/checks/aria/aria-hidden-body.json new file mode 100644 index 0000000000..52e8530279 --- /dev/null +++ b/lib/checks/aria/aria-hidden-body.json @@ -0,0 +1,11 @@ +{ + "id": "aria-hidden-body", + "evaluate": "aria-hidden-body.js", + "metadata": { + "impact": "critical", + "messages": { + "pass": "No aria-hidden attribute is present on document body", + "fail": "aria-hidden should not be present on the document body" + } + } +} diff --git a/lib/rules/aria-hidden-body.js b/lib/rules/aria-hidden-body.js new file mode 100644 index 0000000000..5150f5fceb --- /dev/null +++ b/lib/rules/aria-hidden-body.js @@ -0,0 +1,11 @@ +var aria = /^aria-hidden/; +if (node && node.hasAttributes()) { + var attrs = node.attributes; + for (var i = 0, l = attrs.length; i < l; i++) { + if (aria.test(attrs[i].name)) { + return false; + } + } +} + +return true; diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json new file mode 100644 index 0000000000..3f3b5b5810 --- /dev/null +++ b/lib/rules/aria-hidden-body.json @@ -0,0 +1,19 @@ +{ + "id": "aria-hidden-body", + "matches": "aria-hidden-body.js", + "tags": [ + "cat.aria", + "wcag2a", + "wcag411", + "wcag412" + ], + "metadata": { + "description": "Ensures aria-hidden is not present on the document body.", + "help": "Document body should not include the aria-hidden attribute." + }, + "all": [], + "any": [ + "aria-hidden-body" + ], + "none": [] +} diff --git a/test/checks/aria/aria-hidden-body.js b/test/checks/aria/aria-hidden-body.js new file mode 100644 index 0000000000..69762edeba --- /dev/null +++ b/test/checks/aria/aria-hidden-body.js @@ -0,0 +1,22 @@ +describe('aria-hidden', function () { + 'use strict'; + + var node = document.body; + + var checkContext = { + _data: null, + data: function (d) { + this._data = d; + } + }; + + it('should not be present on document.body', function () { + assert.isTrue(checks['aria-hidden-body'].evaluate.call(checkContext, node)); + }); + + it('fails appropriately if aria-hidden on document.body', function () { + node.setAttribute('aria-hidden', 'false'); + assert.isFalse(checks['aria-hidden-body'].evaluate.call(checkContext, node)); + }); + +}); From 34a6db6cd97257adb2e8c1a836fccb0928cd1118 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Mon, 22 May 2017 09:14:57 -0400 Subject: [PATCH 08/41] PR feedback: check for attr instead of iterating --- doc/rule-descriptions.md | 1 + lib/checks/aria/aria-hidden-body.js | 11 +++-------- lib/rules/aria-hidden-body.js | 11 ----------- lib/rules/aria-hidden-body.json | 2 +- test/checks/aria/aria-hidden-body.js | 11 ++++++++--- 5 files changed, 13 insertions(+), 23 deletions(-) delete mode 100644 lib/rules/aria-hidden-body.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index bdcd8bd46d..3e2b473f16 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -3,6 +3,7 @@ | accesskeys | Ensures every accesskey attribute value is unique | wcag2a, wcag211, cat.keyboard | true | | area-alt | Ensures <area> elements of image maps have alternate text | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | true | | aria-allowed-attr | Ensures ARIA attributes are allowed for an element's role | cat.aria, wcag2a, wcag411, wcag412 | true | +| aria-hidden-body | Ensures aria-hidden is not present on the document body. | cat.aria, wcag2a, wcag411, wcag412 | true | | aria-required-attr | Ensures elements with ARIA roles have all required ARIA attributes | cat.aria, wcag2a, wcag411, wcag412 | true | | aria-required-children | Ensures elements with an ARIA role that require child roles contain them | cat.aria, wcag2a, wcag131 | true | | aria-required-parent | Ensures elements with an ARIA role that require parent roles are contained by them | cat.aria, wcag2a, wcag131 | true | diff --git a/lib/checks/aria/aria-hidden-body.js b/lib/checks/aria/aria-hidden-body.js index 5150f5fceb..ac3dcea844 100644 --- a/lib/checks/aria/aria-hidden-body.js +++ b/lib/checks/aria/aria-hidden-body.js @@ -1,11 +1,6 @@ -var aria = /^aria-hidden/; -if (node && node.hasAttributes()) { - var attrs = node.attributes; - for (var i = 0, l = attrs.length; i < l; i++) { - if (aria.test(attrs[i].name)) { - return false; - } - } +var aria = 'aria-hidden'; +if (node && node.hasAttribute(aria)) { + return false; } return true; diff --git a/lib/rules/aria-hidden-body.js b/lib/rules/aria-hidden-body.js deleted file mode 100644 index 5150f5fceb..0000000000 --- a/lib/rules/aria-hidden-body.js +++ /dev/null @@ -1,11 +0,0 @@ -var aria = /^aria-hidden/; -if (node && node.hasAttributes()) { - var attrs = node.attributes; - for (var i = 0, l = attrs.length; i < l; i++) { - if (aria.test(attrs[i].name)) { - return false; - } - } -} - -return true; diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json index 3f3b5b5810..671fb78b6e 100644 --- a/lib/rules/aria-hidden-body.json +++ b/lib/rules/aria-hidden-body.json @@ -1,6 +1,6 @@ { "id": "aria-hidden-body", - "matches": "aria-hidden-body.js", + "selector": "body", "tags": [ "cat.aria", "wcag2a", diff --git a/test/checks/aria/aria-hidden-body.js b/test/checks/aria/aria-hidden-body.js index 69762edeba..003626c24b 100644 --- a/test/checks/aria/aria-hidden-body.js +++ b/test/checks/aria/aria-hidden-body.js @@ -1,7 +1,7 @@ describe('aria-hidden', function () { 'use strict'; - var node = document.body; + var node = document.getElementsByTagName('body')[0]; var checkContext = { _data: null, @@ -10,13 +10,18 @@ describe('aria-hidden', function () { } }; + afterEach(function () { + checkContext._data = null; + node.removeAttribute('aria-hidden'); + }); + it('should not be present on document.body', function () { assert.isTrue(checks['aria-hidden-body'].evaluate.call(checkContext, node)); }); it('fails appropriately if aria-hidden on document.body', function () { - node.setAttribute('aria-hidden', 'false'); - assert.isFalse(checks['aria-hidden-body'].evaluate.call(checkContext, node)); + node.setAttribute('aria-hidden', 'true'); + assert.isTrue(checks['aria-hidden-body'].evaluate.call(checkContext, node)); }); }); From d5dbbc8e6a79eba3b8b07bf4911dff32f4d13b15 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Mon, 5 Jun 2017 13:39:24 -0400 Subject: [PATCH 09/41] Updated test --- test/checks/aria/aria-hidden-body.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/checks/aria/aria-hidden-body.js b/test/checks/aria/aria-hidden-body.js index 003626c24b..d632a75870 100644 --- a/test/checks/aria/aria-hidden-body.js +++ b/test/checks/aria/aria-hidden-body.js @@ -21,7 +21,7 @@ describe('aria-hidden', function () { it('fails appropriately if aria-hidden on document.body', function () { node.setAttribute('aria-hidden', 'true'); - assert.isTrue(checks['aria-hidden-body'].evaluate.call(checkContext, node)); + assert.isFalse(checks['aria-hidden-body'].evaluate.call(checkContext, node)); }); }); From 2546a82a78fb26edc6b090f3ff7e5308121cbebd Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 7 Jun 2017 11:31:04 +0200 Subject: [PATCH 10/41] fix: Remove duplicate test for disable --- lib/core/utils/get-selector.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index f9ab4f7822..667a8d61d7 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -5,7 +5,7 @@ function isUncommonClassName (className) { 'focus', 'hover', 'hidden', 'visible', 'dirty', 'touched', 'valid', 'disable', - 'disable', 'enable', 'active', 'col-' + 'enable', 'active', 'col-' ].find(str => className.includes(str)); } From 75a22267456326c44447461623af21fc27f4e343 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Thu, 8 Jun 2017 13:21:28 -0400 Subject: [PATCH 11/41] Removed false positives, updated help description --- lib/checks/aria/aria-hidden-body.js | 2 +- lib/checks/aria/aria-hidden-body.json | 2 +- lib/rules/aria-hidden-body.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checks/aria/aria-hidden-body.js b/lib/checks/aria/aria-hidden-body.js index ac3dcea844..bf04eff602 100644 --- a/lib/checks/aria/aria-hidden-body.js +++ b/lib/checks/aria/aria-hidden-body.js @@ -1,5 +1,5 @@ var aria = 'aria-hidden'; -if (node && node.hasAttribute(aria)) { +if (node && node.hasAttribute(aria) && node.getAttribute(aria) === 'true') { return false; } diff --git a/lib/checks/aria/aria-hidden-body.json b/lib/checks/aria/aria-hidden-body.json index 52e8530279..c4d2d85cca 100644 --- a/lib/checks/aria/aria-hidden-body.json +++ b/lib/checks/aria/aria-hidden-body.json @@ -5,7 +5,7 @@ "impact": "critical", "messages": { "pass": "No aria-hidden attribute is present on document body", - "fail": "aria-hidden should not be present on the document body" + "fail": "aria-hidden=true should not be present on the document body" } } } diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json index 671fb78b6e..96a27bd149 100644 --- a/lib/rules/aria-hidden-body.json +++ b/lib/rules/aria-hidden-body.json @@ -9,7 +9,7 @@ ], "metadata": { "description": "Ensures aria-hidden is not present on the document body.", - "help": "Document body should not include the aria-hidden attribute." + "help": "Document body should not include the aria-hidden attribute. Including aria-hidden=true on the document body would render the entire page unusable by screen readers." }, "all": [], "any": [ From 7c48efb6cc4ba0a30e0e9436dfb9af4bbdd101d2 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Thu, 8 Jun 2017 13:24:02 -0400 Subject: [PATCH 12/41] Updated test for previous commit --- test/checks/aria/aria-hidden-body.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/checks/aria/aria-hidden-body.js b/test/checks/aria/aria-hidden-body.js index d632a75870..dd074eb77d 100644 --- a/test/checks/aria/aria-hidden-body.js +++ b/test/checks/aria/aria-hidden-body.js @@ -19,9 +19,14 @@ describe('aria-hidden', function () { assert.isTrue(checks['aria-hidden-body'].evaluate.call(checkContext, node)); }); - it('fails appropriately if aria-hidden on document.body', function () { + it('fails appropriately if aria-hidden=true on document.body', function () { node.setAttribute('aria-hidden', 'true'); assert.isFalse(checks['aria-hidden-body'].evaluate.call(checkContext, node)); }); + it('passes if aria-hidden=false on document.body', function () { + node.setAttribute('aria-hidden', 'false'); + assert.isTrue(checks['aria-hidden-body'].evaluate.call(checkContext, node)); + }); + }); From c38b3b5da0bdeb85adcd4e46d41e1bfda8988a9b Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Fri, 9 Jun 2017 10:52:09 -0400 Subject: [PATCH 13/41] Removed unused tag; added integration tests --- lib/checks/aria/aria-hidden-body.js | 7 +------ lib/rules/aria-hidden-body.json | 4 +--- .../rules/aria-hidden-body/hidden-body.html | 5 +++++ .../rules/aria-hidden-body/hidden-body.json | 11 +++++++++++ 4 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 test/integration/rules/aria-hidden-body/hidden-body.html create mode 100644 test/integration/rules/aria-hidden-body/hidden-body.json diff --git a/lib/checks/aria/aria-hidden-body.js b/lib/checks/aria/aria-hidden-body.js index bf04eff602..689bc2739f 100644 --- a/lib/checks/aria/aria-hidden-body.js +++ b/lib/checks/aria/aria-hidden-body.js @@ -1,6 +1 @@ -var aria = 'aria-hidden'; -if (node && node.hasAttribute(aria) && node.getAttribute(aria) === 'true') { - return false; -} - -return true; +return node.getAttribute('aria-hidden') === 'true'; diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json index 96a27bd149..d4cc633299 100644 --- a/lib/rules/aria-hidden-body.json +++ b/lib/rules/aria-hidden-body.json @@ -3,9 +3,7 @@ "selector": "body", "tags": [ "cat.aria", - "wcag2a", - "wcag411", - "wcag412" + "wcag2a" ], "metadata": { "description": "Ensures aria-hidden is not present on the document body.", diff --git a/test/integration/rules/aria-hidden-body/hidden-body.html b/test/integration/rules/aria-hidden-body/hidden-body.html new file mode 100644 index 0000000000..a1c57847c7 --- /dev/null +++ b/test/integration/rules/aria-hidden-body/hidden-body.html @@ -0,0 +1,5 @@ + +ok +ok + +fail diff --git a/test/integration/rules/aria-hidden-body/hidden-body.json b/test/integration/rules/aria-hidden-body/hidden-body.json new file mode 100644 index 0000000000..d207016f83 --- /dev/null +++ b/test/integration/rules/aria-hidden-body/hidden-body.json @@ -0,0 +1,11 @@ +{ + "description": "aria-hidden-body tests", + "rule": "aria-hidden-body", + "violations": [ + ["#violation1"] + ], + "passes": [ + ["#pass1"], + ["#pass2"] + ] +} From 7ce15d10eb57dd209b85782be7569c2ec8a71a42 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Fri, 9 Jun 2017 13:00:31 -0400 Subject: [PATCH 14/41] Updated rule/check and test --- doc/rule-descriptions.md | 2 +- lib/checks/aria/aria-hidden-body.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index 3e2b473f16..7487f59b49 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -3,7 +3,7 @@ | accesskeys | Ensures every accesskey attribute value is unique | wcag2a, wcag211, cat.keyboard | true | | area-alt | Ensures <area> elements of image maps have alternate text | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | true | | aria-allowed-attr | Ensures ARIA attributes are allowed for an element's role | cat.aria, wcag2a, wcag411, wcag412 | true | -| aria-hidden-body | Ensures aria-hidden is not present on the document body. | cat.aria, wcag2a, wcag411, wcag412 | true | +| aria-hidden-body | Ensures aria-hidden is not present on the document body. | cat.aria, wcag2a | true | | aria-required-attr | Ensures elements with ARIA roles have all required ARIA attributes | cat.aria, wcag2a, wcag411, wcag412 | true | | aria-required-children | Ensures elements with an ARIA role that require child roles contain them | cat.aria, wcag2a, wcag131 | true | | aria-required-parent | Ensures elements with an ARIA role that require parent roles are contained by them | cat.aria, wcag2a, wcag131 | true | diff --git a/lib/checks/aria/aria-hidden-body.js b/lib/checks/aria/aria-hidden-body.js index 689bc2739f..2cb98c9513 100644 --- a/lib/checks/aria/aria-hidden-body.js +++ b/lib/checks/aria/aria-hidden-body.js @@ -1 +1 @@ -return node.getAttribute('aria-hidden') === 'true'; +return node.getAttribute('aria-hidden') !== 'true'; From 1d2d49dd8ad4a0ce94ab1cfc530db0e8aa68adb8 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Mon, 12 Jun 2017 12:57:27 -0400 Subject: [PATCH 15/41] Updated integration tests --- .../full/aria-hidden-body/fail.html | 26 +++++++++++++++++++ .../integration/full/aria-hidden-body/fail.js | 17 ++++++++++++ .../full/aria-hidden-body/pass.html | 26 +++++++++++++++++++ .../integration/full/aria-hidden-body/pass.js | 23 ++++++++++++++++ .../rules/aria-hidden-body/hidden-body.html | 5 ---- .../rules/aria-hidden-body/hidden-body.json | 11 -------- 6 files changed, 92 insertions(+), 16 deletions(-) create mode 100644 test/integration/full/aria-hidden-body/fail.html create mode 100644 test/integration/full/aria-hidden-body/fail.js create mode 100644 test/integration/full/aria-hidden-body/pass.html create mode 100644 test/integration/full/aria-hidden-body/pass.js delete mode 100644 test/integration/rules/aria-hidden-body/hidden-body.html delete mode 100644 test/integration/rules/aria-hidden-body/hidden-body.json diff --git a/test/integration/full/aria-hidden-body/fail.html b/test/integration/full/aria-hidden-body/fail.html new file mode 100644 index 0000000000..63228c8811 --- /dev/null +++ b/test/integration/full/aria-hidden-body/fail.html @@ -0,0 +1,26 @@ + + + +aria-hidden on body Test + + + + + + + + + +

Some title.

+ Deque +
+ + + + diff --git a/test/integration/full/aria-hidden-body/fail.js b/test/integration/full/aria-hidden-body/fail.js new file mode 100644 index 0000000000..28f22d5aee --- /dev/null +++ b/test/integration/full/aria-hidden-body/fail.js @@ -0,0 +1,17 @@ +describe('aria-hidden on body test ' + window.location.pathname, function () { + 'use strict'; + var results; + before(function (done) { + axe.run({ runOnly: { type: 'rule', values: ['aria-hidden-body'] } }, function (err, r) { + assert.isNull(err); + results = r; + done(); + }); + }); + + describe('violations', function () { + it('should find 1', function () { + assert.lengthOf(results.violations, 1); + }); + }); +}); diff --git a/test/integration/full/aria-hidden-body/pass.html b/test/integration/full/aria-hidden-body/pass.html new file mode 100644 index 0000000000..f912c95584 --- /dev/null +++ b/test/integration/full/aria-hidden-body/pass.html @@ -0,0 +1,26 @@ + + + +aria-hidden on body Test + + + + + + + + + +

Some title.

+ Deque +
+ + + + diff --git a/test/integration/full/aria-hidden-body/pass.js b/test/integration/full/aria-hidden-body/pass.js new file mode 100644 index 0000000000..12854025ab --- /dev/null +++ b/test/integration/full/aria-hidden-body/pass.js @@ -0,0 +1,23 @@ +describe('aria-hidden on body test ' + window.location.pathname, function () { + 'use strict'; + var results; + before(function (done) { + axe.run({ runOnly: { type: 'rule', values: ['aria-hidden-body'] } }, function (err, r) { + assert.isNull(err); + results = r; + done(); + }); + }); + + describe('violations', function () { + it('should find none', function () { + assert.lengthOf(results.violations, 0); + }); + }); + + describe('passes', function () { + it('should find 1', function () { + assert.lengthOf(results.passes[0].nodes, 1); + }); + }); +}); diff --git a/test/integration/rules/aria-hidden-body/hidden-body.html b/test/integration/rules/aria-hidden-body/hidden-body.html deleted file mode 100644 index a1c57847c7..0000000000 --- a/test/integration/rules/aria-hidden-body/hidden-body.html +++ /dev/null @@ -1,5 +0,0 @@ - -ok -ok - -fail diff --git a/test/integration/rules/aria-hidden-body/hidden-body.json b/test/integration/rules/aria-hidden-body/hidden-body.json deleted file mode 100644 index d207016f83..0000000000 --- a/test/integration/rules/aria-hidden-body/hidden-body.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "description": "aria-hidden-body tests", - "rule": "aria-hidden-body", - "violations": [ - ["#violation1"] - ], - "passes": [ - ["#pass1"], - ["#pass2"] - ] -} From d866717584b81cc2afc1164229cbf4ba98724a5e Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Mon, 12 Jun 2017 13:18:07 -0400 Subject: [PATCH 16/41] Fixed integration tests for violations --- test/integration/full/aria-hidden-body/fail.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/full/aria-hidden-body/fail.js b/test/integration/full/aria-hidden-body/fail.js index 28f22d5aee..7aa1ede799 100644 --- a/test/integration/full/aria-hidden-body/fail.js +++ b/test/integration/full/aria-hidden-body/fail.js @@ -11,7 +11,7 @@ describe('aria-hidden on body test ' + window.location.pathname, function () { describe('violations', function () { it('should find 1', function () { - assert.lengthOf(results.violations, 1); + assert.lengthOf(results.violations[0].nodes, 1); }); }); }); From ee02a32c637e6ff1a8b42128b393002a80a15f23 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Mon, 22 May 2017 14:00:18 -0400 Subject: [PATCH 17/41] feat: added new rule/check notifying for unseen or hidden content Checks for hidden content or aria-hidden on elements flagged them for review #669 --- doc/rule-descriptions.md | 1 + lib/checks/visibility/hidden-content.js | 8 +++++ lib/checks/visibility/hidden-content.json | 12 ++++++++ lib/rules/hidden-content.json | 18 ++++++++++++ test/checks/visibility/hidden-content.js | 36 +++++++++++++++++++++++ 5 files changed, 75 insertions(+) create mode 100644 lib/checks/visibility/hidden-content.js create mode 100644 lib/checks/visibility/hidden-content.json create mode 100644 lib/rules/hidden-content.json create mode 100644 test/checks/visibility/hidden-content.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index bdcd8bd46d..cf0e344803 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -23,6 +23,7 @@ | frame-title-unique | Ensures <iframe> and <frame> elements contain a unique title attribute | cat.text-alternatives, best-practice | true | | frame-title | Ensures <iframe> and <frame> elements contain a non-empty title attribute | cat.text-alternatives, wcag2a, wcag241, section508, section508.22.i | true | | heading-order | Ensures the order of headings is semantically correct | cat.semantics, best-practice | false | +| hidden-content | Informs users about hidden content. | experimental, review-item | false | | href-no-hash | Ensures that href values are valid link references to promote only using anchors as links | cat.semantics, best-practice | false | | html-has-lang | Ensures every HTML document has a lang attribute | cat.language, wcag2a, wcag311 | true | | html-lang-valid | Ensures the lang attribute of the <html> element has a valid value | cat.language, wcag2a, wcag311 | true | diff --git a/lib/checks/visibility/hidden-content.js b/lib/checks/visibility/hidden-content.js new file mode 100644 index 0000000000..ae2bd4b997 --- /dev/null +++ b/lib/checks/visibility/hidden-content.js @@ -0,0 +1,8 @@ +let styles = window.getComputedStyle(node); + +if (axe.commons.dom.hasContent(node)) { + if (styles.getPropertyValue('display') === 'none' || styles.getPropertyValue('visibility') === 'hidden') { + return undefined; + } +} +return true; diff --git a/lib/checks/visibility/hidden-content.json b/lib/checks/visibility/hidden-content.json new file mode 100644 index 0000000000..6b4c974af0 --- /dev/null +++ b/lib/checks/visibility/hidden-content.json @@ -0,0 +1,12 @@ +{ + "id": "hidden-content", + "evaluate": "hidden-content.js", + "metadata": { + "impact": "minor", + "messages": { + "pass": "All content on the page has been analyzed.", + "fail": "There were problems analyzing the content on this page.", + "undefined": "There is hidden content on the page that was not analyzed. You will need to trigger the display of this content in order to analyze it." + } + } +} diff --git a/lib/rules/hidden-content.json b/lib/rules/hidden-content.json new file mode 100644 index 0000000000..07616b59c8 --- /dev/null +++ b/lib/rules/hidden-content.json @@ -0,0 +1,18 @@ +{ + "id": "hidden-content", + "selector": "*", + "tags": [ + "experimental", + "review-item" + ], + "metadata": { + "description": "Informs users about hidden content.", + "help": "Trigger the UI state that displays the content and then run the analysis again." + }, + "all": [], + "any": [ + "hidden-content" + ], + "none": [], + "enabled": false +} diff --git a/test/checks/visibility/hidden-content.js b/test/checks/visibility/hidden-content.js new file mode 100644 index 0000000000..924f5cb9e9 --- /dev/null +++ b/test/checks/visibility/hidden-content.js @@ -0,0 +1,36 @@ +describe('hidden content', function () { + 'use strict'; + + var fixture = document.getElementById('fixture'); + + var checkContext = { + _data: null, + data: function (d) { + this._data = d; + } + }; + + afterEach(function () { + fixture.innerHTML = ''; + checkContext._data = null; + }); + + it('should return undefined with display:none and children', function () { + fixture.innerHTML = ''; + var node = fixture.querySelector('#target'); + assert.isUndefined(checks['hidden-content'].evaluate.call(checkContext, node)); + }); + + it('should return undefined with visibility:hidden and children', function () { + fixture.innerHTML = ''; + var node = fixture.querySelector('#target'); + assert.isUndefined(checks['hidden-content'].evaluate.call(checkContext, node)); + }); + + it('should return true with aria-hidden and no content', function () { + fixture.innerHTML = ''; + var node = fixture.querySelector('#target'); + assert.isTrue(checks['hidden-content'].evaluate.call(checkContext, node)); + }); + +}); From 8add1142635e82f2fd7106bc8fc9c3ee8f593cbf Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Tue, 13 Jun 2017 08:43:39 -0400 Subject: [PATCH 18/41] Updated test to check for hidden --- lib/rules/aria-hidden-body.json | 1 + test/integration/full/aria-hidden-body/fail.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json index d4cc633299..26d339582e 100644 --- a/lib/rules/aria-hidden-body.json +++ b/lib/rules/aria-hidden-body.json @@ -1,6 +1,7 @@ { "id": "aria-hidden-body", "selector": "body", + "excludeHidden": "false", "tags": [ "cat.aria", "wcag2a" diff --git a/test/integration/full/aria-hidden-body/fail.js b/test/integration/full/aria-hidden-body/fail.js index 7aa1ede799..fa206031b4 100644 --- a/test/integration/full/aria-hidden-body/fail.js +++ b/test/integration/full/aria-hidden-body/fail.js @@ -10,8 +10,8 @@ describe('aria-hidden on body test ' + window.location.pathname, function () { }); describe('violations', function () { - it('should find 1', function () { - assert.lengthOf(results.violations[0].nodes, 1); + it('should find some', function () { + assert.lengthOf(results.violations, 1); }); }); }); From dec33cfa7ffb530587f20cd503fbeabe8c292ded Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Tue, 13 Jun 2017 09:21:31 -0400 Subject: [PATCH 19/41] Updated tests and checks --- lib/checks/visibility/hidden-content.js | 9 ++++++++- lib/rules/hidden-content.json | 1 + test/checks/visibility/hidden-content.js | 6 ++++++ .../rules/hidden-content/hidden-content.html | 3 +++ .../rules/hidden-content/hidden-content.json | 14 ++++++++++++++ 5 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 test/integration/rules/hidden-content/hidden-content.html create mode 100644 test/integration/rules/hidden-content/hidden-content.json diff --git a/lib/checks/visibility/hidden-content.js b/lib/checks/visibility/hidden-content.js index ae2bd4b997..5d60a319de 100644 --- a/lib/checks/visibility/hidden-content.js +++ b/lib/checks/visibility/hidden-content.js @@ -1,8 +1,15 @@ let styles = window.getComputedStyle(node); if (axe.commons.dom.hasContent(node)) { - if (styles.getPropertyValue('display') === 'none' || styles.getPropertyValue('visibility') === 'hidden') { + if (styles.getPropertyValue('display') === 'none') { return undefined; + } else if (styles.getPropertyValue('visibility') === 'hidden') { + if (node.parentNode) { + var parentStyle = window.getComputedStyle(node.parentNode); + } + if (!parentStyle || parentStyle.getPropertyValue('visibility') !== 'hidden') { + return undefined; + } } } return true; diff --git a/lib/rules/hidden-content.json b/lib/rules/hidden-content.json index 07616b59c8..5d0cf19c6c 100644 --- a/lib/rules/hidden-content.json +++ b/lib/rules/hidden-content.json @@ -1,6 +1,7 @@ { "id": "hidden-content", "selector": "*", + "excludeHidden": false, "tags": [ "experimental", "review-item" diff --git a/test/checks/visibility/hidden-content.js b/test/checks/visibility/hidden-content.js index 924f5cb9e9..a3b61041c1 100644 --- a/test/checks/visibility/hidden-content.js +++ b/test/checks/visibility/hidden-content.js @@ -27,6 +27,12 @@ describe('hidden content', function () { assert.isUndefined(checks['hidden-content'].evaluate.call(checkContext, node)); }); + it('should return true with visibility:hidden and parent with visibility:hidden', function () { + fixture.innerHTML = '
'; + var node = fixture.querySelector('#target'); + assert.isTrue(checks['hidden-content'].evaluate.call(checkContext, node)); + }); + it('should return true with aria-hidden and no content', function () { fixture.innerHTML = ''; var node = fixture.querySelector('#target'); diff --git a/test/integration/rules/hidden-content/hidden-content.html b/test/integration/rules/hidden-content/hidden-content.html new file mode 100644 index 0000000000..00865c23ab --- /dev/null +++ b/test/integration/rules/hidden-content/hidden-content.html @@ -0,0 +1,3 @@ + + + diff --git a/test/integration/rules/hidden-content/hidden-content.json b/test/integration/rules/hidden-content/hidden-content.json new file mode 100644 index 0000000000..81de6a9f79 --- /dev/null +++ b/test/integration/rules/hidden-content/hidden-content.json @@ -0,0 +1,14 @@ +{ + "description": "hidden-content test", + "rule": "hidden-content", + "incomplete": [ + ["#canttell1"], + ["#canttell2"] + ], + "passes": [ + ["#fixture"], + ["#pass1"], + ["#pass2"], + ["#pass3"] + ] +} From 62a22e6ef6652d2202c4d47682cb3bde85bad8fd Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Tue, 13 Jun 2017 09:24:13 -0400 Subject: [PATCH 20/41] Made actual truthy --- lib/rules/aria-hidden-body.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json index 26d339582e..bdbd58f976 100644 --- a/lib/rules/aria-hidden-body.json +++ b/lib/rules/aria-hidden-body.json @@ -1,7 +1,7 @@ { "id": "aria-hidden-body", "selector": "body", - "excludeHidden": "false", + "excludeHidden": false, "tags": [ "cat.aria", "wcag2a" From ae5b9da04e649803d1c1a8d0a252943218ff1d0f Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Tue, 13 Jun 2017 11:42:16 -0400 Subject: [PATCH 21/41] Adding tag for criterion --- lib/rules/aria-hidden-body.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/aria-hidden-body.json b/lib/rules/aria-hidden-body.json index bdbd58f976..48f355748a 100644 --- a/lib/rules/aria-hidden-body.json +++ b/lib/rules/aria-hidden-body.json @@ -4,7 +4,8 @@ "excludeHidden": false, "tags": [ "cat.aria", - "wcag2a" + "wcag2a", + "wcag412" ], "metadata": { "description": "Ensures aria-hidden is not present on the document body.", From 36a7c3003a6730e184a3dbeb0641bb5766680e0f Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Fri, 12 May 2017 17:03:23 -0700 Subject: [PATCH 22/41] fix: improve calculateObscuringAlpha for overlap Don't count implicit labels as obscuring --- lib/commons/color/get-background-color.js | 19 ++++++-- test/checks/color/color-contrast.js | 35 +++++++++++++++ .../rules/color-contrast/color-contrast.html | 44 ++++++++++++++----- .../rules/color-contrast/color-contrast.json | 4 +- 4 files changed, 86 insertions(+), 16 deletions(-) diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 316d028118..a9350d5a9f 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -38,16 +38,27 @@ function getBgColor(elm, elmStyle) { return bgColor; } -function calculateObscuringAlpha(elmIndex, elmStack) { +function contentOverlapped(targetElement) { + // Get dimensions of text content box. + // Subsequent clientRects are bounding boxes, if different than content box + var targetRect = targetElement.getClientRects()[0]; + var elementAtPoint = document.elementFromPoint(targetRect.left, targetRect.top); + if (elementAtPoint !== null && elementAtPoint.innerHTML !== targetElement.innerHTML) { + return true; + } + return false; +} + +function calculateObscuringAlpha(elmIndex, elmStack, originalElm) { var totalAlpha = 0; if (elmIndex > 0) { - // there are elements above our element, check if they are contribute to the background + // there are elements above our element, check if they contribute to the background for (var i = elmIndex - 1; i >= 0; i--) { let bgElm = elmStack[i]; let bgElmStyle = window.getComputedStyle(bgElm); let bgColor = getBgColor(bgElm, bgElmStyle); - if (bgColor.alpha) { + if (bgColor.alpha && contentOverlapped(originalElm, bgElm)) { totalAlpha += bgColor.alpha; } else { // remove elements not contributing to the background @@ -102,7 +113,7 @@ color.getBackgroundStack = function(elm) { // Return all elements BELOW the current element, null if the element is undefined let elmIndex = elmStack.indexOf(elm); - if (calculateObscuringAlpha(elmIndex, elmStack) >= 0.99) { + if (calculateObscuringAlpha(elmIndex, elmStack, elm) >= 0.99) { // if the total of the elements above our element results in total obscuring, return null axe.commons.color.incompleteData.set('bgColor', 'bgOverlap'); return null; diff --git a/test/checks/color/color-contrast.js b/test/checks/color/color-contrast.js index 15e0040295..16af5f5c82 100644 --- a/test/checks/color/color-contrast.js +++ b/test/checks/color/color-contrast.js @@ -157,4 +157,39 @@ describe('color-contrast', function () { assert.equal(checkContext._data.missingData, 'bgGradient'); assert.equal(checkContext._data.contrastRatio, 0); }); + + it('should return undefined when there are elements overlapping', function () { + fixture.innerHTML = '
' + + 'My text
'; + var target = fixture.querySelector('#target'); + assert.isUndefined(checks['color-contrast'].evaluate.call(checkContext, target)); + assert.equal(checkContext._data.missingData[0].reason, 'bgOverlap'); + assert.equal(checkContext._data.contrastRatio, 0); + }); + + it('should return true when a form wraps mixed content', function() { + fixture.innerHTML = '

Some text

'; + var target = fixture.querySelector('#pass6'); + assert.isTrue(checks['color-contrast'].evaluate.call(checkContext, target)); + }); + + it('should return true when a label wraps a text input', function () { + fixture.innerHTML = ''; + var target = fixture.querySelector('#target'); + var result = checks['color-contrast'].evaluate.call(checkContext, target); + assert.isTrue(result); + }); + + it('should return undefined if element overlaps text content', function () { + fixture.innerHTML = '
' + + '
Hi
' + + '
' + + '
'; + var target = fixture.querySelector('#target'); + var actual = checks['color-contrast'].evaluate.call(checkContext, target); + assert.isUndefined(actual); + assert.equal(checkContext._data.missingData[0].reason, 'bgOverlap'); + assert.equal(checkContext._data.contrastRatio, 0); + }); }); diff --git a/test/integration/rules/color-contrast/color-contrast.html b/test/integration/rules/color-contrast/color-contrast.html index ff6b960879..43e349d32b 100644 --- a/test/integration/rules/color-contrast/color-contrast.html +++ b/test/integration/rules/color-contrast/color-contrast.html @@ -1,12 +1,17 @@ -
This is a pass.
This is a pass.
This is a pass.
+
This is a fail.But this is a pass.
+ +
+
Pass.
+
+ + +
This is a fail.
This is a fail.
-
Pass.
-
Fail.
Hi
This is a fail.
@@ -25,6 +30,7 @@ + @@ -33,18 +39,34 @@ +
text
- +
Hello
+ + +
+
Background image
+
+ +
+
+
Outside parent
+
+
+ +
+
Background overlap
+
-
Pass by default.
-
Hi
-
Hi
-
Hi
+
+
Element overlap
+
+
diff --git a/test/integration/rules/color-contrast/color-contrast.json b/test/integration/rules/color-contrast/color-contrast.json index ee6cd7a3b5..80ec9a8eba 100644 --- a/test/integration/rules/color-contrast/color-contrast.json +++ b/test/integration/rules/color-contrast/color-contrast.json @@ -27,7 +27,9 @@ ["#pass2"], ["#pass3"], ["#pass4"], - ["#pass5"] + ["#pass5"], + ["#pass6"], + ["#pass6 > input"] ], "incomplete": [ ["#canttell1"], From 6c34d45c249670670e5cabffe14124897ef9af6c Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Mon, 15 May 2017 18:42:11 -0700 Subject: [PATCH 23/41] feat: add incomplete data for elmOutsideParent Helps debugging when this condition is met --- lib/checks/color/color-contrast.json | 1 + lib/commons/color/get-background-color.js | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/checks/color/color-contrast.json b/lib/checks/color/color-contrast.json index a1cbb0077e..6a768eca1a 100644 --- a/lib/checks/color/color-contrast.json +++ b/lib/checks/color/color-contrast.json @@ -12,6 +12,7 @@ "imgNode": "Element's background color could not be determined because element contains an image node", "bgOverlap": "Element's background color could not be determined because it is overlapped by another element", "fgAlpha" : "Element's foreground color could not be determined because of alpha transparency", + "elmOutsideParent": "Element's background color could not be determined because it extends beyond a parent node", "default": "Unable to determine contrast ratio" } } diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index a9350d5a9f..9aa6e1c3d4 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -70,8 +70,16 @@ function calculateObscuringAlpha(elmIndex, elmStack, originalElm) { } function elmOutsideParent(elm, bgElm, bgColor) { - var visible = (elm !== bgElm && !dom.visuallyContains(elm, bgElm) && bgColor.alpha !== 0); - return visible; + var outside = (elm !== bgElm && !dom.visuallyContains(elm, bgElm) && bgColor.alpha !== 0); + if (outside) { + axe.commons.color.incompleteData.set('bgColor', { + node: elm, + reason: 'elmOutsideParent' + }); + } + return outside; +} + } /** * Get all elements rendered underneath the current element, From 098e39401b824b343cafc23cd61c22afd4bb43a5 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Mon, 15 May 2017 18:48:28 -0700 Subject: [PATCH 24/41] feat(visuallyContains): limit to non-inline elms scrollPosition/area is very unpredictable cross-browser for inline elements, so just return true in that case --- lib/commons/dom/visually-contains.js | 11 ++++++++--- test/commons/dom/visually-contains.js | 8 ++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/commons/dom/visually-contains.js b/lib/commons/dom/visually-contains.js index b0ccb340d2..158892e691 100644 --- a/lib/commons/dom/visually-contains.js +++ b/lib/commons/dom/visually-contains.js @@ -1,5 +1,5 @@ /* global dom */ -/* jshint maxcomplexity: 11 */ +/* jshint maxcomplexity: 12 */ /** * Checks whether a parent element visually contains its child, either directly or via scrolling. @@ -27,6 +27,13 @@ dom.visuallyContains = function (node, parent) { right: parentLeft - parent.scrollLeft + parent.scrollWidth }; + var style = window.getComputedStyle(parent); + + // if parent element is inline, scrollArea will be too unpredictable + if (style.getPropertyValue('display') === 'inline') { + return true; + } + //In theory, we should just be able to look at the scroll area as a superset of the parentRect, //but that's not true in Firefox if ((rect.left < parentScrollArea.left && rect.left < parentRect.left) || @@ -36,8 +43,6 @@ dom.visuallyContains = function (node, parent) { return false; } - var style = window.getComputedStyle(parent); - if (rect.right > parentRect.right || rect.bottom > parentRect.bottom) { return (style.overflow === 'scroll' || style.overflow === 'auto' || style.overflow === 'hidden' || parent instanceof HTMLBodyElement || diff --git a/test/commons/dom/visually-contains.js b/test/commons/dom/visually-contains.js index 55f1d4e3e5..302ea6e494 100644 --- a/test/commons/dom/visually-contains.js +++ b/test/commons/dom/visually-contains.js @@ -47,4 +47,12 @@ describe('dom.visuallyContains', function () { assert.isTrue(axe.commons.dom.visuallyContains(target, target.parentNode)); }); + it('should return true when element is inline', function () { + // result depends on the display property of the element + fixture.innerHTML = ''; + var target = fixture.querySelector('#target'); + assert.isTrue(axe.commons.dom.visuallyContains(target, target.parentNode)); + }); }); From 67e250081356840215a7faa31e9d2c3f7760166c Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Mon, 15 May 2017 18:50:21 -0700 Subject: [PATCH 25/41] fix: ensure missing elms are tested for contrast PhantomJS skips label if it doesn't have a background color, and elementsFromPoint skips TR entirely. This change adds them to the elmStack Closes https://github.com/dequelabs/axe-core/issues/273 --- lib/commons/color/get-background-color.js | 33 ++++++++++ test/checks/color/color-contrast.js | 2 +- test/commons/color/get-background-color.js | 63 +++++++++++++++++-- .../rules/color-contrast/color-contrast.html | 5 +- 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 9aa6e1c3d4..7520a1b624 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -1,4 +1,5 @@ /* global axe, color, dom */ +/*jshint maxstatements: 22 */ const graphicNodes = [ 'IMG', 'CANVAS', 'OBJECT', 'IFRAME', 'VIDEO', 'SVG' ]; @@ -80,7 +81,38 @@ function elmOutsideParent(elm, bgElm, bgColor) { return outside; } +// document.elementsFromPoint misses some elements we need +// i.e. TR is missing from table elementStack and leaves out bgColor +// https://github.com/dequelabs/axe-core/issues/273 +function includeMissingElements(elmStack, elm) { + const elementMap = {'TD': 'TR', 'INPUT': 'LABEL'}; + let bgSplice = []; + elmStack.forEach(function(bgNode) { + if (bgNode.nodeType === 1) { + for (var elmTagName in elementMap) { + if (elm.tagName === elmTagName) { + let tagMatch = elementMap[elmTagName]; + if (elmStack.indexOf(tagMatch) === -1 && bgNode.tagName === elmTagName) { + // look up the tree for a parent that matches + let ancestorMatch = axe.commons.dom.findUp(bgNode, tagMatch); + if (ancestorMatch) { + bgSplice.push([elmStack.indexOf(bgNode) + 1, ancestorMatch]); + } + } + } + } + } + }); + for (var i=0; i'; var target = fixture.querySelector('#target'); var result = checks['color-contrast'].evaluate.call(checkContext, target); diff --git a/test/commons/color/get-background-color.js b/test/commons/color/get-background-color.js index c93a6691d8..4848305b11 100644 --- a/test/commons/color/get-background-color.js +++ b/test/commons/color/get-background-color.js @@ -190,6 +190,64 @@ describe('color.getBackgroundColor', function () { assert.deepEqual(bgNodes, [target]); }); + it('should count a tr as a background element', function () { + fixture.innerHTML = '
' + + '' + + '' + + '' + + '' + + '
' + + 'Cell content
'; + var target = fixture.querySelector('#target'), + parent = fixture.querySelector('#parent'); + var bgNodes = []; + var actual = axe.commons.color.getBackgroundColor(target, bgNodes); + var expected = new axe.commons.color.Color(243, 243, 243, 1); + assert.equal(actual.red, expected.red); + assert.equal(actual.green, expected.green); + assert.equal(actual.blue, expected.blue); + assert.equal(actual.alpha, expected.alpha); + assert.deepEqual(bgNodes, [parent]); + }); + + it('should handle multiple ancestors of the same name', function () { + fixture.innerHTML = '
' + + '' + + '' + + '
' + + '' + + '' + + '' + + '' + + '
' + + 'Cell content
' + + '
'; + var target = fixture.querySelector('#target'), + parent = fixture.querySelector('#parent'); + var bgNodes = []; + var actual = axe.commons.color.getBackgroundColor(target, bgNodes); + var expected = new axe.commons.color.Color(243, 243, 243, 1); + assert.equal(actual.red, expected.red); + assert.equal(actual.green, expected.green); + assert.equal(actual.blue, expected.blue); + assert.equal(actual.alpha, expected.alpha); + assert.deepEqual(bgNodes, [parent]); + }); + + it('should count an implicit label as a background element', function () { + fixture.innerHTML = ''; + var target = fixture.querySelector('#target'); + var bgNodes = []; + var actual = axe.commons.color.getBackgroundColor(target, bgNodes); + var expected = new axe.commons.color.Color(255, 255, 255, 1); + assert.equal(actual.red, expected.red); + assert.equal(actual.green, expected.green); + assert.equal(actual.blue, expected.blue); + assert.equal(actual.alpha, expected.alpha); + }); + it('should use hierarchical DOM traversal if possible', function () { fixture.innerHTML = '
elm
'; + fixture.innerHTML = '
'; var orig = document.documentElement.style.background; document.documentElement.style.background = '#0F0'; @@ -406,5 +462,4 @@ describe('color.getBackgroundColor', function () { assert.closeTo(actual.alpha, expected.alpha, 0.1); }); - }); diff --git a/test/integration/rules/color-contrast/color-contrast.html b/test/integration/rules/color-contrast/color-contrast.html index 43e349d32b..cd52ddba5f 100644 --- a/test/integration/rules/color-contrast/color-contrast.html +++ b/test/integration/rules/color-contrast/color-contrast.html @@ -8,7 +8,10 @@
Pass.
- +
This is a fail.
This is a fail.
From 09c8a2c324c3781c4411420e0fcc4cb0a08f41ba Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Tue, 6 Jun 2017 14:16:03 -0700 Subject: [PATCH 26/41] chore: rename elmOutsideParent to be more accurate --- lib/checks/color/color-contrast.json | 2 +- lib/commons/color/get-background-color.js | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/checks/color/color-contrast.json b/lib/checks/color/color-contrast.json index 6a768eca1a..561213c39b 100644 --- a/lib/checks/color/color-contrast.json +++ b/lib/checks/color/color-contrast.json @@ -12,7 +12,7 @@ "imgNode": "Element's background color could not be determined because element contains an image node", "bgOverlap": "Element's background color could not be determined because it is overlapped by another element", "fgAlpha" : "Element's foreground color could not be determined because of alpha transparency", - "elmOutsideParent": "Element's background color could not be determined because it extends beyond a parent node", + "elmPartiallyObscured": "Element's background color could not be determined because it's partially obscured by another element", "default": "Unable to determine contrast ratio" } } diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 7520a1b624..33cd48517b 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -70,15 +70,12 @@ function calculateObscuringAlpha(elmIndex, elmStack, originalElm) { return totalAlpha; } -function elmOutsideParent(elm, bgElm, bgColor) { - var outside = (elm !== bgElm && !dom.visuallyContains(elm, bgElm) && bgColor.alpha !== 0); - if (outside) { - axe.commons.color.incompleteData.set('bgColor', { - node: elm, - reason: 'elmOutsideParent' - }); +function elmPartiallyObscured(elm, bgElm, bgColor) { + var obscured = (elm !== bgElm && !dom.visuallyContains(elm, bgElm) && bgColor.alpha !== 0); + if (obscured) { + axe.commons.color.incompleteData.set('bgColor', 'elmPartiallyObscured'); } - return outside; + return obscured; } // document.elementsFromPoint misses some elements we need @@ -176,8 +173,8 @@ color.getBackgroundColor = function(elm, bgElms = [], noScroll = false) { // Get the background color let bgColor = getBgColor(bgElm, bgElmStyle); - if (// abort if a node is outside it's parent and its parent has a background - elmOutsideParent(elm, bgElm, bgColor) || + if (// abort if a node is partially obscured and obscuring element has a background + elmPartiallyObscured(elm, bgElm, bgColor) || // OR if the background elm is a graphic elmHasImage(bgElm, bgElmStyle) ) { From 62f0948141a2719a7fbc70df7ed7865c352f1a31 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Wed, 7 Jun 2017 09:22:51 -0700 Subject: [PATCH 27/41] chore: improve content overlap checking Addresses both the browser and PhantomJS --- lib/commons/color/get-background-color.js | 56 ++++++++------- test/checks/color/color-contrast.js | 15 +++- test/commons/color/get-background-color.js | 71 ++++++++++++++----- .../rules/color-contrast/color-contrast.html | 7 ++ .../rules/color-contrast/color-contrast.json | 4 +- 5 files changed, 104 insertions(+), 49 deletions(-) diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 33cd48517b..0eee3b541e 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -39,13 +39,17 @@ function getBgColor(elm, elmStyle) { return bgColor; } -function contentOverlapped(targetElement) { - // Get dimensions of text content box. - // Subsequent clientRects are bounding boxes, if different than content box +function contentOverlapping(targetElement, bgNode) { + // get content box of target element + // check to see if the current bgNode is on top var targetRect = targetElement.getClientRects()[0]; - var elementAtPoint = document.elementFromPoint(targetRect.left, targetRect.top); - if (elementAtPoint !== null && elementAtPoint.innerHTML !== targetElement.innerHTML) { - return true; + var obscuringElements = document.elementsFromPoint(targetRect.left, targetRect.top); + if (obscuringElements) { + for(var i = 0; i < obscuringElements.length; i++) { + if (obscuringElements[i] !== targetElement && obscuringElements[i] === bgNode) { + return true; + } + } } return false; } @@ -59,7 +63,7 @@ function calculateObscuringAlpha(elmIndex, elmStack, originalElm) { let bgElm = elmStack[i]; let bgElmStyle = window.getComputedStyle(bgElm); let bgColor = getBgColor(bgElm, bgElmStyle); - if (bgColor.alpha && contentOverlapped(originalElm, bgElm)) { + if (bgColor.alpha && contentOverlapping(originalElm, bgElm)) { totalAlpha += bgColor.alpha; } else { // remove elements not contributing to the background @@ -83,30 +87,28 @@ function elmPartiallyObscured(elm, bgElm, bgColor) { // https://github.com/dequelabs/axe-core/issues/273 function includeMissingElements(elmStack, elm) { const elementMap = {'TD': 'TR', 'INPUT': 'LABEL'}; - let bgSplice = []; - elmStack.forEach(function(bgNode) { - if (bgNode.nodeType === 1) { - for (var elmTagName in elementMap) { - if (elm.tagName === elmTagName) { - let tagMatch = elementMap[elmTagName]; - if (elmStack.indexOf(tagMatch) === -1 && bgNode.tagName === elmTagName) { - // look up the tree for a parent that matches - let ancestorMatch = axe.commons.dom.findUp(bgNode, tagMatch); - if (ancestorMatch) { - bgSplice.push([elmStack.indexOf(bgNode) + 1, ancestorMatch]); - } + const tagArray = elmStack.map((elm) => { + return elm.tagName; + }); + for (let candidate in elementMap) { + if (elementMap.hasOwnProperty(candidate)) { + // tagName matches key + if (elm.tagName === candidate) { + let ancestorMatch = axe.commons.dom.findUp(elm, elementMap[candidate]); + if (ancestorMatch && elmStack.indexOf(ancestorMatch) === -1) { + // found an ancestor not in elmStack, and it overlaps + let overlaps = axe.commons.dom.visuallyOverlaps(elm.getBoundingClientRect(), ancestorMatch); + if (overlaps) { + elmStack.splice(elmStack.indexOf(elm) + 1, 0, ancestorMatch); } } } + // tagName matches value + // (such as LABEL, when matching itself. It should be in the list, but Phantom skips it) + if (elm.tagName === elementMap[candidate] && tagArray.indexOf(elm.tagName) === -1) { + elmStack.splice(tagArray.indexOf(candidate) + 1, 0, elm); + } } - }); - for (var i=0; i'; var target = fixture.querySelector('#target'); - assert.isUndefined(checks['color-contrast'].evaluate.call(checkContext, target)); - assert.equal(checkContext._data.missingData[0].reason, 'bgOverlap'); + var result = checks['color-contrast'].evaluate.call(checkContext, target); + assert.isUndefined(result); + assert.equal(checkContext._data.missingData, 'bgOverlap'); assert.equal(checkContext._data.contrastRatio, 0); }); @@ -181,6 +182,14 @@ describe('color-contrast', function () { assert.isTrue(result); }); + it('should return true when a label wraps a text input but doesn\'t overlap', function () { + fixture.innerHTML = ''; + var target = fixture.querySelector('#target'); + var result = checks['color-contrast'].evaluate.call(checkContext, target); + assert.isTrue(result); + }); + it('should return undefined if element overlaps text content', function () { fixture.innerHTML = '
' + '
Hi
' + @@ -189,7 +198,7 @@ describe('color-contrast', function () { var target = fixture.querySelector('#target'); var actual = checks['color-contrast'].evaluate.call(checkContext, target); assert.isUndefined(actual); - assert.equal(checkContext._data.missingData[0].reason, 'bgOverlap'); + assert.equal(checkContext._data.missingData, 'bgOverlap'); assert.equal(checkContext._data.contrastRatio, 0); }); }); diff --git a/test/commons/color/get-background-color.js b/test/commons/color/get-background-color.js index 4848305b11..6ebc85a531 100644 --- a/test/commons/color/get-background-color.js +++ b/test/commons/color/get-background-color.js @@ -5,6 +5,7 @@ describe('color.getBackgroundColor', function () { afterEach(function () { document.getElementById('fixture').innerHTML = ''; + axe.commons.color.incompleteData.clear(); }); it('should return the blended color if it has no background set', function () { @@ -190,11 +191,11 @@ describe('color.getBackgroundColor', function () { assert.deepEqual(bgNodes, [target]); }); - it('should count a tr as a background element', function () { - fixture.innerHTML = '
' + + it('should count a TR as a background element', function () { + fixture.innerHTML = '
' + '' + - '' + - '' + + '' + '' + '
' + + '
' + 'Cell content
'; @@ -210,6 +211,54 @@ describe('color.getBackgroundColor', function () { assert.deepEqual(bgNodes, [parent]); }); + it('should ignore TR elements that don\'t overlap', function () { + fixture.innerHTML = '' + + '' + + ''+ + '
Content
'; + var bgNodes = []; + var target = fixture.querySelector('#target'); + var parent = fixture.querySelector('#parent'); + var actual = axe.commons.color.getBackgroundColor(target, bgNodes); + var expected = new axe.commons.color.Color(255, 255, 255, 1); + assert.equal(actual.red, expected.red); + assert.equal(actual.green, expected.green); + assert.equal(actual.blue, expected.blue); + assert.equal(actual.alpha, expected.alpha); + assert.notEqual(bgNodes, [parent]); + }); + + it('should count an implicit label as a background element', function () { + fixture.innerHTML = ''; + var target = fixture.querySelector('#target'); + var bgNodes = []; + var actual = axe.commons.color.getBackgroundColor(target, bgNodes); + var expected = new axe.commons.color.Color(255, 255, 255, 1); + assert.equal(actual.red, expected.red); + assert.equal(actual.green, expected.green); + assert.equal(actual.blue, expected.blue); + assert.equal(actual.alpha, expected.alpha); + }); + + it('should ignore inline ancestors of non-overlapping elements', function () { + fixture.innerHTML = '
'+ + '
'; + var target = fixture.querySelector('#target'); + var parent = fixture.querySelector('#parent'); + var bgNodes = []; + var actual = axe.commons.color.getBackgroundColor(target, bgNodes); + var expected = new axe.commons.color.Color(255, 255, 255, 1); + assert.equal(actual.red, expected.red); + assert.equal(actual.green, expected.green); + assert.equal(actual.blue, expected.blue); + assert.equal(actual.alpha, expected.alpha); + assert.notEqual(bgNodes, [parent]); + }); + it('should handle multiple ancestors of the same name', function () { fixture.innerHTML = '
' + '' + @@ -234,20 +283,6 @@ describe('color.getBackgroundColor', function () { assert.deepEqual(bgNodes, [parent]); }); - it('should count an implicit label as a background element', function () { - fixture.innerHTML = ''; - var target = fixture.querySelector('#target'); - var bgNodes = []; - var actual = axe.commons.color.getBackgroundColor(target, bgNodes); - var expected = new axe.commons.color.Color(255, 255, 255, 1); - assert.equal(actual.red, expected.red); - assert.equal(actual.green, expected.green); - assert.equal(actual.blue, expected.blue); - assert.equal(actual.alpha, expected.alpha); - }); - it('should use hierarchical DOM traversal if possible', function () { fixture.innerHTML = '
+
+ +
+
This is a fail.
This is a fail.
Fail.
diff --git a/test/integration/rules/color-contrast/color-contrast.json b/test/integration/rules/color-contrast/color-contrast.json index 80ec9a8eba..98245f3c4b 100644 --- a/test/integration/rules/color-contrast/color-contrast.json +++ b/test/integration/rules/color-contrast/color-contrast.json @@ -29,7 +29,9 @@ ["#pass4"], ["#pass5"], ["#pass6"], - ["#pass6 > input"] + ["#pass6 > input"], + ["#pass7"], + ["#pass7 > input"] ], "incomplete": [ ["#canttell1"], From e851a3a14c0706340eb0097d0335d51bb198dc1f Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Thu, 8 Jun 2017 13:47:21 -0700 Subject: [PATCH 28/41] chore: refactor to avoid maxstatements limit --- lib/commons/color/get-background-color.js | 45 ++++++++++++++--------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 0eee3b541e..7c69bdf4b0 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -1,5 +1,4 @@ /* global axe, color, dom */ -/*jshint maxstatements: 22 */ const graphicNodes = [ 'IMG', 'CANVAS', 'OBJECT', 'IFRAME', 'VIDEO', 'SVG' ]; @@ -82,9 +81,12 @@ function elmPartiallyObscured(elm, bgElm, bgColor) { return obscured; } -// document.elementsFromPoint misses some elements we need -// i.e. TR is missing from table elementStack and leaves out bgColor -// https://github.com/dequelabs/axe-core/issues/273 +/** + * Include nodes missing from initial gathering because + * document.elementsFromPoint misses some elements we need + * i.e. TR is missing from table elementStack and leaves out bgColor + * https://github.com/dequelabs/axe-core/issues/273 + */ function includeMissingElements(elmStack, elm) { const elementMap = {'TD': 'TR', 'INPUT': 'LABEL'}; const tagArray = elmStack.map((elm) => { @@ -112,6 +114,26 @@ function includeMissingElements(elmStack, elm) { } } +/** + * Look at document and body elements for relevant background information + */ +function consultDocumentBody(elmStack) { + let bodyIndex = elmStack.indexOf(document.body); + + if (// Check that the body background is the page's background + bodyIndex > 1 && // only if there are negative z-index elements + !elmHasImage(document.documentElement) && + getBgColor(document.documentElement).alpha === 0 + ) { + // Remove body and html from it's current place + elmStack.splice(bodyIndex, 1); + elmStack.splice( elmStack.indexOf(document.documentElement), 1); + + // Put the body background as the lowest element + elmStack.push(document.body); + } +} + /** * Get all elements rendered underneath the current element, * in the order in which it is rendered @@ -136,20 +158,7 @@ color.getBackgroundStack = function(elm) { includeMissingElements(elmStack, elm); elmStack = dom.reduceToElementsBelowFloating(elmStack, elm); - let bodyIndex = elmStack.indexOf(document.body); - - if (// Check that the body background is the page's background - bodyIndex > 1 && // only if there are negative z-index elements - !elmHasImage(document.documentElement) && - getBgColor(document.documentElement).alpha === 0 - ) { - // Remove body and html from it's current place - elmStack.splice(bodyIndex, 1); - elmStack.splice( elmStack.indexOf(document.documentElement), 1); - - // Put the body background as the lowest element - elmStack.push(document.body); - } + consultDocumentBody(elmStack); // Return all elements BELOW the current element, null if the element is undefined let elmIndex = elmStack.indexOf(elm); From f84994511cc3c21469da60b8b43904f48569b4ad Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Thu, 8 Jun 2017 14:16:38 -0700 Subject: [PATCH 29/41] chore: add function comments --- lib/commons/color/get-background-color.js | 41 +++++++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 7c69bdf4b0..674e3b31cf 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -3,6 +3,13 @@ const graphicNodes = [ 'IMG', 'CANVAS', 'OBJECT', 'IFRAME', 'VIDEO', 'SVG' ]; +/** + * Reports if an element has a background image or gradient + * @private + * @param {Element} elm + * @param {Object|null} style + * @return {Boolean} + */ function elmHasImage(elm, style) { var nodeName = elm.nodeName.toUpperCase(); if (graphicNodes.includes(nodeName)) { @@ -22,6 +29,7 @@ function elmHasImage(elm, style) { /** * Returns the non-alpha-blended background color of an element + * @private * @param {Element} elm * @return {Color} */ @@ -38,9 +46,16 @@ function getBgColor(elm, elmStyle) { return bgColor; } +/** + * Determines overlap of node's content with a bgNode. Used for inline elements + * @private + * @param {Element} targetElement + * @param {Element} bgNode + * @return {Boolean} + */ function contentOverlapping(targetElement, bgNode) { // get content box of target element - // check to see if the current bgNode is on top + // check to see if the current bgNode is overlapping var targetRect = targetElement.getClientRects()[0]; var obscuringElements = document.elementsFromPoint(targetRect.left, targetRect.top); if (obscuringElements) { @@ -52,7 +67,14 @@ function contentOverlapping(targetElement, bgNode) { } return false; } - +/** + * Calculate alpha transparency of a background element obscuring the current node + * @private + * @param {Number} elmIndex + * @param {Array} elmStack + * @param {Element} originalElm + * @return {Number|undefined} + */ function calculateObscuringAlpha(elmIndex, elmStack, originalElm) { var totalAlpha = 0; @@ -72,7 +94,14 @@ function calculateObscuringAlpha(elmIndex, elmStack, originalElm) { } return totalAlpha; } - +/** + * Determine if element is partially overlapped, triggering a Can't Tell result + * @private + * @param {Element} elm + * @param {Element} bgElm + * @param {Object} bgColor + * @return {Boolean} + */ function elmPartiallyObscured(elm, bgElm, bgColor) { var obscured = (elm !== bgElm && !dom.visuallyContains(elm, bgElm) && bgColor.alpha !== 0); if (obscured) { @@ -86,6 +115,10 @@ function elmPartiallyObscured(elm, bgElm, bgColor) { * document.elementsFromPoint misses some elements we need * i.e. TR is missing from table elementStack and leaves out bgColor * https://github.com/dequelabs/axe-core/issues/273 + * + * @private + * @param {Array} elmStack + * @param {Element} elm */ function includeMissingElements(elmStack, elm) { const elementMap = {'TD': 'TR', 'INPUT': 'LABEL'}; @@ -116,6 +149,8 @@ function includeMissingElements(elmStack, elm) { /** * Look at document and body elements for relevant background information + * @private + * @param {Array} elmStack */ function consultDocumentBody(elmStack) { let bodyIndex = elmStack.indexOf(document.body); From ad0794ce6a60c1d2852388e783c17ab37a01b407 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Fri, 9 Jun 2017 14:02:51 -0700 Subject: [PATCH 30/41] fix: don't mutate elmStack --- lib/commons/color/get-background-color.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 674e3b31cf..1896909538 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -125,6 +125,7 @@ function includeMissingElements(elmStack, elm) { const tagArray = elmStack.map((elm) => { return elm.tagName; }); + let bgNodes = elmStack; for (let candidate in elementMap) { if (elementMap.hasOwnProperty(candidate)) { // tagName matches key @@ -134,17 +135,18 @@ function includeMissingElements(elmStack, elm) { // found an ancestor not in elmStack, and it overlaps let overlaps = axe.commons.dom.visuallyOverlaps(elm.getBoundingClientRect(), ancestorMatch); if (overlaps) { - elmStack.splice(elmStack.indexOf(elm) + 1, 0, ancestorMatch); + bgNodes.splice(elmStack.indexOf(elm) + 1, 0, ancestorMatch); } } } // tagName matches value // (such as LABEL, when matching itself. It should be in the list, but Phantom skips it) if (elm.tagName === elementMap[candidate] && tagArray.indexOf(elm.tagName) === -1) { - elmStack.splice(tagArray.indexOf(candidate) + 1, 0, elm); + bgNodes.splice(tagArray.indexOf(candidate) + 1, 0, elm); } } } + return bgNodes; } /** @@ -152,21 +154,24 @@ function includeMissingElements(elmStack, elm) { * @private * @param {Array} elmStack */ -function consultDocumentBody(elmStack) { +function sortPageBackground(elmStack) { let bodyIndex = elmStack.indexOf(document.body); + let bgNodes = elmStack; + if (// Check that the body background is the page's background bodyIndex > 1 && // only if there are negative z-index elements !elmHasImage(document.documentElement) && getBgColor(document.documentElement).alpha === 0 ) { // Remove body and html from it's current place - elmStack.splice(bodyIndex, 1); - elmStack.splice( elmStack.indexOf(document.documentElement), 1); + bgNodes.splice(bodyIndex, 1); + bgNodes.splice( elmStack.indexOf(document.documentElement), 1); // Put the body background as the lowest element - elmStack.push(document.body); + bgNodes.push(document.body); } + return bgNodes; } /** @@ -190,10 +195,9 @@ color.getBackgroundStack = function(elm) { window.innerHeight - 1); let elmStack = document.elementsFromPoint(x, y); - includeMissingElements(elmStack, elm); + elmStack = includeMissingElements(elmStack, elm); elmStack = dom.reduceToElementsBelowFloating(elmStack, elm); - - consultDocumentBody(elmStack); + elmStack = sortPageBackground(elmStack); // Return all elements BELOW the current element, null if the element is undefined let elmIndex = elmStack.indexOf(elm); From 9feb74a3044dc9171d5b494ccdb4d0e73b9e8b67 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Mon, 12 Jun 2017 14:24:51 -0700 Subject: [PATCH 31/41] feat: put 1:1 ratio contrast into incomplete --- lib/checks/color/color-contrast.js | 10 ++++- lib/checks/color/color-contrast.json | 1 + lib/commons/color/incomplete-data.js | 3 +- test/checks/color/color-contrast.js | 11 +++++ test/commons/color/contrast.js | 8 +++- .../rules/color-contrast/color-contrast.html | 41 ++++++++++--------- .../rules/color-contrast/color-contrast.json | 36 ++++++++-------- 7 files changed, 68 insertions(+), 42 deletions(-) diff --git a/lib/checks/color/color-contrast.js b/lib/checks/color/color-contrast.js index 96f336cd3d..b59a49103d 100644 --- a/lib/checks/color/color-contrast.js +++ b/lib/checks/color/color-contrast.js @@ -23,6 +23,12 @@ var missing; if (bgColor === null) { missing = axe.commons.color.incompleteData.get('bgColor'); } + +let equalRatio = false; +if (truncatedResult === 1) { + equalRatio = true; + missing = axe.commons.color.incompleteData.set('bgColor', 'equalRatio'); +} // need both independently in case both are missing var data = { fgColor: fgColor ? fgColor.toHexString() : undefined, @@ -35,12 +41,12 @@ var data = { this.data(data); -if (!cr.isValid) { +if (!cr.isValid || equalRatio) { this.relatedNodes(bgNodes); } //We don't know, so we'll put it into Can't Tell -if (fgColor === null || bgColor === null) { +if (fgColor === null || bgColor === null || equalRatio) { missing = null; axe.commons.color.incompleteData.clear(); return undefined; diff --git a/lib/checks/color/color-contrast.json b/lib/checks/color/color-contrast.json index 561213c39b..09dc0acc3b 100644 --- a/lib/checks/color/color-contrast.json +++ b/lib/checks/color/color-contrast.json @@ -13,6 +13,7 @@ "bgOverlap": "Element's background color could not be determined because it is overlapped by another element", "fgAlpha" : "Element's foreground color could not be determined because of alpha transparency", "elmPartiallyObscured": "Element's background color could not be determined because it's partially obscured by another element", + "equalRatio": "Element has a 1:1 contrast ratio with the background", "default": "Unable to determine contrast ratio" } } diff --git a/lib/commons/color/incomplete-data.js b/lib/commons/color/incomplete-data.js index b12a2737c4..288f61457d 100644 --- a/lib/commons/color/incomplete-data.js +++ b/lib/commons/color/incomplete-data.js @@ -15,9 +15,10 @@ color.incompleteData = (function() { if (typeof key !== 'string') { throw new Error('Incomplete data: key must be a string'); } - if (reason){ + if (reason) { data[key] = reason; } + return data[key]; }, /** * Get incomplete data by key diff --git a/test/checks/color/color-contrast.js b/test/checks/color/color-contrast.js index 0bcb63bb29..82b9ab56e2 100644 --- a/test/checks/color/color-contrast.js +++ b/test/checks/color/color-contrast.js @@ -201,4 +201,15 @@ describe('color-contrast', function () { assert.equal(checkContext._data.missingData, 'bgOverlap'); assert.equal(checkContext._data.contrastRatio, 0); }); + + it('should return undefined if element has same color as background', function () { + fixture.innerHTML = '
' + + '
Text
'+ + '
'; + var target = fixture.querySelector('#target'); + var actual = checks['color-contrast'].evaluate.call(checkContext, target); + assert.isUndefined(actual); + assert.equal(checkContext._data.missingData, 'equalRatio'); + assert.equal(checkContext._data.contrastRatio, 1); + }); }); diff --git a/test/commons/color/contrast.js b/test/commons/color/contrast.js index 5980145538..c9c4ebf19b 100644 --- a/test/commons/color/contrast.js +++ b/test/commons/color/contrast.js @@ -114,8 +114,6 @@ describe('color.Color', function () { assert.isTrue(axe.commons.color.hasValidContrastRatio(black, white, 8, false).isValid); assert.isTrue(axe.commons.color.hasValidContrastRatio(black, white, 8, false).contrastRatio > 4.5); - assert.isFalse(axe.commons.color.hasValidContrastRatio(black, black, 16, true).isValid); - assert.isTrue(axe.commons.color.hasValidContrastRatio(black, black, 16, true).contrastRatio < 3); assert.isTrue(axe.commons.color.hasValidContrastRatio(white, gray, 24, false).isValid); assert.isTrue(axe.commons.color.hasValidContrastRatio(white, gray, 24, false).contrastRatio > 3); assert.isTrue(axe.commons.color.hasValidContrastRatio(white, gray, 20, true).isValid); @@ -124,4 +122,10 @@ describe('color.Color', function () { assert.isTrue(axe.commons.color.hasValidContrastRatio(white, gray, 8, false).contrastRatio < 4.5); }); + it('should count 1-1 ratios as visually hidden', function () { + var black = new axe.commons.color.Color(0, 0, 0, 1); + + assert.isFalse(axe.commons.color.hasValidContrastRatio(black, black, 16, true).isValid); + assert.isTrue(axe.commons.color.hasValidContrastRatio(black, black, 16, true).contrastRatio === 1); + }); }); diff --git a/test/integration/rules/color-contrast/color-contrast.html b/test/integration/rules/color-contrast/color-contrast.html index 2d8eb7ab6d..4bf4e3d40d 100644 --- a/test/integration/rules/color-contrast/color-contrast.html +++ b/test/integration/rules/color-contrast/color-contrast.html @@ -22,26 +22,8 @@
This is a fail.
This is a fail.
-
Fail.
-
Hi
-
This is a fail.
-
Hi
- - - - - - - - - - - - - +
This is a fail.
@@ -80,3 +62,24 @@
Element overlap
+
1:1 ratio with transparency
+ + + + + + + + + + + + + + + +
Hi
+ +
Hi
diff --git a/test/integration/rules/color-contrast/color-contrast.json b/test/integration/rules/color-contrast/color-contrast.json index 98245f3c4b..2b27242019 100644 --- a/test/integration/rules/color-contrast/color-contrast.json +++ b/test/integration/rules/color-contrast/color-contrast.json @@ -5,22 +5,7 @@ ["#fail1"], ["#fail2"], ["#fail3"], - ["#fail4"], - ["#fail5"], - ["#fail6"], - ["#fail7"], - ["#fail8"], - ["#fail9"], - ["#fail10"], - ["#fail11"], - ["#fail12"], - ["#fail13"], - ["#fail14"], - ["#fail15"], - ["#fail16"], - ["#fail17"], - ["#fail18"], - ["#fail19"] + ["#fail6"] ], "passes": [ ["#pass1"], @@ -29,7 +14,7 @@ ["#pass4"], ["#pass5"], ["#pass6"], - ["#pass6 > input"], + ["#pass6 > input[type=\"text\"]"], ["#pass7"], ["#pass7 > input"] ], @@ -37,6 +22,21 @@ ["#canttell1"], ["#canttell2"], ["#canttell3"], - ["#canttell4"] + ["#canttell4"], + ["#canttell5"], + ["#canttell6"], + ["#canttell7"], + ["#canttell8"], + ["#canttell9"], + ["#canttell10"], + ["#canttell11"], + ["#canttell12"], + ["#canttell13"], + ["#canttell14"], + ["#canttell15"], + ["#canttell16"], + ["#canttell17"], + ["#canttell18"], + ["#canttell19"] ] } From fc51c57e6dc43bc290eff3eea215c5309ee24a7f Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Tue, 13 Jun 2017 12:15:10 -0700 Subject: [PATCH 32/41] fix: change impact of color-contrast to serious --- lib/checks/color/color-contrast.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/color/color-contrast.json b/lib/checks/color/color-contrast.json index 09dc0acc3b..578a0db2c4 100644 --- a/lib/checks/color/color-contrast.json +++ b/lib/checks/color/color-contrast.json @@ -2,7 +2,7 @@ "id": "color-contrast", "evaluate": "color-contrast.js", "metadata": { - "impact": "critical", + "impact": "serious", "messages": { "pass": "Element has sufficient color contrast of {{=it.data.contrastRatio}}", "fail": "Element has insufficient color contrast of {{=it.data.contrastRatio}} (foreground color: {{=it.data.fgColor}}, background color: {{=it.data.bgColor}}, font size: {{=it.data.fontSize}}, font weight: {{=it.data.fontWeight}})", From e02753edb813c8577d6ed8069f50123b18512c94 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Mon, 12 Jun 2017 14:08:21 -0700 Subject: [PATCH 33/41] fix: update copy in href-no-hash best practice rule --- lib/checks/navigation/href-no-hash.json | 4 ++-- lib/rules/href-no-hash.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checks/navigation/href-no-hash.json b/lib/checks/navigation/href-no-hash.json index 2652999aaa..b37fd57900 100644 --- a/lib/checks/navigation/href-no-hash.json +++ b/lib/checks/navigation/href-no-hash.json @@ -4,8 +4,8 @@ "metadata": { "impact": "moderate", "messages": { - "pass": "Anchor does not have a href quals #", - "fail": "Anchor has a href quals #" + "pass": "Anchor does not have an href value of #", + "fail": "Anchor has an href value of #" } } } diff --git a/lib/rules/href-no-hash.json b/lib/rules/href-no-hash.json index 3852ac553d..e32ed5ed79 100644 --- a/lib/rules/href-no-hash.json +++ b/lib/rules/href-no-hash.json @@ -8,7 +8,7 @@ ], "metadata": { "description": "Ensures that href values are valid link references to promote only using anchors as links", - "help": "Anchors must only be used as links and must therefore have an href value that is a valid reference. Otherwise you should probably usa a button" + "help": "Anchors must only be used as links with valid URLs or URL fragments" }, "all": [], "any": [ From bbe323299e49bba3dda9fa41498add63c34ceb54 Mon Sep 17 00:00:00 2001 From: Harris Schneiderman Date: Wed, 14 Jun 2017 12:27:14 -0400 Subject: [PATCH 34/41] doc: fix readme typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ba14767284..d53dd62834 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ The [aXe API](doc/API.md) package consists of: ## Localization -Axe can be build using your local language. To do so, a localization file must be added to the `./locales` directory. This file must have be named in the following manner: `.json`. To build aXe using this locale, instead of the default, run aXe with the `--lang` flag, like so: +Axe can be built using your local language. To do so, a localization file must be added to the `./locales` directory. This file must have be named in the following manner: `.json`. To build aXe using this locale, instead of the default, run aXe with the `--lang` flag, like so: `grunt build --lang=nl` From 948c6a2dbe0f81ff243e0989efc386d0b360d370 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Tue, 13 Jun 2017 13:02:20 -0700 Subject: [PATCH 35/41] doc: explain category mappings --- doc/API.md | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/doc/API.md b/doc/API.md index be76d43bbb..4043e758ba 100644 --- a/doc/API.md +++ b/doc/API.md @@ -77,12 +77,30 @@ Returns a list of all rules with their ID and description The current set of tags supported are listed in the following table: -| Tag Name | Accessibility Standard | -|--------------------|:-------------------------------------:| -| `wcag2a` | WCAG 2.0 Level A | -| `wcag2aa` | WCAG 2.0 Level AA | -| `section508` | Section 508 | -| `best-practice` | Best practices endorsed by Deque | +| Tag Name | Accessibility Standard/Purpose | +|--------------------|:-------------------------------------------:| +| `wcag2a` | WCAG 2.0 Level A | +| `wcag2aa` | WCAG 2.0 Level AA | +| `section508` | Section 508 | +| `best-practice` | Best practices endorsed by Deque | +| `experimental` | Cutting-edge techniques | +| `cat` | Category mappings used by Deque (see below) | + +| Category name | +|-------------------------------| +| `cat.aria` | +| `cat.color` | +| `cat.forms` | +| `cat.keyboard` | +| `cat.language` | +| `cat.name-role-value` | +| `cat.parsing` | +| `cat.semantics` | +| `cat.sensory-and-visual-cues` | +| `cat.structure` | +| `cat.tables` | +| `cat.text-alternatives` | +| `cat.time-and-media` | #### Example 1 From cc1428fa3e143fdc526a7e527bf52b5a0c64ec75 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 14 Jun 2017 13:24:49 +0200 Subject: [PATCH 36/41] fix: Update axe.source to work with Firefox webdriver --- lib/core/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/index.js b/lib/core/index.js index e2b5028ad3..4ecf70e9e5 100644 --- a/lib/core/index.js +++ b/lib/core/index.js @@ -11,7 +11,7 @@ if (typeof define === 'function' && define.amd) { }); } if (typeof module === 'object' && module.exports && typeof axeFunction.toString === 'function') { - axe.source = '(' + axeFunction.toString() + ')(this, this.document);'; + axe.source = '(' + axeFunction.toString() + ')(typeof window === "object" ? window : this);'; module.exports = axe; } if (typeof window.getComputedStyle === 'function') { From 0df5c0cb09a834853f24bd1526cb6f12e5649032 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Wed, 14 Jun 2017 11:23:36 -0700 Subject: [PATCH 37/41] chore: bump version to 2.2.4 --- CHANGELOG | 13 ++++++++++++- axe.d.ts | 2 +- package.json | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b7212e209a..830abac269 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -86,4 +86,15 @@ v2.2.2: v2.2.3: date: 2017-06-01 changes: - - Removed the disable property from link-in-text-block \ No newline at end of file + - Removed the disable property from link-in-text-block +v2.2.4: + date: 2017-06-14 + changes: + - Overhaul of selectors API + - New experimental rule for hidden content + - New rule for aria-hidden="true" on the body + - Color-contrast rule impact is now serious + - Color-contrast fixes for implicit labels and TR elements + - Color-contrast puts 1:1 ratio elements into Needs Review/incomplete + - List category mappings in docs + - Update axe.source to work with Firefox webdriver diff --git a/axe.d.ts b/axe.d.ts index 7cedb09023..5d08c544ff 100644 --- a/axe.d.ts +++ b/axe.d.ts @@ -1,4 +1,4 @@ -// Type definitions for axe-core 2.2.3 +// Type definitions for axe-core 2.2.4 // Project: https://github.com/dequelabs/axe-core // Definitions by: Marcy Sutton diff --git a/package.json b/package.json index d0d0e0bb62..b78a0a4076 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "axe-core", "description": "Accessibility engine for automated Web UI testing", - "version": "2.2.3", + "version": "2.2.4", "license": "MPL-2.0", "contributors": [ { From 6a418d71ec7a8d5abe4b22b8e440d35925ff3e13 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Wed, 14 Jun 2017 12:07:18 -0700 Subject: [PATCH 38/41] chore: bump version to 2.3 Due to new rules and a changed impact --- CHANGELOG | 2 +- axe.d.ts | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 830abac269..1a61a87b91 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -87,7 +87,7 @@ v2.2.3: date: 2017-06-01 changes: - Removed the disable property from link-in-text-block -v2.2.4: +v2.3: date: 2017-06-14 changes: - Overhaul of selectors API diff --git a/axe.d.ts b/axe.d.ts index 5d08c544ff..f976b8698f 100644 --- a/axe.d.ts +++ b/axe.d.ts @@ -1,4 +1,4 @@ -// Type definitions for axe-core 2.2.4 +// Type definitions for axe-core 2.3 // Project: https://github.com/dequelabs/axe-core // Definitions by: Marcy Sutton diff --git a/package.json b/package.json index b78a0a4076..a630cc38ea 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "axe-core", "description": "Accessibility engine for automated Web UI testing", - "version": "2.2.4", + "version": "2.3", "license": "MPL-2.0", "contributors": [ { From d1de79b676234157f474e4a8613ca08106ef1024 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Wed, 14 Jun 2017 12:11:14 -0700 Subject: [PATCH 39/41] chore: fix version number for 2.3.0 --- CHANGELOG | 2 +- axe.d.ts | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 1a61a87b91..7396196875 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -87,7 +87,7 @@ v2.2.3: date: 2017-06-01 changes: - Removed the disable property from link-in-text-block -v2.3: +v2.3.0: date: 2017-06-14 changes: - Overhaul of selectors API diff --git a/axe.d.ts b/axe.d.ts index f976b8698f..8203d5c4d6 100644 --- a/axe.d.ts +++ b/axe.d.ts @@ -1,4 +1,4 @@ -// Type definitions for axe-core 2.3 +// Type definitions for axe-core 2.3.0 // Project: https://github.com/dequelabs/axe-core // Definitions by: Marcy Sutton diff --git a/package.json b/package.json index a630cc38ea..76eef057f5 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "axe-core", "description": "Accessibility engine for automated Web UI testing", - "version": "2.3", + "version": "2.3.0", "license": "MPL-2.0", "contributors": [ { From fd959296f5e33b5d49133e888563825f12a02a88 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Wed, 14 Jun 2017 12:19:40 -0700 Subject: [PATCH 40/41] chore: update rule-description for aria-hidden-body --- doc/rule-descriptions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index d82fa663db..ff0380d11b 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -3,7 +3,7 @@ | accesskeys | Ensures every accesskey attribute value is unique | wcag2a, wcag211, cat.keyboard | true | | area-alt | Ensures <area> elements of image maps have alternate text | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | true | | aria-allowed-attr | Ensures ARIA attributes are allowed for an element's role | cat.aria, wcag2a, wcag411, wcag412 | true | -| aria-hidden-body | Ensures aria-hidden is not present on the document body. | cat.aria, wcag2a | true | +| aria-hidden-body | Ensures aria-hidden is not present on the document body. | cat.aria, wcag2a, wcag412 | true | | aria-required-attr | Ensures elements with ARIA roles have all required ARIA attributes | cat.aria, wcag2a, wcag411, wcag412 | true | | aria-required-children | Ensures elements with an ARIA role that require child roles contain them | cat.aria, wcag2a, wcag131 | true | | aria-required-parent | Ensures elements with an ARIA role that require parent roles are contained by them | cat.aria, wcag2a, wcag131 | true | From 288fc9dea1218aea8ff041e04e934ec00caa0c8f Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Tue, 13 Jun 2017 13:16:17 +0200 Subject: [PATCH 41/41] docs: Add Jest + React example (#346) * docs: Add an example with Jest, React and Enzyme * fix: update jest example package.json * fix: Avoid linting jsx example * docs: Add readme to jest+react example --- Gruntfile.js | 5 +++- doc/examples/jest+react/.babelrc | 3 +++ doc/examples/jest+react/README.md | 36 +++++++++++++++++++++++++ doc/examples/jest+react/link.js | 11 ++++++++ doc/examples/jest+react/link.test.js | 18 +++++++++++++ doc/examples/jest+react/package.json | 28 +++++++++++++++++++ doc/examples/jest+react/test-helpers.js | 18 +++++++++++++ 7 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 doc/examples/jest+react/.babelrc create mode 100644 doc/examples/jest+react/README.md create mode 100644 doc/examples/jest+react/link.js create mode 100644 doc/examples/jest+react/link.test.js create mode 100644 doc/examples/jest+react/package.json create mode 100644 doc/examples/jest+react/test-helpers.js diff --git a/Gruntfile.js b/Gruntfile.js index 70be6fedbb..ec9880ecd3 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -327,7 +327,10 @@ module.exports = function (grunt) { reporter: grunt.option('report') ? 'checkstyle' : undefined, reporterOutput: grunt.option('report') ? 'tmp/lint.xml' : undefined }, - src: ['lib/**/*.js', 'test/**/*.js', 'build/tasks/**/*.js', 'doc/**/*.js', 'Gruntfile.js'] + src: [ + 'lib/**/*.js', 'test/**/*.js', 'build/tasks/**/*.js', + 'doc/**/*.js', '!doc/examples/jest+react/*.js', 'Gruntfile.js' + ] } } }); diff --git a/doc/examples/jest+react/.babelrc b/doc/examples/jest+react/.babelrc new file mode 100644 index 0000000000..facd180928 --- /dev/null +++ b/doc/examples/jest+react/.babelrc @@ -0,0 +1,3 @@ +{ + "presets": ["es2015", "react"] +} \ No newline at end of file diff --git a/doc/examples/jest+react/README.md b/doc/examples/jest+react/README.md new file mode 100644 index 0000000000..af227678cd --- /dev/null +++ b/doc/examples/jest+react/README.md @@ -0,0 +1,36 @@ +# Jest + React README # + +This example demonstrates how to use aXe to test React components using the +Jest unit testing framework. + +The unit test is in `link.test.js`, and has one test cases, showing how to run +axe-core in Jest (using JSDOM and Enzyme). + +## To configure the example ## + +* Node must be installed; please follow the directions at http://www.nodejs.org + to install it. +* Move to the `doc/examples/jest+react` directory +* `npm install` to install dependencies + +## To run the example ## + +* Move to the `doc/examples/jest+react` directory +* `npm test` to run Jasmine + +You should see output indicating that the tests ran successfully, with zero +failures. + +## To modify the example ## + +This example can be modified to test components in other test frameworks as well. To use axe-core with JSDOM (Like Jest does), you will need to ensure that JSDOM variables are made available on the global object. An easy way to do this is to use [jsdom-global](https://github.com/rstacruz/jsdom-global). + +For example, when running Mocha, you should require `jsdom-global/register`. The command for this is as follows: + +```shell +mocha *.test.js --require jsdom-global/register +``` + +## Timeout Issues ## + +Axe-core is very fast for what it does, but when testing larger components, it may take a few seconds to complete. This is because axe will be running thousands of tests in a single call. When testing composite components, you may have to increase the timeout setting. diff --git a/doc/examples/jest+react/link.js b/doc/examples/jest+react/link.js new file mode 100644 index 0000000000..7a8ffd41b4 --- /dev/null +++ b/doc/examples/jest+react/link.js @@ -0,0 +1,11 @@ +import React from 'react'; + +export default class Link extends React.Component { + render() { + return ( + + {this.props.children} + + ); + } +}; diff --git a/doc/examples/jest+react/link.test.js b/doc/examples/jest+react/link.test.js new file mode 100644 index 0000000000..0d4d9123eb --- /dev/null +++ b/doc/examples/jest+react/link.test.js @@ -0,0 +1,18 @@ +import React from 'react'; +import axe from 'axe-core'; +import { mountToDoc } from './test-helpers'; + +import Link from './Link'; + +test('Link has no aXe violations', (done) => { + const linkComponent = mountToDoc( + aXe website + ); + const linkNode = linkComponent.getDOMNode(); + + axe.run(linkNode, (err, { violations }) => { + expect(err).toBe(null); + expect(violations).toHaveLength(0); + done(); + }); +}); diff --git a/doc/examples/jest+react/package.json b/doc/examples/jest+react/package.json new file mode 100644 index 0000000000..081ee5a91d --- /dev/null +++ b/doc/examples/jest+react/package.json @@ -0,0 +1,28 @@ +{ + "name": "axe-jest-react-example", + "description": "aXe Jest + React Example", + "version": "0.0.1", + "private": true, + "author": { + "name": "Wilco Fiers", + "organization": "Deque Systems, Inc.", + "url": "http://deque.com/" + }, + "dependencies": { + }, + "scripts": { + "test": "jest" + }, + "devDependencies": { + "axe-core": "^2.2.3", + "babel-jest": "^20.0.3", + "babel-preset-es2015": "^6.24.1", + "babel-preset-react": "^6.24.1", + "enzyme": "^2.8.2", + "jest": "^20.0.4", + "jest-cli": "^20.0.4", + "react": "^15.5.4", + "react-dom": "^15.5.4", + "react-test-renderer": "^15.5.4" + } +} diff --git a/doc/examples/jest+react/test-helpers.js b/doc/examples/jest+react/test-helpers.js new file mode 100644 index 0000000000..5be9997e1c --- /dev/null +++ b/doc/examples/jest+react/test-helpers.js @@ -0,0 +1,18 @@ +import { mount } from 'enzyme'; + +let wrapper +export function mountToDoc (reactElm) { + if (!document) { + // Set up a basic DOM + global.document = jsdom('') + } + if (!wrapper) { + wrapper = document.createElement('main') + document.body.appendChild(wrapper) + } + + const container = mount(reactElm) + wrapper.innerHTML = '' + wrapper.appendChild(container.getDOMNode()) + return container +}