Skip to content

Commit 69d81c1

Browse files
committed
fix(target-size): determine offset using clientRects if target is display:inline (#5012)
This pr does two things: 1. For target size we decided to calculate the visible unobscured rects by taking the bounding rect of the target and subtracting the client rects of the obscurer if the obscurer is inline. This is because an inline element only accepts pointer events on the client rects and not the bounding rect. For the target we still take the bounding rect and not the client rects if the target is inline because we both agreed that the goal of the target size rule is to prevent clicking on the wrong target and not that the target itself is large enough to click on. So an inline element with multiple lines would have dead pointer zones within it, but accidentally clicking on a dead zone is not a failure of target size. 2. This also applies using client rects for inline elements for the spacing exception for the same reasons as above. What this does is allow checking multiple target rects for inline nodes and makes sure each rect passes the spacing exception. If at least one rect does not pass the rule fails for that element. Closes: #4928
1 parent 99d1e77 commit 69d81c1

10 files changed

Lines changed: 176 additions & 36 deletions

File tree

lib/checks/mobile/target-size-evaluate.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,11 @@ export default function targetSizeEvaluate(node, options, vNode) {
6666
return true;
6767
}
6868

69-
const largestInnerRect = getLargestUnobscuredArea(vNode, obscuredWidgets);
69+
const largestInnerRect = getLargestUnobscuredArea(
70+
vNode,
71+
obscuredWidgets,
72+
minSize
73+
);
7074
if (!largestInnerRect) {
7175
this.data({ minSize, messageKey: 'tooManyRects' });
7276
return undefined;
@@ -127,11 +131,16 @@ function filterByElmsOverlap(vNode, nearbyElms) {
127131
}
128132

129133
// Find areas of the target that are not obscured
130-
function getLargestUnobscuredArea(vNode, obscuredNodes) {
134+
function getLargestUnobscuredArea(vNode, obscuredNodes, minSize) {
131135
const nodeRect = vNode.boundingClientRect;
132-
const obscuringRects = obscuredNodes.map(
133-
({ boundingClientRect: rect }) => rect
134-
);
136+
const obscuringRects = obscuredNodes
137+
.map(obscuredNode => {
138+
const display = obscuredNode.getComputedStylePropertyValue('display');
139+
return display === 'inline'
140+
? obscuredNode.clientRects
141+
: obscuredNode.boundingClientRect;
142+
})
143+
.flat(Infinity);
135144
let unobscuredRects;
136145
try {
137146
unobscuredRects = splitRects(nodeRect, obscuringRects);
@@ -140,7 +149,7 @@ function getLargestUnobscuredArea(vNode, obscuredNodes) {
140149
}
141150

142151
// Of the unobscured inner rects, work out the largest
143-
return getLargestRect(unobscuredRects);
152+
return getLargestRect(unobscuredRects, minSize);
144153
}
145154

146155
// Find the largest rectangle in the array, prioritize ones that meet a minimum size

lib/commons/dom/get-target-rects.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ export default memoize(getTargetRects);
1313
* @return {DOMRect[]}
1414
*/
1515
function getTargetRects(vNode) {
16-
const nodeRect = vNode.boundingClientRect;
16+
const display = vNode.getComputedStylePropertyValue('display');
17+
const nodeRects =
18+
display === 'inline' ? vNode.clientRects : [vNode.boundingClientRect];
1719
const overlappingVNodes = findNearbyElms(vNode).filter(vNeighbor => {
1820
return (
1921
hasVisualOverlap(vNode, vNeighbor) &&
@@ -23,13 +25,19 @@ function getTargetRects(vNode) {
2325
});
2426

2527
if (!overlappingVNodes.length) {
26-
return [nodeRect];
28+
return nodeRects;
2729
}
2830

29-
const obscuringRects = overlappingVNodes.map(
30-
({ boundingClientRect: rect }) => rect
31-
);
32-
return splitRects(nodeRect, obscuringRects);
31+
const obscuringRects = overlappingVNodes
32+
.map(overlappingVNode => {
33+
const overlappingDisplay =
34+
overlappingVNode.getComputedStylePropertyValue('display');
35+
return overlappingDisplay === 'inline'
36+
? overlappingVNode.clientRects
37+
: overlappingVNode.boundingClientRect;
38+
})
39+
.flat(Infinity);
40+
return splitRects(nodeRects, obscuringRects);
3341
}
3442

3543
function isDescendantNotInTabOrder(vAncestor, vNode) {

lib/commons/math/get-offset.js

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,33 +21,33 @@ export default function getOffset(vTarget, vNeighbor, minRadiusNeighbour = 12) {
2121
return null;
2222
}
2323

24-
const targetBoundingBox = targetRects.reduce(getBoundingRect);
25-
const targetCenter = getRectCenter(targetBoundingBox);
24+
// calculate distance of each target rect to the closest edge of each neighbor rect
25+
return targetRects.reduce((minDistance, targetRect) => {
26+
const targetCenter = getRectCenter(targetRect);
2627

27-
// calculate distance to the closest edge of each neighbor rect
28-
let minDistance = Infinity;
29-
for (const rect of neighborRects) {
30-
if (isPointInRect(targetCenter, rect)) {
31-
return 0;
32-
}
28+
for (const rect of neighborRects) {
29+
if (isPointInRect(targetCenter, rect)) {
30+
return 0;
31+
}
3332

34-
const closestPoint = getClosestPoint(targetCenter, rect);
35-
const distance = pointDistance(targetCenter, closestPoint);
36-
minDistance = Math.min(minDistance, distance);
37-
}
33+
const closestPoint = getClosestPoint(targetCenter, rect);
34+
const distance = pointDistance(targetCenter, closestPoint);
35+
minDistance = Math.min(minDistance, distance);
36+
}
3837

39-
const neighborTargetSize = getTargetSize(vNeighbor);
40-
if (rectHasMinimumSize(minRadiusNeighbour * 2, neighborTargetSize)) {
41-
return minDistance;
42-
}
38+
const neighborTargetSize = getTargetSize(vNeighbor);
39+
if (rectHasMinimumSize(minRadiusNeighbour * 2, neighborTargetSize)) {
40+
return minDistance;
41+
}
4342

44-
const neighborBoundingBox = neighborRects.reduce(getBoundingRect);
45-
const neighborCenter = getRectCenter(neighborBoundingBox);
46-
// subtract the radius of the circle from the distance to center to get distance to edge of the neighbor circle
47-
const centerDistance =
48-
pointDistance(targetCenter, neighborCenter) - minRadiusNeighbour;
43+
const neighborBoundingBox = neighborRects.reduce(getBoundingRect);
44+
const neighborCenter = getRectCenter(neighborBoundingBox);
45+
// subtract the radius of the circle from the distance to center to get distance to edge of the neighbor circle
46+
const centerDistance =
47+
pointDistance(targetCenter, neighborCenter) - minRadiusNeighbour;
4948

50-
return Math.max(0, Math.min(minDistance, centerDistance));
49+
return Math.max(0, Math.min(minDistance, centerDistance));
50+
}, Infinity);
5151
}
5252

5353
function getClosestPoint(point, rect) {

lib/commons/math/split-rects.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@
33
* space that does not overlap.
44
* @method getOffset
55
* @memberof axe.commons.math
6-
* @param {DOMRect} outerRect
6+
* @param {DOMRect|DOMRect[]} outerRect
77
* @param {DOMRect[]} overlapRects
88
* @returns {DOMRect[]} Unique array of rects
99
*/
1010
export default function splitRects(outerRect, overlapRects) {
11-
let uniqueRects = [outerRect];
11+
let uniqueRects = Array.isArray(outerRect) ? outerRect : [outerRect];
1212
for (const overlapRect of overlapRects) {
1313
uniqueRects = uniqueRects.reduce((rects, inputRect) => {
1414
return rects.concat(splitRect(inputRect, overlapRect));

test/checks/mobile/target-offset.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,19 @@ describe('target-offset tests', () => {
3434
assert.closeTo(checkContext._data.closestOffset, 24, 0.2);
3535
});
3636

37+
it('returns true when wrapped inline elements offset is 24px', () => {
38+
const checkArgs = checkSetup(`
39+
<div style="font-size: 18px; margin: 1em auto; width: 6em; line-height: 1.3;">
40+
<a id="target" href="/foo" class="A"> Hello hello hello</a>
41+
<a href="/bar" class="B">Hello hello hello</a>
42+
</div>
43+
`);
44+
45+
assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
46+
assert.equal(checkContext._data.minOffset, 24);
47+
assert.closeTo(checkContext._data.closestOffset, 24, 0.2);
48+
});
49+
3750
describe('when the offset is insufficient', () => {
3851
it('returns false for targets in the tab order', () => {
3952
const checkArgs = checkSetup(
@@ -66,6 +79,32 @@ describe('target-offset tests', () => {
6679
assert.equal(checkContext._data.minOffset, 24);
6780
assert.closeTo(checkContext._data.closestOffset, 22, 0.2);
6881
});
82+
83+
it('returns false when wrapped inline elements offset is <24px', () => {
84+
const checkArgs = checkSetup(`
85+
<div style="font-size: 18px; margin: 1em auto; width: 6em; line-height: 1.1;">
86+
<a id="target" href="/foo" class="A"> Hello hello hello</a>
87+
<a href="/bar" class="B">Hello hello hello</a>
88+
</div>
89+
`);
90+
91+
assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
92+
assert.equal(checkContext._data.minOffset, 24);
93+
assert.closeTo(checkContext._data.closestOffset, 20, 3);
94+
});
95+
96+
it('returns false when one line of a wrapped inline elements offset is <24px', () => {
97+
const checkArgs = checkSetup(`
98+
<div style="font-size: 18px; margin: 1em auto; width: 6em; line-height: 1.3;">
99+
<a id="target" href="/foo" class="A"> Hello hello hello</a>
100+
<a style="margin-left: -30px" href="/bar" class="B">Hello hello hello</a>
101+
</div>
102+
`);
103+
104+
assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
105+
assert.equal(checkContext._data.minOffset, 24);
106+
assert.closeTo(checkContext._data.closestOffset, 15.5, 5);
107+
});
69108
});
70109

71110
it('ignores non-widget elements as neighbors', () => {

test/checks/mobile/target-size.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,36 @@ describe('target-size tests', () => {
176176
assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']);
177177
});
178178

179+
it('returns true for obscured target when an unobscured rect has sufficient size but smaller area', () => {
180+
const checkArgs = checkSetup(`
181+
<div style="position: relative;">
182+
<button id="target" style="width: 80px; height: 40px;">X</button>
183+
<button id="obscurer" style="width: 80px; height: 40px; position: absolute; left: 30px; top: 20px;">x</button>
184+
</div>
185+
`);
186+
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
187+
assert.deepEqual(checkContext._data, {
188+
minSize: 24,
189+
width: 30,
190+
height: 40
191+
});
192+
assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']);
193+
});
194+
195+
it('returns true for multiline inline target with sufficient space', () => {
196+
const checkArgs = checkSetup(`
197+
<div style="font-size: 18px; margin: 1em auto; width: 6em; line-height: 1.3">
198+
<a id="not-obscurer" href="/foo" class="A"> Hello hello</a>
199+
<a id="target" href="/bar" class="B">Hello hello hello</a>
200+
<a id="obscurer" href="/bar" class="C">Hello hello hello</a>
201+
</div>
202+
`);
203+
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
204+
assert.closeTo(checkContext._data.width, 40.5, 10);
205+
assert.closeTo(checkContext._data.height, 40.5, 5);
206+
assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']);
207+
});
208+
179209
it('returns undefined if there are too many focusable widgets', () => {
180210
let html = '';
181211
for (let i = 0; i < 100; i++) {

test/commons/dom/get-target-rects.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,14 @@ describe('get-target-rects', () => {
9696
const rects = getTargetRects(vNode);
9797
assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
9898
});
99+
100+
it('uses client rects if target is inline', () => {
101+
const vNode = queryFixture(`
102+
<div style="width: 20px">
103+
<a href="#" id="target">hello world</a>
104+
</div>
105+
`);
106+
const rects = getTargetRects(vNode);
107+
assert.deepEqual(rects, Array.from(vNode.actualNode.getClientRects()));
108+
});
99109
});

test/commons/math/split-rects.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ describe('splitRects', () => {
2828
}, 'splitRects: Too many rects');
2929
});
3030

31+
it('accepts an array of rects', () => {
32+
const rectA = new DOMRect(0, 0, 100, 50);
33+
const rectB = new DOMRect(0, 50, 50, 50);
34+
const rects = splitRects([rectA], [rectB]);
35+
assert.lengthOf(rects, 1);
36+
assert.deepEqual(rects[0], rectA);
37+
});
38+
3139
describe('with one overlapping rect', () => {
3240
it('returns one rect if overlaps covers two corners', () => {
3341
const rectA = new DOMRect(0, 0, 100, 50);
@@ -73,6 +81,14 @@ describe('splitRects', () => {
7381
const rects = splitRects(rectA, [rectB]);
7482
assert.lengthOf(rects, 0);
7583
});
84+
85+
it('accepts an array of rects', () => {
86+
const rectA = new DOMRect(0, 0, 100, 50);
87+
const rectB = new DOMRect(40, 0, 100, 50);
88+
const rects = splitRects([rectA], [rectB]);
89+
assert.lengthOf(rects, 1);
90+
assert.deepEqual(rects[0], new DOMRect(0, 0, 40, 50));
91+
});
7692
});
7793

7894
describe('with multiple overlaps', () => {

test/integration/rules/target-size/target-size.html

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,27 @@
145145
>
146146
</p>
147147

148+
<div style="font-size: 18px; margin: 1em auto; width: 6em; line-height: 1.3">
149+
<a id="pass18" href="/foo" class="A"> Hello hello hello</a>
150+
<a id="pass19" href="/bar" class="B">Hello hello hello</a>
151+
</div>
152+
153+
<div style="font-size: 18px; margin: 1em auto; width: 6em; line-height: 1.3">
154+
<a id="pass20" href="/foo" class="A"> Hello hello</a>
155+
<a id="pass21" href="/bar" class="B">Hello hello hello</a>
156+
<a id="pass22" href="/bar" class="C">Hello hello hello</a>
157+
</div>
158+
159+
<div style="position: relative">
160+
<button id="pass23" style="width: 80px; height: 40px">X</button>
161+
<button
162+
id="pass24"
163+
style="width: 80px; height: 40px; position: absolute; left: 30px; top: 20px"
164+
>
165+
x
166+
</button>
167+
</div>
168+
148169
<p>
149170
<!-- These links touch, images overflow on the left and right -->
150171
<a href="#" id="incomplete5"

test/integration/rules/target-size/target-size.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@
2929
["#pass15"],
3030
["#pass16"],
3131
["#pass17"],
32+
["#pass18"],
33+
["#pass19"],
34+
["#pass20"],
35+
["#pass21"],
36+
["#pass22"],
37+
["#pass23"],
38+
["#pass24"],
3239
["#pass-adjacent"],
3340
["#pass-fixed"]
3441
],

0 commit comments

Comments
 (0)