From 54a8116546b056569d1551b8ea3c8b01fa7d8df5 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Tue, 4 Oct 2022 10:47:43 +0200 Subject: [PATCH] fix(label): avoid passing labels because of an input[value] (#3688) * fix(label): avoid passing labels because of an input[value] * correct label examples * Add textarea integration test * Fix failing test * Use sanitize * Add addition test for explicit-evaluate --- lib/checks/label/explicit-evaluate.js | 55 +++-- lib/checks/label/implicit-evaluate.js | 14 +- test/checks/label/explicit.js | 201 ++++++++++-------- test/checks/label/implicit.js | 116 ++++++---- test/checks/shared/aria-labelledby.js | 103 ++++----- test/integration/rules/label/label.html | 19 +- test/integration/rules/label/label.json | 6 +- .../rules/select-name/select-name.html | 16 ++ .../rules/select-name/select-name.json | 5 +- 9 files changed, 312 insertions(+), 223 deletions(-) diff --git a/lib/checks/label/explicit-evaluate.js b/lib/checks/label/explicit-evaluate.js index b056b8e41d..585371989b 100644 --- a/lib/checks/label/explicit-evaluate.js +++ b/lib/checks/label/explicit-evaluate.js @@ -1,34 +1,43 @@ import { getRootNode, isVisibleOnScreen } from '../../commons/dom'; -import { accessibleText } from '../../commons/text'; +import { accessibleText, sanitize } from '../../commons/text'; import { escapeSelector } from '../../core/utils'; function explicitEvaluate(node, options, virtualNode) { - if (virtualNode.attr('id')) { - if (!virtualNode.actualNode) { - return undefined; - } + if (!virtualNode.attr('id')) { + return false; + } + if (!virtualNode.actualNode) { + return undefined; + } - const root = getRootNode(virtualNode.actualNode); - const id = escapeSelector(virtualNode.attr('id')); - const labels = Array.from(root.querySelectorAll(`label[for="${id}"]`)); + const root = getRootNode(virtualNode.actualNode); + const id = escapeSelector(virtualNode.attr('id')); + const labels = Array.from(root.querySelectorAll(`label[for="${id}"]`)); + this.relatedNodes(labels); - if (labels.length) { - try { - return labels.some(label => { - // defer to hidden-explicit-label check for better messaging - if (!isVisibleOnScreen(label)) { - return true; - } else { - return !!accessibleText(label); - } - }); - } catch (e) { - return undefined; - } - } + if (!labels.length) { + return false; } - return false; + try { + return labels.some(label => { + // defer to hidden-explicit-label check for better messaging + if (!isVisibleOnScreen(label)) { + return true; + } else { + const explicitLabel = sanitize( + accessibleText(label, { + inControlContext: true, + startNode: virtualNode + }) + ); + this.data({ explicitLabel }); + return !!explicitLabel; + } + }); + } catch (e) { + return undefined; + } } export default explicitEvaluate; diff --git a/lib/checks/label/implicit-evaluate.js b/lib/checks/label/implicit-evaluate.js index a3d0687b2c..4d43d9f5ef 100644 --- a/lib/checks/label/implicit-evaluate.js +++ b/lib/checks/label/implicit-evaluate.js @@ -1,11 +1,21 @@ import { closest } from '../../core/utils'; -import { accessibleTextVirtual } from '../../commons/text'; +import { accessibleTextVirtual, sanitize } from '../../commons/text'; function implicitEvaluate(node, options, virtualNode) { try { const label = closest(virtualNode, 'label'); if (label) { - return !!accessibleTextVirtual(label, { inControlContext: true }); + const implicitLabel = sanitize( + accessibleTextVirtual(label, { + inControlContext: true, + startNode: virtualNode + }) + ); + if (label.actualNode) { + this.relatedNodes([label.actualNode]); + } + this.data({ implicitLabel }); + return !!implicitLabel; } return false; } catch (e) { diff --git a/test/checks/label/explicit.js b/test/checks/label/explicit.js index d26703672a..eafb001199 100644 --- a/test/checks/label/explicit.js +++ b/test/checks/label/explicit.js @@ -1,136 +1,167 @@ -describe('explicit-label', function() { - 'use strict'; - - var fixture = document.getElementById('fixture'); - var fixtureSetup = axe.testUtils.fixtureSetup; - var queryFixture = axe.testUtils.queryFixture; - var shadowSupport = axe.testUtils.shadowSupport; - - afterEach(function() { - fixture.innerHTML = ''; +describe('explicit-label', () => { + const fixtureSetup = axe.testUtils.fixtureSetup; + const checkSetup = axe.testUtils.checkSetup; + const checkEvaluate = axe.testUtils.getCheckEvaluate('explicit-label'); + const checkContext = axe.testUtils.MockCheckContext(); + + afterEach(() => { + checkContext.reset(); }); - it('should return false if an empty label is present', function() { - var vNode = queryFixture( + it('returns false if an empty label is present', () => { + const params = checkSetup( '' ); - assert.isFalse( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) + assert.isFalse(checkEvaluate.apply(checkContext, params)); + }); + + it('returns false if the label is empty except for the target value', () => { + const params = checkSetup( + '' ); + assert.isFalse(checkEvaluate.apply(checkContext, params)); }); - it('should return true if a non-empty label is present', function() { - var vNode = queryFixture( - '' + it('returns false if an empty label is present that uses aria-labelledby', () => { + const params = checkSetup( + '' + + '' + + 'aria label' ); - assert.isTrue( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) + assert.isFalse(checkEvaluate.apply(checkContext, params)); + }); + + it('returns true if a non-empty label is present', () => { + const params = checkSetup( + '' ); + assert.isTrue(checkEvaluate.apply(checkContext, params)); }); - it('should return true if an invisible non-empty label is present, to defer to hidden-explicit-label', function() { - var vNode = queryFixture( + it('returns true if an invisible non-empty label is present, to defer to hidden-explicit-label', () => { + const params = checkSetup( '' ); - assert.isTrue( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); + assert.isTrue(checkEvaluate.apply(checkContext, params)); }); - it('should return false if a label is not present', function() { - var vNode = queryFixture(''); - assert.isFalse( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); + it('returns false if a label is not present', () => { + const params = checkSetup(''); + assert.isFalse(checkEvaluate.apply(checkContext, params)); }); - it('should work for multiple labels', function() { - var vNode = queryFixture( + it('should work for multiple labels', () => { + const params = checkSetup( '' ); - assert.isTrue( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); + assert.isTrue(checkEvaluate.apply(checkContext, params)); }); - (shadowSupport.v1 ? it : xit)( - 'should return true if input and label are in the same shadow root', - function() { - var root = document.createElement('div'); - var shadow = root.attachShadow({ mode: 'open' }); + describe('.data', () => { + it('is null if there is no label', () => { + const params = checkSetup(''); + checkEvaluate.apply(checkContext, params); + assert.isNull(checkContext._data); + }); + + it('includes the `explicitLabel` text of the first non-empty label', () => { + const params = checkSetup( + '' + + '' + + '' + + '' + ); + checkEvaluate.apply(checkContext, params); + assert.deepEqual(checkContext._data, { explicitLabel: 'text' }); + }); + + it('is empty { explicitLabel: "" } if the label is empty', () => { + const params = checkSetup( + '' + + '' + + '' + ); + checkEvaluate.apply(checkContext, params); + assert.deepEqual(checkContext._data, { explicitLabel: '' }); + }); + }); + + describe('related nodes', () => { + it('is empty when there are no labels', () => { + const params = checkSetup(''); + checkEvaluate.apply(checkContext, params); + assert.isEmpty(checkContext._relatedNodes); + }); + + it('includes each associated label', () => { + const params = checkSetup( + '' + + '' + + '' + ); + checkEvaluate.apply(checkContext, params); + const ids = checkContext._relatedNodes.map(node => '#' + node.id); + assert.deepEqual(ids, ['#lbl1', '#lbl2']); + }); + }); + + describe('with shadow DOM', () => { + it('returns true if input and label are in the same shadow root', () => { + const root = document.createElement('div'); + const shadow = root.attachShadow({ mode: 'open' }); shadow.innerHTML = ''; fixtureSetup(root); - var vNode = axe.utils.getNodeFromTree(shadow.querySelector('#target')); - assert.isTrue( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); - } - ); + const vNode = axe.utils.getNodeFromTree(shadow.querySelector('#target')); + assert.isTrue(checkEvaluate.call(checkContext, null, {}, vNode)); + }); - (shadowSupport.v1 ? it : xit)( - 'should return true if label content is slotted', - function() { - var root = document.createElement('div'); + it('returns true if label content is slotted', () => { + const root = document.createElement('div'); root.innerHTML = 'American band'; - var shadow = root.attachShadow({ mode: 'open' }); + const shadow = root.attachShadow({ mode: 'open' }); shadow.innerHTML = ''; fixtureSetup(root); - var vNode = axe.utils.getNodeFromTree(shadow.querySelector('#target')); - assert.isTrue( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); - } - ); + const vNode = axe.utils.getNodeFromTree(shadow.querySelector('#target')); + assert.isTrue(checkEvaluate.call(checkContext, null, {}, vNode)); + }); - (shadowSupport.v1 ? it : xit)( - 'should return false if input is inside shadow DOM and the label is not', - function() { - var root = document.createElement('div'); + it('returns false if input is inside shadow DOM and the label is not', () => { + const root = document.createElement('div'); root.innerHTML = ''; - var shadow = root.attachShadow({ mode: 'open' }); + const shadow = root.attachShadow({ mode: 'open' }); shadow.innerHTML = ''; fixtureSetup(root); - var vNode = axe.utils.getNodeFromTree(shadow.querySelector('#target')); - assert.isFalse( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); - } - ); + const vNode = axe.utils.getNodeFromTree(shadow.querySelector('#target')); + assert.isFalse(checkEvaluate.call(checkContext, null, {}, vNode)); + }); - (shadowSupport.v1 ? it : xit)( - 'should return false if label is inside shadow DOM and the input is not', - function() { - var root = document.createElement('div'); + it('returns false if label is inside shadow DOM and the input is not', () => { + const root = document.createElement('div'); root.innerHTML = ''; - var shadow = root.attachShadow({ mode: 'open' }); + const shadow = root.attachShadow({ mode: 'open' }); shadow.innerHTML = ''; fixtureSetup(root); - var vNode = axe.utils.getNodeFromTree(root.querySelector('#target')); - assert.isFalse( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, vNode) - ); - } - ); + const vNode = axe.utils.getNodeFromTree(root.querySelector('#target')); + assert.isFalse(checkEvaluate.call(checkContext, null, {}, vNode)); + }); + }); - describe('SerialVirtualNode', function() { - it('should return undefined', function() { - var virtualNode = new axe.SerialVirtualNode({ + describe('SerialVirtualNode', () => { + it('returns undefined', () => { + const virtualNode = new axe.SerialVirtualNode({ nodeName: 'input', attributes: { type: 'text' } }); - - assert.isFalse( - axe.testUtils.getCheckEvaluate('explicit-label')(null, {}, virtualNode) - ); + assert.isFalse(checkEvaluate.call(checkContext, null, {}, virtualNode)); }); }); }); diff --git a/test/checks/label/implicit.js b/test/checks/label/implicit.js index 60aa168604..83eae364ba 100644 --- a/test/checks/label/implicit.js +++ b/test/checks/label/implicit.js @@ -1,79 +1,109 @@ -describe('implicit-label', function() { - 'use strict'; +describe('implicit-label', () => { + const fixtureSetup = axe.testUtils.fixtureSetup; + const checkSetup = axe.testUtils.checkSetup; + const checkEvaluate = axe.testUtils.getCheckEvaluate('implicit-label'); + const checkContext = axe.testUtils.MockCheckContext(); - var fixture = document.getElementById('fixture'); - var fixtureSetup = axe.testUtils.fixtureSetup; + afterEach(() => { + checkContext.reset(); + }); - afterEach(function() { - fixture.innerHTML = ''; - axe._tree = undefined; + it('returns false if an empty label is present', () => { + const params = checkSetup(''); + assert.isFalse(checkEvaluate.apply(checkContext, params)); }); - it('should return false if an empty label is present', function() { - fixtureSetup(''); - var node = fixture.querySelector('#target'); - var virtualNode = axe.utils.getNodeFromTree(node); - assert.isFalse( - axe.testUtils.getCheckEvaluate('implicit-label')(null, {}, virtualNode) + it('returns false on an empty label when then control has a value', () => { + const params = checkSetup( + '' ); + assert.isFalse(checkEvaluate.apply(checkContext, params)); }); - it('should return false if an invisible non-empty label is present', function() { - fixtureSetup( + it('returns false if an invisible non-empty label is present', () => { + const params = checkSetup( '' ); - var node = fixture.querySelector('#target'); - var virtualNode = axe.utils.getNodeFromTree(node); - assert.isFalse( - axe.testUtils.getCheckEvaluate('implicit-label')(null, {}, virtualNode) - ); + assert.isFalse(checkEvaluate.apply(checkContext, params)); }); - it('should return true if a non-empty label is present', function() { - fixtureSetup(''); - var node = fixture.querySelector('#target'); - var virtualNode = axe.utils.getNodeFromTree(node); - assert.isTrue( - axe.testUtils.getCheckEvaluate('implicit-label')(null, {}, virtualNode) + it('returns true if a non-empty label is present', () => { + const params = checkSetup( + '' ); + assert.isTrue(checkEvaluate.apply(checkContext, params)); }); - it('should return false if a label is not present', function() { - var node = document.createElement('input'); + it('returns false if a label is not present', () => { + const node = document.createElement('input'); node.type = 'text'; fixtureSetup(node); + const virtualNode = axe.utils.getNodeFromTree(node); + assert.isFalse(checkEvaluate.call(checkContext, null, {}, virtualNode)); + }); - var virtualNode = axe.utils.getNodeFromTree(node); - assert.isFalse( - axe.testUtils.getCheckEvaluate('implicit-label')(null, {}, virtualNode) - ); + describe('data', () => { + it('is null if there is no label', () => { + const params = checkSetup(''); + checkEvaluate.apply(checkContext, params); + assert.isNull(checkContext._data); + }); + + it('includes the implicit label if one is set', () => { + const params = checkSetup( + '' + ); + checkEvaluate.apply(checkContext, params); + assert.deepEqual(checkContext._data, { implicitLabel: 'Some text' }); + }); + + it('has { implicitLabel: "" } when the label is empty', () => { + const params = checkSetup( + '' + ); + checkEvaluate.apply(checkContext, params); + assert.deepEqual(checkContext._data, { implicitLabel: '' }); + }); + }); + + describe('relatedNodes', () => { + it('is null if there is no label', () => { + const params = checkSetup(''); + checkEvaluate.apply(checkContext, params); + assert.isEmpty(checkContext._relatedNodes); + }); + + it('includes the nearest label as its related node', () => { + const params = checkSetup( + '' + ); + checkEvaluate.apply(checkContext, params); + const ids = checkContext._relatedNodes.map(node => '#' + node.id); + assert.deepEqual(ids, ['#lbl']); + }); }); - describe('SerialVirtualNode', function() { - it('should return false if no implicit label', function() { - var virtualNode = new axe.SerialVirtualNode({ + describe('SerialVirtualNode', () => { + it('returns false if no implicit label', () => { + const virtualNode = new axe.SerialVirtualNode({ nodeName: 'input', attributes: { type: 'text' } }); virtualNode.parent = null; - - assert.isFalse( - axe.testUtils.getCheckEvaluate('implicit-label')(null, {}, virtualNode) - ); + assert.isFalse(checkEvaluate.call(checkContext, null, {}, virtualNode)); }); - it('should return undefined if tree is not complete', function() { - var virtualNode = new axe.SerialVirtualNode({ + it('returns undefined if tree is not complete', () => { + const virtualNode = new axe.SerialVirtualNode({ nodeName: 'input', attributes: { type: 'text' } }); - assert.isUndefined( - axe.testUtils.getCheckEvaluate('implicit-label')(null, {}, virtualNode) + checkEvaluate.call(checkContext, null, {}, virtualNode) ); }); }); diff --git a/test/checks/shared/aria-labelledby.js b/test/checks/shared/aria-labelledby.js index 7a58680af5..5ec6549fd2 100644 --- a/test/checks/shared/aria-labelledby.js +++ b/test/checks/shared/aria-labelledby.js @@ -1,103 +1,84 @@ -describe('aria-labelledby', function() { - 'use strict'; +describe('aria-labelledby', () => { + const queryFixture = axe.testUtils.queryFixture; + const checkEvaluate = axe.testUtils.getCheckEvaluate('aria-labelledby'); - var fixture = document.getElementById('fixture'); - 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 = queryFixture( + it('should return true if an aria-labelledby and its target is present', () => { + const node = queryFixture( '
bananas
' ); - - assert.isTrue( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + assert.isTrue(checkEvaluate(null, {}, node)); }); - it('should return true if only one element referenced by aria-labelledby has visible text', function() { - var node = queryFixture( + it('should return true if only one element referenced by aria-labelledby has visible text', () => { + const node = queryFixture( '
bananas
' ); - - assert.isTrue( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + assert.isTrue(checkEvaluate(null, {}, node)); }); - it('should return false if an aria-labelledby is not present', function() { - var node = queryFixture('
'); - - assert.isFalse( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + it('should return false if an aria-labelledby is not present', () => { + const node = queryFixture('
'); + assert.isFalse(checkEvaluate(null, {}, node)); }); - it('should return true if an aria-labelledby is present that references hidden elements', function() { - var node = queryFixture( + it('should return true if an aria-labelledby is present that references hidden elements', () => { + const node = queryFixture( '
' ); - - assert.isTrue( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + assert.isTrue(checkEvaluate(null, {}, node)); }); - it('should return false if an aria-labelledby is present, but references an element with only hidden content', function() { - var node = queryFixture( + it('should return false if an aria-labelledby is present, but references an element with only hidden content', () => { + const node = queryFixture( '
bananas
' ); + assert.isFalse(checkEvaluate(null, {}, node)); + }); - assert.isFalse( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) + it('returns false if aria-labelledby refers to the own element', () => { + const vNode = queryFixture( + '' ); + assert.isFalse(checkEvaluate(null, {}, vNode)); }); - it('should return true if an aria-labelledby is present that references elements with has aria-hidden=true', function() { - var node = queryFixture( - '
' + it('returns false if aria-labelledby refers to parent, and there are no sibling', () => { + const vNode = queryFixture( + '
' ); + assert.isFalse(checkEvaluate(null, {}, vNode)); + }); - assert.isTrue( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) + it('should return true if an aria-labelledby is present that references elements with has aria-hidden=true', () => { + const node = queryFixture( + '
' ); + assert.isTrue(checkEvaluate(null, {}, 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 = queryFixture( + it('should return false if an aria-labelledby is present that references elements with has aria-hidden=true in the content', () => { + const node = queryFixture( '
' ); - - assert.isFalse( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + assert.isFalse(checkEvaluate(null, {}, node)); }); - describe('SerialVirtualNode', function() { - it('should return false if an aria-labelledby is not present', function() { - var node = new axe.SerialVirtualNode({ + describe('SerialVirtualNode', () => { + it('should return false if an aria-labelledby is not present', () => { + const node = new axe.SerialVirtualNode({ nodeName: 'div' }); - - assert.isFalse( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + assert.isFalse(checkEvaluate(null, {}, node)); }); - it('should return undefined if an aria-labelledby is present', function() { - var node = new axe.SerialVirtualNode({ + it('should return undefined if an aria-labelledby is present', () => { + const node = new axe.SerialVirtualNode({ nodeName: 'div', attributes: { 'aria-labelledby': 'woohoo' } }); - - assert.isUndefined( - axe.testUtils.getCheckEvaluate('aria-labelledby')(null, {}, node) - ); + assert.isUndefined(checkEvaluate(null, {}, node)); }); }); }); diff --git a/test/integration/rules/label/label.html b/test/integration/rules/label/label.html index 995011e25c..a9a5d99b3d 100644 --- a/test/integration/rules/label/label.html +++ b/test/integration/rules/label/label.html @@ -11,15 +11,15 @@
Label
- + - + - Text @@ -28,7 +28,7 @@
@@ -52,9 +52,7 @@
- +
@@ -63,4 +61,11 @@ + + + +
+ +
+ diff --git a/test/integration/rules/label/label.json b/test/integration/rules/label/label.json index 6df2763965..abccc763b6 100644 --- a/test/integration/rules/label/label.json +++ b/test/integration/rules/label/label.json @@ -11,7 +11,11 @@ ["#fail11"], ["#fail22"], ["#fail25"], - ["#fail27"] + ["#fail27"], + ["#fail28"], + ["#fail29"], + ["#fail30"], + ["#fail31"] ], "passes": [ ["#pass1"], diff --git a/test/integration/rules/select-name/select-name.html b/test/integration/rules/select-name/select-name.html index a380fb7e35..a794379258 100644 --- a/test/integration/rules/select-name/select-name.html +++ b/test/integration/rules/select-name/select-name.html @@ -33,4 +33,20 @@ + + + +
+ +
diff --git a/test/integration/rules/select-name/select-name.json b/test/integration/rules/select-name/select-name.json index 4473a99db2..b2e7e39219 100644 --- a/test/integration/rules/select-name/select-name.json +++ b/test/integration/rules/select-name/select-name.json @@ -7,7 +7,10 @@ ["#fail3"], ["#fail4"], ["#fail5"], - ["#fail6"] + ["#fail6"], + ["#fail7"], + ["#fail8"], + ["#fail9"] ], "passes": [ ["#pass1"],