From 1494b4c2159fbae2a937cc7c3dc1d269915ef4d4 Mon Sep 17 00:00:00 2001
From: Steven Lambert <2433219+straker@users.noreply.github.com>
Date: Mon, 21 Aug 2023 10:54:01 -0500
Subject: [PATCH] fix(target-size): correctly calculate bounding box (#4125)
* fix(target-size): correctly calculate bounding box
* test rectHasMinimumSize
* return early
* comment and refactor
* text
* use neighbor target size
* suggestions
* fix bug
---
lib/checks/mobile/target-size-evaluate.js | 14 ++--
lib/commons/dom/get-target-rects.js | 38 +++++++++
lib/commons/dom/get-target-size.js | 45 +---------
lib/commons/dom/index.js | 1 +
lib/commons/math/get-offset.js | 61 +++++++++-----
lib/commons/math/index.js | 1 +
lib/commons/math/rect-has-minimum-size.js | 7 ++
test/commons/dom/get-target-rects.js | 83 +++++++++++++++++++
test/commons/dom/get-target-size.js | 4 +-
test/commons/math/get-offset.js | 10 +++
test/commons/math/rect-has-minimum-size.js | 28 +++++++
.../full/target-size/target-size.html | 22 +++--
12 files changed, 230 insertions(+), 84 deletions(-)
create mode 100644 lib/commons/dom/get-target-rects.js
create mode 100644 lib/commons/math/rect-has-minimum-size.js
create mode 100644 test/commons/dom/get-target-rects.js
create mode 100644 test/commons/math/rect-has-minimum-size.js
diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js
index 4f18351adf..a0224c8ad2 100644
--- a/lib/checks/mobile/target-size-evaluate.js
+++ b/lib/checks/mobile/target-size-evaluate.js
@@ -1,8 +1,10 @@
import { findNearbyElms, isFocusable, isInTabOrder } from '../../commons/dom';
import { getRoleType } from '../../commons/aria';
-import { splitRects, hasVisualOverlap } from '../../commons/math';
-
-const roundingMargin = 0.05;
+import {
+ splitRects,
+ rectHasMinimumSize,
+ hasVisualOverlap
+} from '../../commons/math';
/**
* Determine if an element has a minimum size, taking into account
@@ -165,12 +167,6 @@ function isDescendantNotInTabOrder(vAncestor, vNode) {
);
}
-function rectHasMinimumSize(minSize, { width, height }) {
- return (
- width + roundingMargin >= minSize && height + roundingMargin >= minSize
- );
-}
-
function mapActualNodes(vNodes) {
return vNodes.map(({ actualNode }) => actualNode);
}
diff --git a/lib/commons/dom/get-target-rects.js b/lib/commons/dom/get-target-rects.js
new file mode 100644
index 0000000000..fc5b2aabe7
--- /dev/null
+++ b/lib/commons/dom/get-target-rects.js
@@ -0,0 +1,38 @@
+import findNearbyElms from './find-nearby-elms';
+import isInTabOrder from './is-in-tab-order';
+import { splitRects, hasVisualOverlap } from '../math';
+import memoize from '../../core/utils/memoize';
+
+export default memoize(getTargetRects);
+
+/**
+ * Return all unobscured rects of a target.
+ * @see https://www.w3.org/TR/WCAG22/#dfn-bounding-boxes
+ * @param {VitualNode} vNode
+ * @return {DOMRect[]}
+ */
+function getTargetRects(vNode) {
+ const nodeRect = vNode.boundingClientRect;
+ const overlappingVNodes = findNearbyElms(vNode).filter(vNeighbor => {
+ return (
+ hasVisualOverlap(vNode, vNeighbor) &&
+ vNeighbor.getComputedStylePropertyValue('pointer-events') !== 'none' &&
+ !isDescendantNotInTabOrder(vNode, vNeighbor)
+ );
+ });
+
+ if (!overlappingVNodes.length) {
+ return [nodeRect];
+ }
+
+ const obscuringRects = overlappingVNodes.map(
+ ({ boundingClientRect: rect }) => rect
+ );
+ return splitRects(nodeRect, obscuringRects);
+}
+
+function isDescendantNotInTabOrder(vAncestor, vNode) {
+ return (
+ vAncestor.actualNode.contains(vNode.actualNode) && !isInTabOrder(vNode)
+ );
+}
diff --git a/lib/commons/dom/get-target-size.js b/lib/commons/dom/get-target-size.js
index f16c245227..926bbc0d0d 100644
--- a/lib/commons/dom/get-target-size.js
+++ b/lib/commons/dom/get-target-size.js
@@ -1,9 +1,7 @@
-import findNearbyElms from './find-nearby-elms';
-import { splitRects, hasVisualOverlap } from '../math';
+import getTargetRects from './get-target-rects';
+import { rectHasMinimumSize } from '../math';
import memoize from '../../core/utils/memoize';
-const roundingMargin = 0.05;
-
export default memoize(getTargetSize);
/**
@@ -11,37 +9,8 @@ export default memoize(getTargetSize);
* @see https://www.w3.org/TR/WCAG22/#dfn-targets
*/
function getTargetSize(vNode, minSize) {
- const nodeRect = vNode.boundingClientRect;
- const overlappingVNodes = findNearbyElms(vNode).filter(vNeighbor => {
- return (
- vNeighbor.getComputedStylePropertyValue('pointer-events') !== 'none' &&
- hasVisualOverlap(vNode, vNeighbor)
- );
- });
-
- if (!overlappingVNodes.length) {
- return nodeRect;
- }
-
- return getLargestUnobscuredArea(vNode, overlappingVNodes, minSize);
-}
-
-// Find areas of the target that are not obscured
-function getLargestUnobscuredArea(vNode, obscuredNodes, minSize) {
- 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, minSize);
+ const rects = getTargetRects(vNode);
+ return getLargestRect(rects, minSize);
}
// Find the largest rectangle in the array, prioritize ones that meet a minimum size
@@ -58,9 +27,3 @@ function getLargestRect(rects, minSize) {
return areaA > areaB ? rectA : rectB;
});
}
-
-function rectHasMinimumSize(minSize, { width, height }) {
- return (
- width + roundingMargin >= minSize && height + roundingMargin >= minSize
- );
-}
diff --git a/lib/commons/dom/index.js b/lib/commons/dom/index.js
index fbac8213f0..57ae0756c9 100644
--- a/lib/commons/dom/index.js
+++ b/lib/commons/dom/index.js
@@ -17,6 +17,7 @@ export { default as getOverflowHiddenAncestors } from './get-overflow-hidden-anc
export { default as getRootNode } from './get-root-node';
export { default as getScrollOffset } from './get-scroll-offset';
export { default as getTabbableElements } from './get-tabbable-elements';
+export { default as getTargetRects } from './get-target-rects';
export { default as getTargetSize } from './get-target-size';
export { default as getTextElementStack } from './get-text-element-stack';
export { default as getViewportSize } from './get-viewport-size';
diff --git a/lib/commons/math/get-offset.js b/lib/commons/math/get-offset.js
index 65cba9b0e3..664871fda0 100644
--- a/lib/commons/math/get-offset.js
+++ b/lib/commons/math/get-offset.js
@@ -1,38 +1,53 @@
-import { getTargetSize } from '../dom';
+import { getTargetRects, getTargetSize } from '../dom';
+import { getBoundingRect } from './get-bounding-rect';
+import { isPointInRect } from './is-point-in-rect';
+import { getRectCenter } from './get-rect-center';
+import rectHasMinimumSize from './rect-has-minimum-size';
/**
- * Get the offset between node A and node B
+ * Get the offset between target A and neighbor B. This assumes that the target is undersized and needs to check the spacing exception.
* @method getOffset
* @memberof axe.commons.math
- * @param {VirtualNode} vNodeA
- * @param {VirtualNode} vNodeB
+ * @param {VirtualNode} vTarget
+ * @param {VirtualNode} vNeighbor
* @param {Number} radius
* @returns {number}
*/
-export default function getOffset(vNodeA, vNodeB, minRadiusNeighbour = 12) {
- const rectA = getTargetSize(vNodeA);
- const rectB = getTargetSize(vNodeB);
+export default function getOffset(vTarget, vNeighbor, minRadiusNeighbour = 12) {
+ const targetRects = getTargetRects(vTarget);
+ const neighborRects = getTargetRects(vNeighbor);
- // one of the rects is fully obscured
- if (rectA === null || rectB === null) {
+ if (!targetRects.length || !neighborRects.length) {
return 0;
}
- const centerA = {
- x: rectA.x + rectA.width / 2,
- y: rectA.y + rectA.height / 2
- };
- const centerB = {
- x: rectB.x + rectB.width / 2,
- y: rectB.y + rectB.height / 2
- };
- const sideB = getClosestPoint(centerA, rectB);
+ const targetBoundingBox = targetRects.reduce(getBoundingRect);
+ const targetCenter = getRectCenter(targetBoundingBox);
- return Math.min(
- // subtract the radius of the circle from the distance
- pointDistance(centerA, centerB) - minRadiusNeighbour,
- pointDistance(centerA, sideB)
- );
+ // calculate distance to the closest edge of each neighbor rect
+ let minDistance = Infinity;
+ for (const rect of neighborRects) {
+ if (isPointInRect(targetCenter, rect)) {
+ return 0;
+ }
+
+ const closestPoint = getClosestPoint(targetCenter, rect);
+ const distance = pointDistance(targetCenter, closestPoint);
+ minDistance = Math.min(minDistance, distance);
+ }
+
+ const neighborTargetSize = getTargetSize(vNeighbor);
+ if (rectHasMinimumSize(minRadiusNeighbour * 2, neighborTargetSize)) {
+ return minDistance;
+ }
+
+ const neighborBoundingBox = neighborRects.reduce(getBoundingRect);
+ const neighborCenter = getRectCenter(neighborBoundingBox);
+ // subtract the radius of the circle from the distance to center to get distance to edge of the neighbor circle
+ const centerDistance =
+ pointDistance(targetCenter, neighborCenter) - minRadiusNeighbour;
+
+ return Math.max(0, Math.min(minDistance, centerDistance));
}
function getClosestPoint(point, rect) {
diff --git a/lib/commons/math/index.js b/lib/commons/math/index.js
index b6edc3d4aa..88ac3a9cdc 100644
--- a/lib/commons/math/index.js
+++ b/lib/commons/math/index.js
@@ -4,5 +4,6 @@ export { default as getOffset } from './get-offset';
export { getRectCenter } from './get-rect-center';
export { default as hasVisualOverlap } from './has-visual-overlap';
export { isPointInRect } from './is-point-in-rect';
+export { default as rectHasMinimumSize } from './rect-has-minimum-size';
export { default as rectsOverlap } from './rects-overlap';
export { default as splitRects } from './split-rects';
diff --git a/lib/commons/math/rect-has-minimum-size.js b/lib/commons/math/rect-has-minimum-size.js
new file mode 100644
index 0000000000..014a888e35
--- /dev/null
+++ b/lib/commons/math/rect-has-minimum-size.js
@@ -0,0 +1,7 @@
+const roundingMargin = 0.05;
+
+export default function rectHasMinimumSize(minSize, { width, height }) {
+ return (
+ width + roundingMargin >= minSize && height + roundingMargin >= minSize
+ );
+}
diff --git a/test/commons/dom/get-target-rects.js b/test/commons/dom/get-target-rects.js
new file mode 100644
index 0000000000..f25abe2918
--- /dev/null
+++ b/test/commons/dom/get-target-rects.js
@@ -0,0 +1,83 @@
+describe('get-target-rects', () => {
+ const getTargetRects = axe.commons.dom.getTargetRects;
+ const { queryFixture } = axe.testUtils;
+
+ it('returns the bounding rect when unobscured', () => {
+ const vNode = queryFixture('');
+ const rects = getTargetRects(vNode);
+ assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+ });
+
+ it('returns subset rect when obscured', () => {
+ const vNode = queryFixture(`
+
+
+ `);
+ const rects = getTargetRects(vNode);
+ assert.deepEqual(rects, [new DOMRect(10, 5, 20, 40)]);
+ });
+
+ it('ignores elements with "pointer-events: none"', () => {
+ const vNode = queryFixture(`
+
+
+ `);
+ const rects = getTargetRects(vNode);
+ assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+ });
+
+ it("ignores elements that don't overlap the target", () => {
+ const vNode = queryFixture(`
+
+
+ `);
+ const rects = getTargetRects(vNode);
+ assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+ });
+
+ it('ignores non-tabbable descendants of the target', () => {
+ const vNode = queryFixture(`
+
+ `);
+ const rects = getTargetRects(vNode);
+ assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+ });
+
+ it('returns each unobscured area', () => {
+ const vNode = queryFixture(`
+
+
+
+ `);
+ const rects = getTargetRects(vNode);
+ assert.deepEqual(rects, [
+ new DOMRect(10, 5, 20, 15),
+ new DOMRect(10, 30, 20, 15)
+ ]);
+ });
+
+ it('returns empty if target is fully obscured', () => {
+ const vNode = queryFixture(`
+
+
+ `);
+ const rects = getTargetRects(vNode);
+ assert.lengthOf(rects, 0);
+ });
+
+ it('returns subset rect of the target with tabbable descendant', () => {
+ const vNode = queryFixture(`
+
+ `);
+ const rects = getTargetRects(vNode);
+ console.log(JSON.stringify(rects));
+ assert.deepEqual(rects, [
+ new DOMRect(10, 5, 30, 7),
+ new DOMRect(10, 5, 7, 40)
+ ]);
+ });
+});
diff --git a/test/commons/dom/get-target-size.js b/test/commons/dom/get-target-size.js
index ddb876f7aa..10bda51300 100644
--- a/test/commons/dom/get-target-size.js
+++ b/test/commons/dom/get-target-size.js
@@ -23,7 +23,7 @@ describe('get-target-size', () => {
`);
const rect = getTargetSize(vNode);
- assert.deepEqual(rect, new DOMRect(10, 5, 30, 40));
+ assert.deepEqual(rect, vNode.actualNode.getBoundingClientRect());
});
it("ignores elements that don't overlap the target", () => {
@@ -32,7 +32,7 @@ describe('get-target-size', () => {
`);
const rect = getTargetSize(vNode);
- assert.deepEqual(rect, new DOMRect(10, 5, 30, 40));
+ assert.deepEqual(rect, vNode.actualNode.getBoundingClientRect());
});
it('returns the largest unobscured area', () => {
diff --git a/test/commons/math/get-offset.js b/test/commons/math/get-offset.js
index cdbd1a5682..073c8d0649 100644
--- a/test/commons/math/get-offset.js
+++ b/test/commons/math/get-offset.js
@@ -62,4 +62,14 @@ describe('getOffset', () => {
const nodeB = fixture.children[3];
assert.closeTo(getOffset(nodeA, nodeB, 30), 20, round);
});
+
+ it('returns 0 if center of nodeA is enclosed by nodeB', () => {
+ const fixture = fixtureSetup(`
+
+
+ `);
+ const nodeA = fixture.children[1];
+ const nodeB = fixture.children[3];
+ assert.equal(getOffset(nodeA, nodeB, 30), 0);
+ });
});
diff --git a/test/commons/math/rect-has-minimum-size.js b/test/commons/math/rect-has-minimum-size.js
new file mode 100644
index 0000000000..7d93b5c83a
--- /dev/null
+++ b/test/commons/math/rect-has-minimum-size.js
@@ -0,0 +1,28 @@
+describe('rectHasMinimumSize', () => {
+ const rectHasMinimumSize = axe.commons.math.rectHasMinimumSize;
+
+ it('returns true if rect is large enough', () => {
+ const rect = new DOMRect(10, 20, 10, 20);
+ assert.isTrue(rectHasMinimumSize(10, rect));
+ });
+
+ it('returns true for rounding margin', () => {
+ const rect = new DOMRect(10, 20, 9.95, 20);
+ assert.isTrue(rectHasMinimumSize(10, rect));
+ });
+
+ it('returns false if width is too small', () => {
+ const rect = new DOMRect(10, 20, 5, 20);
+ assert.isFalse(rectHasMinimumSize(10, rect));
+ });
+
+ it('returns false if height is too small', () => {
+ const rect = new DOMRect(10, 20, 10, 5);
+ assert.isFalse(rectHasMinimumSize(10, rect));
+ });
+
+ it('returns false when below rounding margin', () => {
+ const rect = new DOMRect(10, 20, 9.94, 20);
+ assert.isFalse(rectHasMinimumSize(10, rect));
+ });
+});
diff --git a/test/integration/full/target-size/target-size.html b/test/integration/full/target-size/target-size.html
index d6a1b2f873..9358f9ab81 100644
--- a/test/integration/full/target-size/target-size.html
+++ b/test/integration/full/target-size/target-size.html
@@ -177,6 +177,9 @@
.ml-n6 {
margin-left: -36px;
}
+ .ml-n5 {
+ margin-left: -30px;
+ }
.ml-n4 {
margin-left: -24px;
}
@@ -456,13 +459,12 @@
-
+
-
-
+
@@ -481,7 +483,7 @@
-
+
@@ -496,26 +498,26 @@
- Example K1 - K3 pass, the middle element of K4 fails.
+ Example K1 and K2 pass, K3 and K4 fail.
@@ -588,6 +590,7 @@
+
@@ -640,6 +643,7 @@
+