diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js index 8e45fc8164..949906576d 100644 --- a/lib/checks/mobile/target-offset-evaluate.js +++ b/lib/checks/mobile/target-offset-evaluate.js @@ -20,8 +20,27 @@ export default function targetOffsetEvaluate(node, options, vNode) { continue; } // the offset code works off radius but we want our messaging to reflect diameter - const offset = - roundToSingleDecimal(getOffset(vNode, vNeighbor, minOffset / 2)) * 2; + let offset = null; + try { + offset = getOffset(vNode, vNeighbor, minOffset / 2); + } catch (err) { + if (err.message.startsWith('splitRects')) { + this.data({ + messageKey: 'tooManyRects', + closestOffset: 0, + minOffset + }); + return undefined; + } + + throw err; + } + + if (offset === null) { + continue; + } + + offset = roundToSingleDecimal(offset) * 2; if (offset + roundingMargin >= minOffset) { continue; } diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json index c06ee11d7a..498ea64dc8 100644 --- a/lib/checks/mobile/target-offset.json +++ b/lib/checks/mobile/target-offset.json @@ -14,7 +14,8 @@ "fail": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px.", "incomplete": { "default": "Element with negative tabindex has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px. Is this a target?", - "nonTabbableNeighbor": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px. Is the neighbor a target?" + "nonTabbableNeighbor": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px. Is the neighbor a target?", + "tooManyRects": "Could not get the target size because there are too many overlapping elements" } } } diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js index 6ee45bc9c5..85561055b2 100644 --- a/lib/checks/mobile/target-size-evaluate.js +++ b/lib/checks/mobile/target-size-evaluate.js @@ -131,8 +131,10 @@ function getLargestUnobscuredArea(vNode, obscuredNodes) { const obscuringRects = obscuredNodes.map( ({ boundingClientRect: rect }) => rect ); - const unobscuredRects = splitRects(nodeRect, obscuringRects); - if (unobscuredRects.length === 0) { + let unobscuredRects; + try { + unobscuredRects = splitRects(nodeRect, obscuringRects); + } catch (err) { return null; } diff --git a/lib/commons/math/get-offset.js b/lib/commons/math/get-offset.js index 664871fda0..957d4e43c4 100644 --- a/lib/commons/math/get-offset.js +++ b/lib/commons/math/get-offset.js @@ -18,7 +18,7 @@ export default function getOffset(vTarget, vNeighbor, minRadiusNeighbour = 12) { const neighborRects = getTargetRects(vNeighbor); if (!targetRects.length || !neighborRects.length) { - return 0; + return null; } const targetBoundingBox = targetRects.reduce(getBoundingRect); diff --git a/lib/commons/math/split-rects.js b/lib/commons/math/split-rects.js index ca4ef232a6..ea6ba53971 100644 --- a/lib/commons/math/split-rects.js +++ b/lib/commons/math/split-rects.js @@ -18,7 +18,7 @@ export default function splitRects(outerRect, overlapRects) { // a performance bottleneck // @see https://github.com/dequelabs/axe-core/issues/4359 if (uniqueRects.length > 4000) { - return []; + throw new Error('splitRects: Too many rects'); } } return uniqueRects; diff --git a/locales/_template.json b/locales/_template.json index a911c60144..6ea35122f6 100644 --- a/locales/_template.json +++ b/locales/_template.json @@ -872,7 +872,8 @@ "fail": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px.", "incomplete": { "default": "Element with negative tabindex has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px. Is this a target?", - "nonTabbableNeighbor": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px. Is the neighbor a target?" + "nonTabbableNeighbor": "Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px instead of at least ${data.minOffset}px. Is the neighbor a target?", + "tooManyRects": "Could not get the target size because there are too many overlapping elements" } }, "target-size": { diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js index 367968f448..747d9a4faa 100644 --- a/test/checks/mobile/target-offset.js +++ b/test/checks/mobile/target-offset.js @@ -98,6 +98,21 @@ describe('target-offset tests', () => { assert.closeTo(checkContext._data.closestOffset, 24, 0.2); }); + it('ignores obscured widget elements as neighbors', () => { + const checkArgs = checkSetup(` +
+ Go to top +
+
+ Cookies: Accept all cookies +
+ `); + + assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 24, 0.2); + }); + it('sets all elements that are too close as related nodes', () => { const checkArgs = checkSetup( '${html} `); - assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); + assert.isUndefined(checkEvaluate.apply(checkContext, checkArgs)); assert.deepEqual(checkContext._data, { + messageKey: 'tooManyRects', closestOffset: 0, minOffset: 24 }); diff --git a/test/commons/math/get-offset.js b/test/commons/math/get-offset.js index 073c8d0649..c4a5e89649 100644 --- a/test/commons/math/get-offset.js +++ b/test/commons/math/get-offset.js @@ -33,24 +33,46 @@ describe('getOffset', () => { assert.closeTo(getOffset(nodeA, nodeB), 63.6, round); }); - it('returns 0 if nodeA is overlapped by nodeB', () => { + it('returns null if nodeA is overlapped by nodeB', () => { const fixture = fixtureSetup(` `); const nodeA = fixture.children[1]; const nodeB = fixture.children[3]; - assert.equal(getOffset(nodeA, nodeB), 0); + assert.isNull(getOffset(nodeA, nodeB)); }); - it('returns 0 if nodeB is overlapped by nodeA', () => { + it('returns null if nodeB is overlapped by nodeA', () => { const fixture = fixtureSetup(` `); const nodeA = fixture.children[3]; const nodeB = fixture.children[1]; - assert.equal(getOffset(nodeA, nodeB), 0); + assert.isNull(getOffset(nodeA, nodeB)); + }); + + it('returns null if nodeA is overlapped by another node', () => { + const fixture = fixtureSetup(` + + +
 
+ `); + const nodeB = fixture.children[1]; + const nodeA = fixture.children[3]; + assert.isNull(getOffset(nodeA, nodeB)); + }); + + it('returns null if nodeB is overlapped by another node', () => { + const fixture = fixtureSetup(` + + +
 
+ `); + const nodeA = fixture.children[3]; + const nodeB = fixture.children[1]; + assert.isNull(getOffset(nodeA, nodeB)); }); it('subtracts minNeighbourRadius from center-to-center calculations', () => { diff --git a/test/commons/math/split-rects.js b/test/commons/math/split-rects.js index 9463a7fbde..968b8c7b79 100644 --- a/test/commons/math/split-rects.js +++ b/test/commons/math/split-rects.js @@ -16,13 +16,16 @@ describe('splitRects', () => { assert.deepEqual(rects[0], rectA); }); - it('returns empty array if there are too many overlapping rects', () => { + it('throws 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); + + assert.throws(() => { + splitRects(rectA, rects); + }, 'splitRects: Too many rects'); }); describe('with one overlapping rect', () => { diff --git a/test/integration/full/target-size/too-many-rects.js b/test/integration/full/target-size/too-many-rects.js index d1364b0d32..d814850247 100644 --- a/test/integration/full/target-size/too-many-rects.js +++ b/test/integration/full/target-size/too-many-rects.js @@ -9,6 +9,7 @@ describe('target-size too many rects test', () => { elementRef: true }; const context = { + include: '#incomplete', // ignore the incomplete table results exclude: 'table tr' };