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

@@ -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);
 }

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)
+  );
+}

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

@@ -1,47 +1,16 @@
-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);
 
 /**
  * Compute the target size of an element.
  * @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
-  );
-}

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';

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) {

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';

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
+  );
+}

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('<button id="target">x</button>');
+    const rects = getTargetRects(vNode);
+    assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+  });
+
+  it('returns subset rect when obscured', () => {
+    const vNode = queryFixture(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">x</button>
+      <div style="position: absolute; left: 30px; top: 0; width: 50px; height: 50px;"></div>
+    `);
+    const rects = getTargetRects(vNode);
+    assert.deepEqual(rects, [new DOMRect(10, 5, 20, 40)]);
+  });
+
+  it('ignores elements with "pointer-events: none"', () => {
+    const vNode = queryFixture(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">x</button>
+      <div style="position: absolute; left: 30px; top: 0; width: 50px; height: 50px; pointer-events: none"></div>
+    `);
+    const rects = getTargetRects(vNode);
+    assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+  });
+
+  it("ignores elements that don't overlap the target", () => {
+    const vNode = queryFixture(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">x</button>
+      <div style="position: absolute; left: 60px; top: 0; width: 50px; height: 50px;"></div>
+    `);
+    const rects = getTargetRects(vNode);
+    assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+  });
+
+  it('ignores non-tabbable descendants of the target', () => {
+    const vNode = queryFixture(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">
+        <div style="position: absolute; left: 5px; top: 5px; width: 50px; height: 50px;"></div>
+      </button>
+    `);
+    const rects = getTargetRects(vNode);
+    assert.deepEqual(rects, [vNode.actualNode.getBoundingClientRect()]);
+  });
+
+  it('returns each unobscured area', () => {
+    const vNode = queryFixture(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">x</button>
+      <div style="position: absolute; left: 30px; top: 0; width: 50px; height: 50px;"></div>
+      <div style="position: absolute; left: 0px; top: 20px; width: 50px; height: 10px"></div>
+    `);
+    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(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">x</button>
+      <div style="position: absolute; left: 0; top: 0; width: 50px; height: 50px;"></div>
+    `);
+    const rects = getTargetRects(vNode);
+    assert.lengthOf(rects, 0);
+  });
+
+  it('returns subset rect of the target with tabbable descendant', () => {
+    const vNode = queryFixture(`
+      <button id="target" style="width: 30px; height: 40px; position: absolute; left: 10px; top: 5px">
+        <div tabindex="0" style="position: absolute; left: 5px; top: 5px; width: 50px; height: 50px;"></div>
+      </button>
+    `);
+    const rects = getTargetRects(vNode);
+    console.log(JSON.stringify(rects));
+    assert.deepEqual(rects, [
+      new DOMRect(10, 5, 30, 7),
+      new DOMRect(10, 5, 7, 40)
+    ]);
+  });
+});

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

@@ -23,7 +23,7 @@ describe('get-target-size', () => {
       <div style="position: absolute; left: 30px; top: 0; width: 50px; height: 50px; pointer-events: none"></div>
     `);
     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', () => {
       <div style="position: absolute; left: 60px; top: 0; width: 50px; height: 50px;"></div>
     `);
     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', () => {

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(`
+      <button style="width: 50px; height: 10px; margin: 0; padding: 0; position: absolute; top: 0; left: 0">&nbsp;</button>
+      <button style="width: 10px; height: 10px; margin: 0; padding: 0; position: absolute; top: 0; left: 20px;">&nbsp;</button>
+    `);
+    const nodeA = fixture.children[1];
+    const nodeB = fixture.children[3];
+    assert.equal(getOffset(nodeA, nodeB, 30), 0);
+  });
 });

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));
+  });
+});