From eeee483bfe190df9230d8386774623554ea65400 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 2 Jul 2020 09:53:13 -0600 Subject: [PATCH 1/2] fix(aria-lablledby): work with virtual and serial virtual nodes --- doc/rule-descriptions.md | 22 ++--- lib/checks/shared/aria-labelledby-evaluate.js | 6 +- lib/checks/shared/aria-labelledby.json | 3 +- lib/commons/aria/arialabelledby-text.js | 22 +++-- lib/commons/dom/idrefs.js | 26 +++--- test/checks/shared/aria-labelledby.js | 90 +++++++++---------- 6 files changed, 92 insertions(+), 77 deletions(-) diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index a9fca816db..a327a0c5a8 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -12,7 +12,7 @@ | Rule ID | Description | Impact | Tags | Issue Type | | :------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------ | :---------------- | :------------------------------------------------------------------------------------ | :------------------------- | -| [area-alt](https://dequeuniversity.com/rules/axe/3.5/area-alt?application=RuleDescription) | Ensures <area> elements of image maps have alternate text | Critical | cat.text-alternatives, wcag2a, wcag111, wcag244, wcag412, section508, section508.22.a | failure | +| [area-alt](https://dequeuniversity.com/rules/axe/3.5/area-alt?application=RuleDescription) | Ensures <area> elements of image maps have alternate text | Critical | cat.text-alternatives, wcag2a, wcag111, wcag244, wcag412, section508, section508.22.a | failure, needs review | | [aria-allowed-attr](https://dequeuniversity.com/rules/axe/3.5/aria-allowed-attr?application=RuleDescription) | Ensures ARIA attributes are allowed for an element's role | Critical | cat.aria, wcag2a, wcag412 | failure | | [aria-hidden-body](https://dequeuniversity.com/rules/axe/3.5/aria-hidden-body?application=RuleDescription) | Ensures aria-hidden='true' is not present on the document body. | Critical | cat.aria, wcag2a, wcag412 | failure | | [aria-hidden-focus](https://dequeuniversity.com/rules/axe/3.5/aria-hidden-focus?application=RuleDescription) | Ensures aria-hidden elements do not contain focusable elements | Serious | cat.name-role-value, wcag2a, wcag412, wcag131 | failure, needs review | @@ -27,7 +27,7 @@ | [aria-valid-attr](https://dequeuniversity.com/rules/axe/3.5/aria-valid-attr?application=RuleDescription) | Ensures attributes that begin with aria- are valid ARIA attributes | Critical | cat.aria, wcag2a, wcag412 | failure | | [audio-caption](https://dequeuniversity.com/rules/axe/3.5/audio-caption?application=RuleDescription) | Ensures <audio> elements have captions | Critical | cat.time-and-media, wcag2a, wcag121, section508, section508.22.a | needs review | | [blink](https://dequeuniversity.com/rules/axe/3.5/blink?application=RuleDescription) | Ensures <blink> elements are not used | Serious | cat.time-and-media, wcag2a, wcag222, section508, section508.22.j | failure | -| [button-name](https://dequeuniversity.com/rules/axe/3.5/button-name?application=RuleDescription) | Ensures buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a | failure | +| [button-name](https://dequeuniversity.com/rules/axe/3.5/button-name?application=RuleDescription) | Ensures buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a | failure, needs review | | [bypass](https://dequeuniversity.com/rules/axe/3.5/bypass?application=RuleDescription) | Ensures each page has at least one mechanism for a user to bypass navigation and jump straight to the content | Serious | cat.keyboard, wcag2a, wcag241, section508, section508.22.o | failure | | [color-contrast](https://dequeuniversity.com/rules/axe/3.5/color-contrast?application=RuleDescription) | Ensures the contrast between foreground and background colors meets WCAG 2 AA contrast ratio thresholds | Serious | cat.color, wcag2aa, wcag143 | failure, needs review | | [definition-list](https://dequeuniversity.com/rules/axe/3.5/definition-list?application=RuleDescription) | Ensures <dl> elements are structured correctly | Serious | cat.structure, wcag2a, wcag131 | failure | @@ -37,24 +37,24 @@ | [duplicate-id-aria](https://dequeuniversity.com/rules/axe/3.5/duplicate-id-aria?application=RuleDescription) | Ensures every id attribute value used in ARIA and in labels is unique | Critical | cat.parsing, wcag2a, wcag411 | failure | | [duplicate-id](https://dequeuniversity.com/rules/axe/3.5/duplicate-id?application=RuleDescription) | Ensures every id attribute value is unique | Minor | cat.parsing, wcag2a, wcag411 | failure | | [form-field-multiple-labels](https://dequeuniversity.com/rules/axe/3.5/form-field-multiple-labels?application=RuleDescription) | Ensures form field does not have multiple label elements | Moderate | cat.forms, wcag2a, wcag332 | needs review | -| [frame-title](https://dequeuniversity.com/rules/axe/3.5/frame-title?application=RuleDescription) | Ensures <iframe> and <frame> elements contain a non-empty title attribute | Serious | cat.text-alternatives, wcag2a, wcag241, wcag412, section508, section508.22.i | failure | +| [frame-title](https://dequeuniversity.com/rules/axe/3.5/frame-title?application=RuleDescription) | Ensures <iframe> and <frame> elements contain a non-empty title attribute | Serious | cat.text-alternatives, wcag2a, wcag241, wcag412, section508, section508.22.i | failure, needs review | | [html-has-lang](https://dequeuniversity.com/rules/axe/3.5/html-has-lang?application=RuleDescription) | Ensures every HTML document has a lang attribute | Serious | cat.language, wcag2a, wcag311 | failure | | [html-lang-valid](https://dequeuniversity.com/rules/axe/3.5/html-lang-valid?application=RuleDescription) | Ensures the lang attribute of the <html> element has a valid value | Serious | cat.language, wcag2a, wcag311 | failure | | [html-xml-lang-mismatch](https://dequeuniversity.com/rules/axe/3.5/html-xml-lang-mismatch?application=RuleDescription) | Ensure that HTML elements with both valid lang and xml:lang attributes agree on the base language of the page | Moderate | cat.language, wcag2a, wcag311 | failure | -| [image-alt](https://dequeuniversity.com/rules/axe/3.5/image-alt?application=RuleDescription) | Ensures <img> elements have alternate text or a role of none or presentation | Critical | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure | -| [input-button-name](https://dequeuniversity.com/rules/axe/3.5/input-button-name?application=RuleDescription) | Ensures input buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a | failure | -| [input-image-alt](https://dequeuniversity.com/rules/axe/3.5/input-image-alt?application=RuleDescription) | Ensures <input type="image"> elements have alternate text | Critical | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure | -| [label](https://dequeuniversity.com/rules/axe/3.5/label?application=RuleDescription) | Ensures every form element has a label | Minor, Critical | cat.forms, wcag2a, wcag412, wcag131, section508, section508.22.n | failure | -| [link-name](https://dequeuniversity.com/rules/axe/3.5/link-name?application=RuleDescription) | Ensures links have discernible text | Serious | cat.name-role-value, wcag2a, wcag412, wcag244, section508, section508.22.a | failure | +| [image-alt](https://dequeuniversity.com/rules/axe/3.5/image-alt?application=RuleDescription) | Ensures <img> elements have alternate text or a role of none or presentation | Critical | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review | +| [input-button-name](https://dequeuniversity.com/rules/axe/3.5/input-button-name?application=RuleDescription) | Ensures input buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a | failure, needs review | +| [input-image-alt](https://dequeuniversity.com/rules/axe/3.5/input-image-alt?application=RuleDescription) | Ensures <input type="image"> elements have alternate text | Critical | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review | +| [label](https://dequeuniversity.com/rules/axe/3.5/label?application=RuleDescription) | Ensures every form element has a label | Minor, Critical | cat.forms, wcag2a, wcag412, wcag131, section508, section508.22.n | failure, needs review | +| [link-name](https://dequeuniversity.com/rules/axe/3.5/link-name?application=RuleDescription) | Ensures links have discernible text | Serious | cat.name-role-value, wcag2a, wcag412, wcag244, section508, section508.22.a | failure, needs review | | [list](https://dequeuniversity.com/rules/axe/3.5/list?application=RuleDescription) | Ensures that lists are structured correctly | Serious | cat.structure, wcag2a, wcag131 | failure | | [listitem](https://dequeuniversity.com/rules/axe/3.5/listitem?application=RuleDescription) | Ensures <li> elements are used semantically | Serious | cat.structure, wcag2a, wcag131 | failure | | [marquee](https://dequeuniversity.com/rules/axe/3.5/marquee?application=RuleDescription) | Ensures <marquee> elements are not used | Serious | cat.parsing, wcag2a, wcag222 | failure | | [meta-refresh](https://dequeuniversity.com/rules/axe/3.5/meta-refresh?application=RuleDescription) | Ensures <meta http-equiv="refresh"> is not used | Critical | cat.time-and-media, wcag2a, wcag2aaa, wcag221, wcag224, wcag325 | failure | -| [object-alt](https://dequeuniversity.com/rules/axe/3.5/object-alt?application=RuleDescription) | Ensures <object> elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure | -| [role-img-alt](https://dequeuniversity.com/rules/axe/3.5/role-img-alt?application=RuleDescription) | Ensures [role='img'] elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure | +| [object-alt](https://dequeuniversity.com/rules/axe/3.5/object-alt?application=RuleDescription) | Ensures <object> elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review | +| [role-img-alt](https://dequeuniversity.com/rules/axe/3.5/role-img-alt?application=RuleDescription) | Ensures [role='img'] elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review | | [scrollable-region-focusable](https://dequeuniversity.com/rules/axe/3.5/scrollable-region-focusable?application=RuleDescription) | Elements that have scrollable content should be accessible by keyboard | Moderate | wcag2a, wcag211 | failure | | [server-side-image-map](https://dequeuniversity.com/rules/axe/3.5/server-side-image-map?application=RuleDescription) | Ensures that server-side image maps are not used | Minor | cat.text-alternatives, wcag2a, wcag211, section508, section508.22.f | needs review | -| [svg-img-alt](https://dequeuniversity.com/rules/axe/3.5/svg-img-alt?application=RuleDescription) | Ensures svg elements with an img, graphics-document or graphics-symbol role have an accessible text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure | +| [svg-img-alt](https://dequeuniversity.com/rules/axe/3.5/svg-img-alt?application=RuleDescription) | Ensures svg elements with an img, graphics-document or graphics-symbol role have an accessible text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review | | [td-headers-attr](https://dequeuniversity.com/rules/axe/3.5/td-headers-attr?application=RuleDescription) | Ensure that each cell in a table using the headers refers to another cell in that table | Serious | cat.tables, wcag2a, wcag131, section508, section508.22.g | failure, needs review | | [th-has-data-cells](https://dequeuniversity.com/rules/axe/3.5/th-has-data-cells?application=RuleDescription) | Ensure that each table header in a data table refers to data cells | Serious | cat.tables, wcag2a, wcag131, section508, section508.22.g | failure, needs review | | [valid-lang](https://dequeuniversity.com/rules/axe/3.5/valid-lang?application=RuleDescription) | Ensures lang attributes have valid values | Serious | cat.language, wcag2aa, wcag312 | failure | diff --git a/lib/checks/shared/aria-labelledby-evaluate.js b/lib/checks/shared/aria-labelledby-evaluate.js index d90abcc6db..c500eaab03 100644 --- a/lib/checks/shared/aria-labelledby-evaluate.js +++ b/lib/checks/shared/aria-labelledby-evaluate.js @@ -2,7 +2,11 @@ import { sanitize } from '../../commons/text'; import { arialabelledbyText } from '../../commons/aria'; function ariaLabelledbyEvaluate(node) { - return !!sanitize(arialabelledbyText(node)); + try { + return !!sanitize(arialabelledbyText(node)); + } catch (e) { + return undefined; + } } export default ariaLabelledbyEvaluate; diff --git a/lib/checks/shared/aria-labelledby.json b/lib/checks/shared/aria-labelledby.json index d5943f95f3..0fea5792f3 100644 --- a/lib/checks/shared/aria-labelledby.json +++ b/lib/checks/shared/aria-labelledby.json @@ -5,7 +5,8 @@ "impact": "serious", "messages": { "pass": "aria-labelledby attribute exists and references elements that are visible to screen readers", - "fail": "aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty" + "fail": "aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty", + "incomplete": "aria-labelledby cannot run on non-DOM nodes" } } } diff --git a/lib/commons/aria/arialabelledby-text.js b/lib/commons/aria/arialabelledby-text.js index 9b4814ce02..af49be6f8b 100644 --- a/lib/commons/aria/arialabelledby-text.js +++ b/lib/commons/aria/arialabelledby-text.js @@ -1,9 +1,12 @@ import idrefs from '../dom/idrefs'; import accessibleText from '../text/accessible-text'; +import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node'; +import { getNodeFromTree } from '../../core/utils'; /** * Get the accessible name based on aria-labelledby * + * @deprecated Do not use Element directly. Pass VirtualNode instead * @param {VirtualNode} element * @param {Object} context * @property {Bool} inLabelledByContext Whether or not the lookup is part of aria-labelledby reference @@ -12,8 +15,14 @@ import accessibleText from '../text/accessible-text'; * @property {Bool} debug Enable logging for formControlValue * @return {string} Cancatinated text value for referenced elements */ -function arialabelledbyText(node, context = {}) { - node = node.actualNode || node; +function arialabelledbyText(vNode, context = {}) { + if (!(vNode instanceof AbstractVirtualNode)) { + if (vNode.nodeType !== 1) { + return ''; + } + vNode = getNodeFromTree(vNode); + } + /** * Note: The there are significant difference in how many "leads" browsers follow. * - Firefox stops after the first IDREF, so it @@ -28,19 +37,20 @@ function arialabelledbyText(node, context = {}) { * something no other browser seems to do. Axe doesn't do that. */ if ( - node.nodeType !== 1 || + vNode.props.nodeType !== 1 || context.inLabelledByContext || - context.inControlContext + context.inControlContext || + !vNode.attr('aria-labelledby') ) { return ''; } - const refs = idrefs(node, 'aria-labelledby').filter(elm => elm); + const refs = idrefs(vNode, 'aria-labelledby').filter(elm => elm); return refs.reduce((accessibleName, elm) => { const accessibleNameAdd = accessibleText(elm, { // Prevent the infinite reference loop: inLabelledByContext: true, - startNode: context.startNode || node, + startNode: context.startNode || vNode, ...context }); diff --git a/lib/commons/dom/idrefs.js b/lib/commons/dom/idrefs.js index cf5c88f609..30c053b62f 100644 --- a/lib/commons/dom/idrefs.js +++ b/lib/commons/dom/idrefs.js @@ -17,21 +17,25 @@ import getRootNode from './get-root-node'; * */ function idrefs(node, attr) { - 'use strict'; + node = node.actualNode || node; - const doc = getRootNode(node); - const result = []; - let attrValue = node.getAttribute(attr); + try { + const doc = getRootNode(node); + const result = []; + let attrValue = node.getAttribute(attr); - if (attrValue) { - // TODO: es-module-utils.tokenList - attrValue = axe.utils.tokenList(attrValue); - for (let index = 0; index < attrValue.length; index++) { - result.push(doc.getElementById(attrValue[index])); + if (attrValue) { + // TODO: es-module-utils.tokenList + attrValue = axe.utils.tokenList(attrValue); + for (let index = 0; index < attrValue.length; index++) { + result.push(doc.getElementById(attrValue[index])); + } } - } - return result; + return result; + } catch (e) { + throw new TypeError('Cannot resolve id references for non-DOM nodes'); + } } export default idrefs; diff --git a/test/checks/shared/aria-labelledby.js b/test/checks/shared/aria-labelledby.js index ea0d411708..4fab39df37 100644 --- a/test/checks/shared/aria-labelledby.js +++ b/test/checks/shared/aria-labelledby.js @@ -2,90 +2,86 @@ describe('aria-labelledby', function() { 'use strict'; var fixture = document.getElementById('fixture'); - var fixtureSetup = axe.testUtils.fixtureSetup; + var queryFixture = axe.testUtils.queryFixture; afterEach(function() { fixture.innerHTML = ''; }); it('should return true if an aria-labelledby and its target is present', function() { - var node = document.createElement('div'); - node.setAttribute('aria-labelledby', 'woohoo'); - fixture.appendChild(node); - var target = document.createElement('div'); - target.id = 'woohoo'; - target.innerHTML = 'bananas'; - fixtureSetup(target); + var node = queryFixture( + '
bananas
' + ); assert.isTrue(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); it('should return true if only one element referenced by aria-labelledby has visible text', function() { - var node = document.createElement('div'); - node.setAttribute('aria-labelledby', 'woohoo noexist hehe'); - fixture.appendChild(node); - var target = document.createElement('div'); - target.id = 'woohoo'; - target.innerHTML = 'bananas'; - fixtureSetup(target); + var node = queryFixture( + '
bananas
' + ); assert.isTrue(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); it('should return false if an aria-labelledby is not present', function() { - var node = document.createElement('div'); - fixtureSetup(node); + var node = queryFixture('
'); assert.isFalse(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); it('should return true if an aria-labelledby is present that references hidden elements', function() { - var node = document.createElement('div'); - node.setAttribute('aria-labelledby', 'woohoo'); - fixture.appendChild(node); - var target = document.createElement('div'); - target.id = 'woohoo'; - target.setAttribute('style', 'display:none;'); - target.innerHTML = 'bananas'; - fixtureSetup(target); + var node = queryFixture( + '
' + ); assert.isTrue(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); it('should return false if an aria-labelledby is present, but references an element with only hidden content', function() { - var node = document.createElement('div'); - node.setAttribute('aria-labelledby', 'woohoo'); - fixture.appendChild(node); - var target = document.createElement('div'); - target.id = 'woohoo'; - target.innerHTML = 'bananas'; - fixtureSetup(target); + var node = queryFixture( + '
bananas
' + ); assert.isFalse(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); it('should return true if an aria-labelledby is present that references elements with has aria-hidden=true', function() { - var node = document.createElement('div'); - node.setAttribute('aria-labelledby', 'woohoo'); - fixture.appendChild(node); - var target = document.createElement('div'); - target.id = 'woohoo'; - target.setAttribute('aria-hidden', 'true'); - target.innerHTML = 'bananas'; - fixtureSetup(target); + var node = queryFixture( + '
' + ); assert.isTrue(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); it('should return false if an aria-labelledby is present that references elements with has aria-hidden=true in the content', function() { - var node = document.createElement('div'); - node.setAttribute('aria-labelledby', 'woohoo'); - fixture.appendChild(node); - var target = document.createElement('div'); - target.id = 'woohoo'; - target.innerHTML = ''; - fixtureSetup(target); + var node = queryFixture( + '
' + ); assert.isFalse(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); }); + + describe('SerialVirtualNode', function() { + it('should return false if an aria-labelledby is not present', function() { + var node = new axe.SerialVirtualNode({ + nodeName: 'div' + }); + + assert.isFalse(axe.testUtils.getCheckEvaluate('aria-labelledby')(node)); + }); + + it('should return undefined if an aria-labelledby is present', function() { + var node = new axe.SerialVirtualNode({ + nodeName: 'div', + attributes: { + 'aria-labelledby': 'woohoo' + } + }); + + assert.isUndefined( + axe.testUtils.getCheckEvaluate('aria-labelledby')(node) + ); + }); + }); }); From 87baf14d196983bac5e643782035b3dd20757ad8 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Thu, 2 Jul 2020 11:25:25 -0600 Subject: [PATCH 2/2] Update lib/checks/shared/aria-labelledby.json Co-authored-by: Wilco Fiers --- lib/checks/shared/aria-labelledby.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/shared/aria-labelledby.json b/lib/checks/shared/aria-labelledby.json index 0fea5792f3..ea756e9f85 100644 --- a/lib/checks/shared/aria-labelledby.json +++ b/lib/checks/shared/aria-labelledby.json @@ -6,7 +6,7 @@ "messages": { "pass": "aria-labelledby attribute exists and references elements that are visible to screen readers", "fail": "aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty", - "incomplete": "aria-labelledby cannot run on non-DOM nodes" + "incomplete": "ensure aria-labelledby references an existing element" } } }