Skip to content

Commit

Permalink
fix(target-size): do not crash for nodes with many overlapping widgets
Browse files Browse the repository at this point in the history
  • Loading branch information
straker committed Mar 14, 2024
1 parent de1baa9 commit 77daf9e
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 22 deletions.
5 changes: 5 additions & 0 deletions lib/checks/mobile/target-offset-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ export default function targetOffsetEvaluate(node, options, vNode) {
continue;
}
// the offset code works off radius but we want our messaging to reflect diameter

// getOffeset can return null now
// pass early if size is 10x (just like target-size)
// handle null as well

const offset =
roundToSingleDecimal(getOffset(vNode, vNeighbor, minOffset / 2)) * 2;
if (offset + roundingMargin >= minOffset) {
Expand Down
57 changes: 36 additions & 21 deletions lib/checks/mobile/target-size-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
* Determine if an element has a minimum size, taking into account
* any elements that may obscure it.
*/
export default function targetSize(node, options, vNode) {
export default function targetSizeEvaluate(node, options, vNode) {
const minSize = options?.minSize || 24;
const nodeRect = vNode.boundingClientRect;
const hasMinimumSize = rectHasMinimumSize.bind(null, minSize);
Expand All @@ -20,9 +20,22 @@ export default function targetSize(node, options, vNode) {
vNode,
nearbyElms
);
const hasOverflowingContent = () => {
this.data({ minSize, messageKey: 'contentOverflow' });
this.relatedNodes(mapActualNodes(overflowingContent));
return undefined;
};

// If the target has sufficient space it should pass instead of incomplete
if (
overflowingContent.length &&
(fullyObscuringElms.length || !hasMinimumSize(nodeRect))
) {
return hasOverflowingContent();
}

// Target is fully obscured and no overflowing content (which may not be obscured)
if (fullyObscuringElms.length && !overflowingContent.length) {
if (fullyObscuringElms.length) {
this.relatedNodes(mapActualNodes(fullyObscuringElms));
this.data({ messageKey: 'obscured' });
return true;
Expand All @@ -32,31 +45,32 @@ export default function targetSize(node, options, vNode) {
const negativeOutcome = isInTabOrder(vNode) ? false : undefined;

// Target is too small, and has no overflowing content that increases the size
if (!hasMinimumSize(nodeRect) && !overflowingContent.length) {
if (!hasMinimumSize(nodeRect)) {
this.data({ minSize, ...toDecimalSize(nodeRect) });
return negativeOutcome;
}

// Figure out the largest space on the target, not obscured by other widgets
const obscuredWidgets = filterFocusableWidgets(partialObscuringElms);
const largestInnerRect = getLargestUnobscuredArea(vNode, obscuredWidgets);

// Target has overflowing content;
// and is either not fully obscured (so may not pass),
// or has insufficient space (and so may not fail)
if (overflowingContent.length) {
if (
fullyObscuringElms.length ||
!hasMinimumSize(largestInnerRect || nodeRect)
) {
this.data({ minSize, messageKey: 'contentOverflow' });
this.relatedNodes(mapActualNodes(overflowingContent));
return undefined;
}
// Target not obscured and has sufficient space
if (!obscuredWidgets.length) {
this.data({ minSize, ...toDecimalSize(nodeRect) });
return true;
}

const largestInnerRect = getLargestUnobscuredArea(vNode, obscuredWidgets);
if (!largestInnerRect) {
this.data({ minSize, messageKey: 'tooManyRects' });
return undefined;
}

// Target is obscured, and insufficient space is left
if (obscuredWidgets.length !== 0 && !hasMinimumSize(largestInnerRect)) {
if (!hasMinimumSize(largestInnerRect)) {
if (overflowingContent.length) {
return hasOverflowingContent();
}

const allTabbable = obscuredWidgets.every(isInTabOrder);
const messageKey = `partiallyObscured${allTabbable ? '' : 'NonTabbable'}`;

Expand All @@ -65,7 +79,7 @@ export default function targetSize(node, options, vNode) {
return allTabbable ? negativeOutcome : undefined;
}

// Target not obscured, or has sufficient space
// Target has sufficient space
this.data({ minSize, ...toDecimalSize(largestInnerRect || nodeRect) });
this.relatedNodes(mapActualNodes(obscuredWidgets));
return true;
Expand Down Expand Up @@ -106,13 +120,14 @@ function filterByElmsOverlap(vNode, nearbyElms) {
// Find areas of the target that are not obscured
function getLargestUnobscuredArea(vNode, obscuredNodes) {
const nodeRect = vNode.boundingClientRect;
if (obscuredNodes.length === 0) {
return null;
}
const obscuringRects = obscuredNodes.map(
({ boundingClientRect: rect }) => rect
);
const unobscuredRects = splitRects(nodeRect, obscuringRects);
if (!unobscuredRects.length) {
return null;
}

// Of the unobscured inner rects, work out the largest
return getLargestRect(unobscuredRects);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/checks/mobile/target-size.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"default": "Element with negative tabindex has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
"contentOverflow": "Element size could not be accurately determined due to overflow content",
"partiallyObscured": "Element with negative tabindex has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
"partiallyObscuredNonTabbable": "Target has insufficient size because it is partially obscured by a neighbor with negative tabindex (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is the neighbor a target?"
"partiallyObscuredNonTabbable": "Target has insufficient size because it is partially obscured by a neighbor with negative tabindex (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is the neighbor a target?",
"tooManyRects": "Could not calculate calculate largest obscured rectangle as there are too many overlapping widgets"
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions lib/commons/math/split-rects.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ export default function splitRects(outerRect, overlapRects) {
uniqueRects = uniqueRects.reduce((rects, inputRect) => {
return rects.concat(splitRect(inputRect, overlapRect));
}, []);

// exit early if we get too many rects that it starts causing
// a performance bottleneck
// @see https://github.com/dequelabs/axe-core/issues/4359
if (uniqueRects.length > 4000) {
return [];
}
}
return uniqueRects;
}
Expand Down
30 changes: 30 additions & 0 deletions test/checks/mobile/target-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,36 @@ describe('target-size tests', function () {
assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']);
});

it('returns undefined if there are too many focusable widgets', () => {
let html = '';
for (let i = 0; i < 100; i++) {
html += `
<tr>
<td><a href="#">Hello</a></td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td><button>view</button></td>
<td><button>download</button></td>
<td><button>expand</button></td>
</tr>
`;
}
const checkArgs = checkSetup(`
<div id="target" role="tabpanel" tabindex="0">
<table id="tab-table">${html}</table>
</div>
`);
assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
assert.deepEqual(checkContext._data, {
messageKey: 'tooManyRects',
minSize: 24
});
});

describe('for obscured targets with insufficient space', () => {
it('returns false if all elements are tabbable', function () {
var checkArgs = checkSetup(
Expand Down
9 changes: 9 additions & 0 deletions test/commons/math/split-rects.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ describe('splitRects', () => {
assert.deepEqual(rects[0], rectA);
});

it('returns empty array if there are too many overlapping rects', () => {
const rects = [];
for (let i = 0; i < 100; i++) {
rects.push(new DOMRect(i, i, 50, 50));
}
const rectA = new DOMRect(0, 0, 1000, 1000);
assert.lengthOf(splitRects(rectA, rects), 0);
});

describe('with one overlapping rect', () => {
it('returns one rect if overlaps covers two corners', () => {
const rectA = new DOMRect(0, 0, 100, 50);
Expand Down

0 comments on commit 77daf9e

Please sign in to comment.