From 77daf9e73cb21ad9a834657f16310bcd6d7df1b3 Mon Sep 17 00:00:00 2001
From: Steven Lambert <2433219+straker@users.noreply.github.com>
Date: Thu, 14 Mar 2024 16:36:14 -0600
Subject: [PATCH] fix(target-size): do not crash for nodes with many
overlapping widgets
---
lib/checks/mobile/target-offset-evaluate.js | 5 ++
lib/checks/mobile/target-size-evaluate.js | 57 +++++++++++++--------
lib/checks/mobile/target-size.json | 3 +-
lib/commons/math/split-rects.js | 7 +++
test/checks/mobile/target-size.js | 30 +++++++++++
test/commons/math/split-rects.js | 9 ++++
6 files changed, 89 insertions(+), 22 deletions(-)
diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js
index e799cb68f1..de361b1131 100644
--- a/lib/checks/mobile/target-offset-evaluate.js
+++ b/lib/checks/mobile/target-offset-evaluate.js
@@ -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) {
diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js
index a0224c8ad2..c88f4fed14 100644
--- a/lib/checks/mobile/target-size-evaluate.js
+++ b/lib/checks/mobile/target-size-evaluate.js
@@ -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);
@@ -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;
@@ -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'}`;
@@ -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;
@@ -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);
}
diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json
index 0fff548a65..5490d54479 100644
--- a/lib/checks/mobile/target-size.json
+++ b/lib/checks/mobile/target-size.json
@@ -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"
}
}
}
diff --git a/lib/commons/math/split-rects.js b/lib/commons/math/split-rects.js
index c7d45bafda..ca4ef232a6 100644
--- a/lib/commons/math/split-rects.js
+++ b/lib/commons/math/split-rects.js
@@ -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;
}
diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js
index f01a05841b..6d34482246 100644
--- a/test/checks/mobile/target-size.js
+++ b/test/checks/mobile/target-size.js
@@ -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 += `
+