fix(target-size): always pass 10x targets (avoid perf bottleneck) (#4376

) Let targets that are more than 240x240 auto-pass the target-size rule to avoid performance issues. QA note: Test that the page provided in #4360 results in a pass for the tabpanel in the comment section (https://almanac.httparchive.org/en/2021/ecommerce)
dequelabs · Mar 18, 2024 · be327c4 · be327c4
1 parent 1dbea83
commit be327c4
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 38 deletions.
diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js
@@ -1,11 +1,18 @@
 import { findNearbyElms, isFocusable, isInTabOrder } from '../../commons/dom';
 import { getRoleType } from '../../commons/aria';
-import { getOffset } from '../../commons/math';
+import { getOffset, rectHasMinimumSize } from '../../commons/math';
 
 const roundingMargin = 0.05;
 
 export default function targetOffsetEvaluate(node, options, vNode) {
   const minOffset = options?.minOffset || 24;
+  // Bail early to avoid hitting very expensive calculations.
+  // Targets are so large they are unlikely to fail.
+  if (rectHasMinimumSize(minOffset * 10, vNode.boundingClientRect)) {
+    this.data({ messageKey: 'large', minOffset });
+    return true;
+  }
+
   const closeNeighbors = [];
   let closestOffset = minOffset;
   for (const vNeighbor of findNearbyElms(vNode, minOffset)) {

diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json
@@ -7,7 +7,10 @@
   "metadata": {
     "impact": "serious",
     "messages": {
-      "pass": "Target has sufficient space from its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px which is at least ${data.minOffset}px.",
+      "pass": {
+        "default": "Target has sufficient space from its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px which is at least ${data.minOffset}px.",
+        "large": "Target far exceeds the minimum size of ${data.minOffset}px."
+      },
       "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?",

diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js
@@ -13,6 +13,13 @@ import {
 export default function targetSizeEvaluate(node, options, vNode) {
   const minSize = options?.minSize || 24;
   const nodeRect = vNode.boundingClientRect;
+  // Bail early to avoid hitting very expensive calculations.
+  // Targets are so large they are unlikely to fail.
+  if (rectHasMinimumSize(minSize * 10, nodeRect)) {
+    this.data({ messageKey: 'large', minSize });
+    return true;
+  }
+
   const hasMinimumSize = rectHasMinimumSize.bind(null, minSize);
   const nearbyElms = findNearbyElms(vNode);
   const overflowingContent = filterOverflowingContent(vNode, nearbyElms);

diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json
@@ -9,7 +9,8 @@
     "messages": {
       "pass": {
         "default": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",
-        "obscured": "Control is ignored because it is fully obscured and thus not clickable"
+        "obscured": "Control is ignored because it is fully obscured and thus not clickable",
+        "large": "Target far exceeds the minimum size of ${data.minSize}px."
       },
       "fail": {
         "default": "Target has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",

diff --git a/locales/_template.json b/locales/_template.json
@@ -865,7 +865,10 @@
       "fail": "${data} on <meta> tag disables zooming on mobile devices"
     },
     "target-offset": {
-      "pass": "Target has sufficient space from its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px which is at least ${data.minOffset}px.",
+      "pass": {
+        "default": "Target has sufficient space from its closest neighbors. Safe clickable space has a diameter of ${data.closestOffset}px which is at least ${data.minOffset}px.",
+        "large": "Target far exceeds the minimum size of ${data.minOffset}px."
+      },
       "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?",
@@ -875,7 +878,8 @@
     "target-size": {
       "pass": {
         "default": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",
-        "obscured": "Control is ignored because it is fully obscured and thus not clickable"
+        "obscured": "Control is ignored because it is fully obscured and thus not clickable",
+        "large": "Target far exceeds the minimum size of ${data.minSize}px."
       },
       "fail": {
         "default": "Target has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",

diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js
@@ -125,21 +125,15 @@ describe('target-offset tests', () => {
     for (let i = 0; i < 100; i++) {
       html += `
         <tr>
-          <td><a href="#">Hello</a></td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td><button>view</button></td>
-          <td><button>download</button></td>
-          <td><button>expand</button></td>
+          <td><a href="#">A</a></td>
+          <td><button>B</button></td>
+          <td><button>C</button></td>
+          <td><button>D</button></td>
         </tr>
       `;
     }
     const checkArgs = checkSetup(`
-      <div id="target" role="tabpanel" tabindex="0">
+      <div id="target" role="tabpanel" tabindex="0" style="display:inline-block">
         <table id="tab-table">${html}</table>
       </div>
     `);
@@ -196,5 +190,22 @@ describe('target-offset tests', () => {
       });
       assert.deepEqual(relatedIds, ['#left', '#right']);
     });
+
+    it('returns true if the target is 10x the minOffset', () => {
+      const checkArgs = checkSetup(
+        '<a href="#" id="left" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>' +
+          '<a href="#" id="target" style="' +
+          'display: inline-block; width:240px; height:240px; margin-right: 4px' +
+          '">x</a>' +
+          '<a href="#" id="right" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>'
+      );
+      assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
+      assert.equal(checkContext._data.minOffset, 24);
+      assert.equal(checkContext._data.messageKey, 'large');
+    });
   });
 });
diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js
@@ -58,6 +58,16 @@ describe('target-size tests', function () {
     });
   });
 
+  it('returns true for very large targets', function () {
+    var checkArgs = checkSetup(
+      '<button id="target" style="' +
+        'display: inline-block; width:240px; height:300px;' +
+        '">x</button>'
+    );
+    assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
+    assert.deepEqual(checkContext._data, { messageKey: 'large', minSize: 24 });
+  });
+
   describe('when fully obscured', function () {
     it('returns true, regardless of size', function () {
       var checkArgs = checkSetup(
@@ -172,21 +182,15 @@ describe('target-size tests', function () {
         for (let i = 0; i < 100; i++) {
           html += `
             <tr>
-              <td><a href="#">Hello</a></td>
-              <td>Hello</td>
-              <td>Hello</td>
-              <td>Hello</td>
-              <td>Hello</td>
-              <td>Hello</td>
-              <td>Hello</td>
-              <td><button>view</button></td>
-              <td><button>download</button></td>
-              <td><button>expand</button></td>
+              <td><a href="#">A</a></td>
+              <td><button>B</button></td>
+              <td><button>C</button></td>
+              <td><button>D</button></td>
             </tr>
           `;
         }
         const checkArgs = checkSetup(`
-          <div id="target" role="tabpanel" tabindex="0">
+          <div id="target" role="tabpanel" tabindex="0" style="display:inline-block">
             <table id="tab-table">${html}</table>
           </div>
         `);

diff --git a/test/integration/full/target-size/too-many-rects.html b/test/integration/full/target-size/too-many-rects.html
@@ -22,7 +22,12 @@
     <main id="mocha"></main>
 
     <section id="incomplete">
-      <div id="incomplete-many-rects" role="tabpanel" tabindex="0">
+      <div
+        id="incomplete-many-rects"
+        role="tabpanel"
+        tabindex="0"
+        style="display: inline-block"
+      >
         <table id="tab-table"></table>
       </div>
     </section>
@@ -32,16 +37,10 @@
       for (let i = 0; i < 100; i++) {
         html += `
         <tr>
-          <td><a href="#">Hello</a></td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td>Hello</td>
-          <td><button>view</button></td>
-          <td><button>download</button></td>
-          <td><button>expand</button></td>
+          <td><a href="#">A</a></td>
+          <td><button>B</button></td>
+          <td><button>C</button></td>
+          <td><button>D</button></td>
         </tr>`;
       }
       document.querySelector('#tab-table').innerHTML = html;