Skip to content

Commit

Permalink
fix(target-size): ignore descendant elements in shadow dom (#4410)
Browse files Browse the repository at this point in the history 
This also adds to the `.eslintrc` to error if `node.contains()` or
`vNode.actualNode.contains()` or used. (also upgraded the
`node.attributes` error to account for `vNode.actualNode.attributes` as
I noticed it was missing).

Closes: #4194

---------

Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
  • Loading branch information
@straker @WilcoFiers
straker and WilcoFiers authored Apr 23, 2024
1 parent 3a90bb7 commit 6091367
Show file tree
Hide file tree
Showing 8 changed files with 222 additions and 63 deletions.
28 changes: 26 additions & 2 deletions .eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) 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) as it doesn't work across shadow DOM. Use axe.utils.contains(node, node2) instead."
}
]
},
Expand Down
5 changes: 2 additions & 3 deletions lib/checks/mobile/target-size-evaluate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -187,9 +188,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) {
Expand Down
5 changes: 2 additions & 3 deletions lib/commons/dom/get-target-rects.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions lib/core/utils/contains.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
122 changes: 68 additions & 54 deletions test/checks/mobile/target-size.js
View file
Original file line number Diff line number Diff line change
@@ -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(
'<button id="target" style="' +
'display: inline-block; width:20px; height:30px;' +
'">x</button>'
Expand All @@ -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(
'<button id="target" tabindex="-1" style="' +
'display: inline-block; width:20px; height:30px;' +
'">x</button>'
Expand All @@ -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(
'<button id="target" style="' +
'display: inline-block; width:40px; height:30px;' +
'">x</button>'
Expand All @@ -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(
'<button id="target" style="' +
'display: inline-block; width:240px; height:300px;' +
'">x</button>'
Expand All @@ -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(
'<a href="#" id="target" style="' +
'display: inline-block; width:20px; height:20px;' +
'">x</a>' +
Expand All @@ -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(
'<a href="#" id="target" style="' +
'display: inline-block; width:20px; height:20px;' +
'">x</a>' +
Expand All @@ -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(
'<a href="#" id="target" style="' +
'display: inline-block; width:20px; height:20px;' +
'">x</a>' +
Expand All @@ -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(
'<button id="target" style="' +
'display: inline-block; width:40px; height:30px; margin-left:30px;' +
'">x</button>' +
Expand All @@ -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(
'<button id="target" style="' +
'display: inline-block; width:40px; height:30px; margin-left:30px;' +
'">x</button>' +
Expand All @@ -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(
'<button id="target" style="' +
'display: inline-block; width:40px; height:30px;' +
'">x</button>' +
Expand Down Expand Up @@ -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(
'<button id="target" style="' +
'display: inline-block; width:40px; height:30px; margin-left:30px;' +
'">x</button>' +
Expand All @@ -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(
'<button id="target" tabindex="-1" style="' +
'display: inline-block; width:40px; height:30px; margin-left:30px;' +
'">x</button>' +
Expand All @@ -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(
'<button id="target" style="' +
'display: inline-block; width:40px; height:30px; margin-left:30px;' +
'">x</button>' +
Expand All @@ -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(
`<a role="link" aria-label="play" tabindex="0" style="display:inline-block" id="target">
<button style="margin:1px; line-height:20px">Play</button>
</a>`
Expand All @@ -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(
`<a role="link" aria-label="play" tabindex="0" style="display:inline-block" id="target">
<button tabindex="-1" style="margin:1px; line-height:20px">Play</button>
</a>`
Expand All @@ -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(
`<a role="link" aria-label="play" tabindex="0" style="display:inline-block" id="target">
<button style="margin:1px; line-height:20px">Play</button>
</a>`
Expand All @@ -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(
`<a role="link" aria-label="play" tabindex="0" style="display:inline-block" id="target">
<button tabindex="-1" style="margin:1px; line-height:20px">Play</button>
</a>`
Expand All @@ -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(
'<a href="#" id="target"><img width="24" height="24"></a>'
);
assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
Expand All @@ -337,15 +336,15 @@ describe('target-size tests', function () {
});

it('returns true if target has sufficient size', () => {
var checkArgs = checkSetup(
const checkArgs = checkSetup(
'<a href="#" id="target" style="font-size:24px;"><img width="24" height="24"></a>'
);
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
});

describe('and partially obscured', () => {
it('is undefined when unobscured area is too small', () => {
var checkArgs = checkSetup(
const checkArgs = checkSetup(
'<a href="#" id="target" style="font-size:24px;">' +
' <img width="24" height="36" style="vertical-align: bottom;">' +
'</a><br>' +
Expand All @@ -359,7 +358,7 @@ describe('target-size tests', function () {
});

it('is true when unobscured area is sufficient', () => {
var checkArgs = checkSetup(
const checkArgs = checkSetup(
'<a href="#" id="target" style="font-size:24px;">' +
' <img width="24" height="36" style="vertical-align: bottom;">' +
'</a><br>' +
Expand All @@ -371,7 +370,7 @@ describe('target-size tests', function () {

describe('and fully obscured', () => {
it('is undefined', () => {
var checkArgs = checkSetup(
const checkArgs = checkSetup(
'<a href="#" id="target" style="font-size:24px;">' +
' <img width="24" height="36" style="vertical-align: bottom;">' +
'</a><br>' +
Expand All @@ -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(
'<span id="shadow"></span>' +
'<button id="obscurer1" style="' +
'display: inline-block; width:40px; height:30px; margin-left: -10px;' +
Expand All @@ -411,4 +410,19 @@ describe('target-size tests', function () {
'#obscurer2'
]);
});

it('ignores descendants of the target that are in shadow dom', () => {
fixture.innerHTML =
'<button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px"><span id="shadow"></span></button>';
const target = fixture.querySelector('#target');
const shadow = fixture
.querySelector('#shadow')
.attachShadow({ mode: 'open' });
shadow.innerHTML =
'<div style="position: absolute; left: 5px; top: 5px; width: 50px; height: 50px;"></div>';

axe.setup(fixture);
const vNode = axe.utils.getNodeFromTree(target);
assert.isTrue(check.evaluate.apply(checkContext, [target, {}, vNode]));
});
});
Loading

0 comments on commit 6091367

Please sign in to comment.