From f921bd217f1591d8e158cf67087c5fead442cc7d Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:57:40 -0600 Subject: [PATCH 1/3] fix(target-size): ignore descendant elements in shadow dom --- .eslintrc.js | 28 +++++++- lib/checks/mobile/target-size-evaluate.js | 5 +- lib/commons/dom/get-target-rects.js | 5 +- lib/core/utils/contains.js | 2 + test/commons/dom/get-target-rects.js | 22 ++++++- .../full/target-size/shadow-dom.html | 66 +++++++++++++++++++ .../full/target-size/shadow-dom.js | 45 +++++++++++++ 7 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 test/integration/full/target-size/shadow-dom.html create mode 100644 test/integration/full/target-size/shadow-dom.js diff --git a/.eslintrc.js b/.eslintrc.js index 71a39bc1ca..a1e87af600 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -59,13 +59,37 @@ module.exports = { selector: 'MemberExpression[property.name=tagName]', message: "Don't use node.tagName, use node.nodeName instead." }, + // node.attributes can be clobbered so is unsafe to use + // @see https://github.com/dequelabs/axe-core/pull/1432 { - // node.attributes can be clobbered so is unsafe to use - // @see https://github.com/dequelabs/axe-core/pull/1432 + // node.attributes selector: 'MemberExpression[object.name=node][property.name=attributes]', message: "Don't use node.attributes, use node.hasAttributes() or axe.utils.getNodeAttributes(node) instead." + }, + { + // vNode.actualNode.attributes + selector: + 'MemberExpression[object.property.name=actualNode][property.name=attributes]', + message: + "Don't use node.attributes, use node.hasAttributes() or axe.utils.getNodeAttributes(node) instead." + }, + // node.contains doesn't work with shadow dom + // @see https://github.com/dequelabs/axe-core/issues/4194 + { + // node.contains() + selector: + 'CallExpression[callee.object.name=node][callee.property.name=contains]', + message: + "Don't use node.contains(node2), use axe.utils.contains(node, node2) instead." + }, + { + // vNode.actualNode.contains() + selector: + 'CallExpression[callee.object.property.name=actualNode][callee.property.name=contains]', + message: + "Don't use node.contains(node2), use axe.utils.contains(node, node2) instead." } ] }, diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js index 6ee45bc9c5..8380304cf6 100644 --- a/lib/checks/mobile/target-size-evaluate.js +++ b/lib/checks/mobile/target-size-evaluate.js @@ -5,6 +5,7 @@ import { rectHasMinimumSize, hasVisualOverlap } from '../../commons/math'; +import { contains } from '../../core/utils'; /** * Determine if an element has a minimum size, taking into account @@ -185,9 +186,7 @@ function toDecimalSize(rect) { } function isDescendantNotInTabOrder(vAncestor, vNode) { - return ( - vAncestor.actualNode.contains(vNode.actualNode) && !isInTabOrder(vNode) - ); + return contains(vAncestor, vNode) && !isInTabOrder(vNode); } function mapActualNodes(vNodes) { diff --git a/lib/commons/dom/get-target-rects.js b/lib/commons/dom/get-target-rects.js index fc5b2aabe7..a2bc2eec12 100644 --- a/lib/commons/dom/get-target-rects.js +++ b/lib/commons/dom/get-target-rects.js @@ -2,6 +2,7 @@ import findNearbyElms from './find-nearby-elms'; import isInTabOrder from './is-in-tab-order'; import { splitRects, hasVisualOverlap } from '../math'; import memoize from '../../core/utils/memoize'; +import { contains } from '../../core/utils'; export default memoize(getTargetRects); @@ -32,7 +33,5 @@ function getTargetRects(vNode) { } function isDescendantNotInTabOrder(vAncestor, vNode) { - return ( - vAncestor.actualNode.contains(vNode.actualNode) && !isInTabOrder(vNode) - ); + return contains(vAncestor, vNode) && !isInTabOrder(vNode); } diff --git a/lib/core/utils/contains.js b/lib/core/utils/contains.js index dfea001e1f..6b05722c0b 100644 --- a/lib/core/utils/contains.js +++ b/lib/core/utils/contains.js @@ -12,8 +12,10 @@ export default function contains(vNode, otherVNode) { !vNode.shadowId && !otherVNode.shadowId && vNode.actualNode && + // eslint-disable-next-line no-restricted-syntax typeof vNode.actualNode.contains === 'function' ) { + // eslint-disable-next-line no-restricted-syntax return vNode.actualNode.contains(otherVNode.actualNode); } diff --git a/test/commons/dom/get-target-rects.js b/test/commons/dom/get-target-rects.js index f25abe2918..d13deda17b 100644 --- a/test/commons/dom/get-target-rects.js +++ b/test/commons/dom/get-target-rects.js @@ -1,6 +1,8 @@ describe('get-target-rects', () => { const getTargetRects = axe.commons.dom.getTargetRects; + const shadowSupport = axe.testUtils.shadowSupport.v1; const { queryFixture } = axe.testUtils; + const fixture = document.getElementById('fixture'); it('returns the bounding rect when unobscured', () => { const vNode = queryFixture(''); @@ -74,10 +76,28 @@ describe('get-target-rects', () => { `); const rects = getTargetRects(vNode); - console.log(JSON.stringify(rects)); assert.deepEqual(rects, [ new DOMRect(10, 5, 30, 7), new DOMRect(10, 5, 7, 40) ]); }); + + (shadowSupport ? it : xit)( + 'ignores non-tabbable descendants of the target that are in shadow dom', + () => { + fixture.innerHTML = + ''; + const target = fixture.querySelector('#target'); + const shadow = fixture + .querySelector('#shadow') + .attachShadow({ mode: 'open' }); + shadow.innerHTML = + '
'; + + axe.setup(fixture); + const vNode = axe.utils.getNodeFromTree(target); + const rects = getTargetRects(vNode); + assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]); + } + ); }); diff --git a/test/integration/full/target-size/shadow-dom.html b/test/integration/full/target-size/shadow-dom.html new file mode 100644 index 0000000000..7cef1b3bde --- /dev/null +++ b/test/integration/full/target-size/shadow-dom.html @@ -0,0 +1,66 @@ + + + + Target-size too many rects Test + + + + + + + +
+ + +
+
+ + +
Title
+ + +
+
+ +
Content
+ +
+
+ + + + + diff --git a/test/integration/full/target-size/shadow-dom.js b/test/integration/full/target-size/shadow-dom.js new file mode 100644 index 0000000000..0ede4e5ba6 --- /dev/null +++ b/test/integration/full/target-size/shadow-dom.js @@ -0,0 +1,45 @@ +describe('target-size shadow dom test', () => { + 'use strict'; + let results; + + before(done => { + axe.testUtils.awaitNestedLoad(() => { + const options = { + runOnly: ['target-size'], + elementRef: true + }; + const context = { + // ignore the mocha links + exclude: '#mocha' + }; + axe.run(context, options, (err, r) => { + if (err) { + done(err); + } + results = r; + console.log(results); + done(); + }); + }); + }); + + describe('violations', function () { + it('should find 0', function () { + assert.lengthOf(results.violations, 0); + }); + }); + + describe('passes', function () { + it('should find 2', function () { + assert.lengthOf(results.passes[0].nodes, 2); + }); + }); + + it('should find 0 inapplicable', function () { + assert.lengthOf(results.inapplicable, 0); + }); + + it('should find 0 incomplete', function () { + assert.lengthOf(results.incomplete, 0); + }); +}); From 0c1927eb2a9b97b9a2c8a4fdddbe42e5b71e17f9 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Wed, 17 Apr 2024 08:07:58 -0600 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Wilco Fiers --- test/commons/dom/get-target-rects.js | 2 +- test/integration/full/target-size/shadow-dom.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/commons/dom/get-target-rects.js b/test/commons/dom/get-target-rects.js index d13deda17b..1222f97c70 100644 --- a/test/commons/dom/get-target-rects.js +++ b/test/commons/dom/get-target-rects.js @@ -82,7 +82,7 @@ describe('get-target-rects', () => { ]); }); - (shadowSupport ? it : xit)( + it( 'ignores non-tabbable descendants of the target that are in shadow dom', () => { fixture.innerHTML = diff --git a/test/integration/full/target-size/shadow-dom.html b/test/integration/full/target-size/shadow-dom.html index 7cef1b3bde..cc5e33f732 100644 --- a/test/integration/full/target-size/shadow-dom.html +++ b/test/integration/full/target-size/shadow-dom.html @@ -1,7 +1,7 @@ - Target-size too many rects Test + Target-size shadow DOM test Date: Wed, 17 Apr 2024 08:54:19 -0600 Subject: [PATCH 3/3] suggestions --- .eslintrc.js | 4 +- test/checks/mobile/target-size.js | 122 ++++++++++-------- test/commons/dom/get-target-rects.js | 32 ++--- .../full/target-size/shadow-dom.js | 12 +- 4 files changed, 87 insertions(+), 83 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index a1e87af600..37592b6609 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -82,14 +82,14 @@ module.exports = { selector: 'CallExpression[callee.object.name=node][callee.property.name=contains]', message: - "Don't use node.contains(node2), use axe.utils.contains(node, node2) instead." + "Don't use node.contains(node2) as it doesn't work across shadow DOM. Use axe.utils.contains(node, node2) instead." }, { // vNode.actualNode.contains() selector: 'CallExpression[callee.object.property.name=actualNode][callee.property.name=contains]', message: - "Don't use node.contains(node2), use axe.utils.contains(node, node2) instead." + "Don't use node.contains(node2) as it doesn't work across shadow DOM. Use axe.utils.contains(node, node2) instead." } ] }, diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js index 688f809753..f0769c3c54 100644 --- a/test/checks/mobile/target-size.js +++ b/test/checks/mobile/target-size.js @@ -1,23 +1,22 @@ -describe('target-size tests', function () { - 'use strict'; - - var checkContext = axe.testUtils.MockCheckContext(); - var checkSetup = axe.testUtils.checkSetup; - var shadowCheckSetup = axe.testUtils.shadowCheckSetup; - var check = checks['target-size']; +describe('target-size tests', () => { + const checkContext = axe.testUtils.MockCheckContext(); + const checkSetup = axe.testUtils.checkSetup; + const shadowCheckSetup = axe.testUtils.shadowCheckSetup; + const check = checks['target-size']; + const fixture = document.querySelector('#fixture'); function elmIds(elms) { - return Array.from(elms).map(function (elm) { + return Array.from(elms).map(elm => { return '#' + elm.id; }); } - afterEach(function () { + afterEach(() => { checkContext.reset(); }); - it('returns false for targets smaller than minSize', function () { - var checkArgs = checkSetup( + it('returns false for targets smaller than minSize', () => { + const checkArgs = checkSetup( '' @@ -30,8 +29,8 @@ describe('target-size tests', function () { }); }); - it('returns undefined for non-tabbable targets smaller than minSize', function () { - var checkArgs = checkSetup( + it('returns undefined for non-tabbable targets smaller than minSize', () => { + const checkArgs = checkSetup( '' @@ -44,8 +43,8 @@ describe('target-size tests', function () { }); }); - it('returns true for unobscured targets larger than minSize', function () { - var checkArgs = checkSetup( + it('returns true for unobscured targets larger than minSize', () => { + const checkArgs = checkSetup( '' @@ -58,8 +57,8 @@ describe('target-size tests', function () { }); }); - it('returns true for very large targets', function () { - var checkArgs = checkSetup( + it('returns true for very large targets', () => { + const checkArgs = checkSetup( '' @@ -68,9 +67,9 @@ describe('target-size tests', function () { assert.deepEqual(checkContext._data, { messageKey: 'large', minSize: 24 }); }); - describe('when fully obscured', function () { - it('returns true, regardless of size', function () { - var checkArgs = checkSetup( + describe('when fully obscured', () => { + it('returns true, regardless of size', () => { + const checkArgs = checkSetup( 'x' + @@ -83,8 +82,8 @@ describe('target-size tests', function () { assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']); }); - it('returns true when obscured by another focusable widget', function () { - var checkArgs = checkSetup( + it('returns true when obscured by another focusable widget', () => { + const checkArgs = checkSetup( 'x' + @@ -97,8 +96,8 @@ describe('target-size tests', function () { assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']); }); - it('ignores obscuring element has pointer-events:none', function () { - var checkArgs = checkSetup( + it('ignores obscuring element has pointer-events:none', () => { + const checkArgs = checkSetup( 'x' + @@ -115,9 +114,9 @@ describe('target-size tests', function () { }); }); - describe('when partially obscured', function () { - it('returns true for focusable non-widgets', function () { - var checkArgs = checkSetup( + describe('when partially obscured', () => { + it('returns true for focusable non-widgets', () => { + const checkArgs = checkSetup( '' + @@ -137,8 +136,8 @@ describe('target-size tests', function () { assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']); }); - it('returns true for non-focusable widgets', function () { - var checkArgs = checkSetup( + it('returns true for non-focusable widgets', () => { + const checkArgs = checkSetup( '' + @@ -158,9 +157,9 @@ describe('target-size tests', function () { assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']); }); - describe('by a focusable widget', function () { - it('returns true for obscured targets with sufficient space', function () { - var checkArgs = checkSetup( + describe('by a focusable widget', () => { + it('returns true for obscured targets with sufficient space', () => { + const checkArgs = checkSetup( '' + @@ -202,8 +201,8 @@ describe('target-size tests', function () { }); describe('for obscured targets with insufficient space', () => { - it('returns false if all elements are tabbable', function () { - var checkArgs = checkSetup( + it('returns false if all elements are tabbable', () => { + const checkArgs = checkSetup( '' + @@ -227,8 +226,8 @@ describe('target-size tests', function () { ]); }); - it('returns undefined if the target is not tabbable', function () { - var checkArgs = checkSetup( + it('returns undefined if the target is not tabbable', () => { + const checkArgs = checkSetup( '' + @@ -252,8 +251,8 @@ describe('target-size tests', function () { ]); }); - it('returns undefined if the obscuring node is not tabbable', function () { - var checkArgs = checkSetup( + it('returns undefined if the obscuring node is not tabbable', () => { + const checkArgs = checkSetup( '' + @@ -279,8 +278,8 @@ describe('target-size tests', function () { }); describe('that is a descendant', () => { - it('returns false if the widget is tabbable', function () { - var checkArgs = checkSetup( + it('returns false if the widget is tabbable', () => { + const checkArgs = checkSetup( ` ` @@ -289,8 +288,8 @@ describe('target-size tests', function () { assert.isFalse(out); }); - it('returns true if the widget is not tabbable', function () { - var checkArgs = checkSetup( + it('returns true if the widget is not tabbable', () => { + const checkArgs = checkSetup( ` ` @@ -301,8 +300,8 @@ describe('target-size tests', function () { }); describe('that is a descendant', () => { - it('returns false if the widget is tabbable', function () { - var checkArgs = checkSetup( + it('returns false if the widget is tabbable', () => { + const checkArgs = checkSetup( ` ` @@ -311,8 +310,8 @@ describe('target-size tests', function () { assert.isFalse(out); }); - it('returns true if the widget is not tabbable', function () { - var checkArgs = checkSetup( + it('returns true if the widget is not tabbable', () => { + const checkArgs = checkSetup( ` ` @@ -324,9 +323,9 @@ describe('target-size tests', function () { }); }); - describe('with overflowing content', function () { + describe('with overflowing content', () => { it('returns undefined target is too small', () => { - var checkArgs = checkSetup( + const checkArgs = checkSetup( '' ); assert.isUndefined(check.evaluate.apply(checkContext, checkArgs)); @@ -337,7 +336,7 @@ describe('target-size tests', function () { }); it('returns true if target has sufficient size', () => { - var checkArgs = checkSetup( + const checkArgs = checkSetup( '' ); assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); @@ -345,7 +344,7 @@ describe('target-size tests', function () { describe('and partially obscured', () => { it('is undefined when unobscured area is too small', () => { - var checkArgs = checkSetup( + const checkArgs = checkSetup( '' + ' ' + '
' + @@ -359,7 +358,7 @@ describe('target-size tests', function () { }); it('is true when unobscured area is sufficient', () => { - var checkArgs = checkSetup( + const checkArgs = checkSetup( '' + ' ' + '
' + @@ -371,7 +370,7 @@ describe('target-size tests', function () { describe('and fully obscured', () => { it('is undefined', () => { - var checkArgs = checkSetup( + const checkArgs = checkSetup( '' + ' ' + '
' + @@ -386,8 +385,8 @@ describe('target-size tests', function () { }); }); - it('works across shadow boundaries', function () { - var checkArgs = shadowCheckSetup( + it('works across shadow boundaries', () => { + const checkArgs = shadowCheckSetup( '' + ''; + const target = fixture.querySelector('#target'); + const shadow = fixture + .querySelector('#shadow') + .attachShadow({ mode: 'open' }); + shadow.innerHTML = + '
'; + + axe.setup(fixture); + const vNode = axe.utils.getNodeFromTree(target); + assert.isTrue(check.evaluate.apply(checkContext, [target, {}, vNode])); + }); }); diff --git a/test/commons/dom/get-target-rects.js b/test/commons/dom/get-target-rects.js index 1222f97c70..be37b7cbb6 100644 --- a/test/commons/dom/get-target-rects.js +++ b/test/commons/dom/get-target-rects.js @@ -1,6 +1,5 @@ describe('get-target-rects', () => { const getTargetRects = axe.commons.dom.getTargetRects; - const shadowSupport = axe.testUtils.shadowSupport.v1; const { queryFixture } = axe.testUtils; const fixture = document.getElementById('fixture'); @@ -82,22 +81,19 @@ describe('get-target-rects', () => { ]); }); - it( - 'ignores non-tabbable descendants of the target that are in shadow dom', - () => { - fixture.innerHTML = - ''; - const target = fixture.querySelector('#target'); - const shadow = fixture - .querySelector('#shadow') - .attachShadow({ mode: 'open' }); - shadow.innerHTML = - '
'; + it('ignores non-tabbable descendants of the target that are in shadow dom', () => { + fixture.innerHTML = + ''; + const target = fixture.querySelector('#target'); + const shadow = fixture + .querySelector('#shadow') + .attachShadow({ mode: 'open' }); + shadow.innerHTML = + '
'; - axe.setup(fixture); - const vNode = axe.utils.getNodeFromTree(target); - const rects = getTargetRects(vNode); - assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]); - } - ); + axe.setup(fixture); + const vNode = axe.utils.getNodeFromTree(target); + const rects = getTargetRects(vNode); + assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]); + }); }); diff --git a/test/integration/full/target-size/shadow-dom.js b/test/integration/full/target-size/shadow-dom.js index 0ede4e5ba6..557ad32f94 100644 --- a/test/integration/full/target-size/shadow-dom.js +++ b/test/integration/full/target-size/shadow-dom.js @@ -3,7 +3,7 @@ describe('target-size shadow dom test', () => { let results; before(done => { - axe.testUtils.awaitNestedLoad(() => { + axe.testUtils.awaitNestedLoad(async () => { const options = { runOnly: ['target-size'], elementRef: true @@ -12,14 +12,8 @@ describe('target-size shadow dom test', () => { // ignore the mocha links exclude: '#mocha' }; - axe.run(context, options, (err, r) => { - if (err) { - done(err); - } - results = r; - console.log(results); - done(); - }); + results = await axe.run(context, options); + done(); }); });