chore(target-size): report non-tabbable widgets for review (#3699)

* chore(target-size): Avoid testing nested widgets * Include descendants in the tab order * Exclude nested widgets with tabindex="-1" * improve tests * chore(target-size): report non-tabbable widgets for review * Handle integration test * Update based on feedback
dequelabs · Oct 12, 2022 · 74dd278 · 74dd278
1 parent ea32fa7
commit 74dd278
Show file tree

Hide file tree

Showing 13 changed files with 298 additions and 68 deletions.
diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md
@@ -84,9 +84,9 @@
 
 These rules are disabled by default, until WCAG 2.2 is more widely adopted and required.
 
-| Rule ID                                                                                          | Description                                        | Impact  | Tags                                         | Issue Type | ACT Rules |
-| :----------------------------------------------------------------------------------------------- | :------------------------------------------------- | :------ | :------------------------------------------- | :--------- | :-------- |
-| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc258, cat.sensory-and-visual-cues | failure    |           |
+| Rule ID                                                                                          | Description                                        | Impact  | Tags                                         | Issue Type                 | ACT Rules |
+| :----------------------------------------------------------------------------------------------- | :------------------------------------------------- | :------ | :------------------------------------------- | :------------------------- | :-------- |
+| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc258, cat.sensory-and-visual-cues | failure, needs&nbsp;review |           |
 
 ## Best Practices Rules
 

diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js
@@ -1,4 +1,4 @@
-import { findNearbyElms, isFocusable } from '../../commons/dom';
+import { findNearbyElms, isFocusable, isInTabOrder } from '../../commons/dom';
 import { getRoleType } from '../../commons/aria';
 import { getOffset } from '../../commons/math';
 
@@ -17,15 +17,27 @@ export default function targetOffsetEvaluate(node, options, vNode) {
       continue;
     }
     closestOffset = Math.min(closestOffset, offset);
-    closeNeighbors.push(vNeighbor.actualNode);
+    closeNeighbors.push(vNeighbor);
   }
 
-  this.data({ closestOffset, minOffset });
-  if (closeNeighbors.length > 0) {
-    this.relatedNodes(closeNeighbors);
-    return false;
+  if (closeNeighbors.length === 0) {
+    this.data({ closestOffset, minOffset });
+    return true;
+  }
+
+  this.relatedNodes(closeNeighbors.map(({ actualNode }) => actualNode));
+
+  if (!closeNeighbors.some(isInTabOrder)) {
+    this.data({
+      messageKey: 'nonTabbableNeighbor',
+      closestOffset,
+      minOffset
+    });
+    return undefined;
   }
-  return true;
+
+  this.data({ closestOffset, minOffset });
+  return isInTabOrder(vNode) ? false : undefined;
 }
 
 function roundToSingleDecimal(num) {

diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json
@@ -8,7 +8,11 @@
     "impact": "serious",
     "messages": {
       "pass": "Target has sufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
-      "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)"
+      "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
+      "incomplete": {
+        "default": "Element with negative tabindex has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px). Is this a target?",
+        "nonTabbableNeighbor": "Target has insufficient offset from a neighbor with negative tabindex (${data.closestOffset}px should be at least ${data.minOffset}px). Is the neighbor a target?"
+      }
     }
   }
 }
diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js
@@ -11,6 +11,8 @@ const roundingMargin = 0.05;
 export default function targetSize(node, options, vNode) {
   const minSize = options?.minSize || 24;
   const nodeRect = vNode.boundingClientRect;
+  const negativeOutcome = isInTabOrder(vNode) ? false : undefined;
+
   const hasMinimumSize = ({ width, height }) => {
     return (
       width + roundingMargin >= minSize && height + roundingMargin >= minSize
@@ -35,7 +37,7 @@ export default function targetSize(node, options, vNode) {
 
   if (!hasMinimumSize(nodeRect)) {
     this.data({ minSize, ...toDecimalSize(nodeRect) });
-    return false;
+    return negativeOutcome;
   }
 
   const obscuredWidgets = obscuringElms.filter(
@@ -70,18 +72,28 @@ export default function targetSize(node, options, vNode) {
     return areaA > areaB ? rectA : rectB;
   });
 
-  if (!hasMinimumSize(largestInnerRect)) {
-    // Element is (partially?) obscured, with insufficient space
+  if (hasMinimumSize(largestInnerRect)) {
+    this.data({ minSize, ...toDecimalSize(largestInnerRect) });
+    return true;
+  }
+
+  if (!obscuredWidgets.every(isInTabOrder)) {
+    // Some obscuring neighbor is focusable but not in the tab order
     this.data({
-      messageKey: 'partiallyObscured',
+      messageKey: 'partiallyObscuredNonTabbable',
       minSize,
       ...toDecimalSize(largestInnerRect)
     });
-    return false;
+    return undefined;
   }
 
-  this.data({ minSize, ...toDecimalSize(largestInnerRect) });
-  return true;
+  // Element is (partially?) obscured, with insufficient space
+  this.data({
+    messageKey: 'partiallyObscured',
+    minSize,
+    ...toDecimalSize(largestInnerRect)
+  });
+  return negativeOutcome;
 }
 
 function isEnclosedRect(vNodeA, vNodeB) {

diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json
@@ -12,8 +12,13 @@
         "obscured": "Control is ignored because it is fully obscured and thus not clickable"
       },
       "fail": {
-        "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",
-        "partiallyObscured": "Element has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)"
+        "default": "Target has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",
+        "partiallyObscured": "Target has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)"
+      },
+      "incomplete": {
+        "default": "Element with negative tabindex has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
+        "partiallyObscured": "Element with negative tabindex has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
+        "partiallyObscuredNonTabbable": "Target has insufficient size because it is partially obscured by a neighbor with negative tabindex (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is the neighbor a target?"
       }
     }
   }

diff --git a/lib/commons/dom/is-skip-link.js b/lib/commons/dom/is-skip-link.js
@@ -21,10 +21,7 @@ export default function isSkipLink(element) {
     return false;
   }
 
-  const firstPageLink = cache.get(
-    'firstPageLink',
-    generateFirstPageLink
-  );
+  const firstPageLink = cache.get('firstPageLink', generateFirstPageLink);
 
   if (!firstPageLink) {
     return true;

diff --git a/lib/commons/standards/get-aria-roles-supporting-name-from-content.js b/lib/commons/standards/get-aria-roles-supporting-name-from-content.js
@@ -6,9 +6,8 @@ import standards from '../../standards';
  * @return {String[]} List of all roles with name from content
  */
 function getAriaRolesSupportingNameFromContent() {
-  return cache.get(
-    'ariaRolesNameFromContent',
-    () => Object.keys(standards.ariaRoles).filter(roleName => {
+  return cache.get('ariaRolesNameFromContent', () =>
+    Object.keys(standards.ariaRoles).filter(roleName => {
       return standards.ariaRoles[roleName].nameFromContent;
     })
   );

diff --git a/lib/commons/text/is-icon-ligature.js b/lib/commons/text/is-icon-ligature.js
@@ -84,9 +84,8 @@ function isIconLigature(
     return false;
   }
 
-  const canvasContext = cache.get(
-    'canvasContext',
-    () => document.createElement('canvas').getContext('2d')
+  const canvasContext = cache.get('canvasContext', () =>
+    document.createElement('canvas').getContext('2d')
   );
   const canvas = canvasContext.canvas;
 

diff --git a/locales/_template.json b/locales/_template.json
@@ -825,16 +825,25 @@
     },
     "target-offset": {
       "pass": "Target has sufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
-      "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)"
+      "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)",
+      "incomplete": {
+        "default": "Element with negative tabindex has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px). Is this a target?",
+        "nonTabbableNeighbor": "Target has insufficient offset from a neighbor with negative tabindex (${data.closestOffset}px should be at least ${data.minOffset}px). Is the neighbor a target?"
+      }
     },
     "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"
       },
       "fail": {
-        "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",
-        "partiallyObscured": "Element has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)"
+        "default": "Target has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)",
+        "partiallyObscured": "Target has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)"
+      },
+      "incomplete": {
+        "default": "Element with negative tabindex has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
+        "partiallyObscured": "Element with negative tabindex has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
+        "partiallyObscuredNonTabbable": "Target has insufficient size because it is partially obscured by a neighbor with negative tabindex (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is the neighbor a target?"
       }
     },
     "header-present": {

diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js
@@ -36,19 +36,38 @@ describe('target-offset tests', function () {
     assert.closeTo(checkContext._data.closestOffset, 24, 0.2);
   });
 
-  it('returns false when the offset is 23px', function () {
-    var checkArgs = checkSetup(
-      '<a href="#" id="target" style="' +
-        'display: inline-block; width:16px; height:16px; margin-right: 7px' +
-        '">x</a>' +
-        '<a href="#" style="' +
-        'display: inline-block; width:16px; height:16px;' +
-        '">x</a>'
-    );
+  describe('when the offset is insufficient', () => {
+    it('returns false for targets in the tab order', function () {
+      var checkArgs = checkSetup(
+        '<a href="#" id="target" style="' +
+          'display: inline-block; width:16px; height:16px; margin-right: 7px' +
+          '">x</a>' +
+          '<a href="#" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>'
+      );
 
-    assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
-    assert.equal(checkContext._data.minOffset, 24);
-    assert.closeTo(checkContext._data.closestOffset, 23, 0.2);
+      assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
+      assert.isUndefined(checkContext._data.messageKey);
+      assert.equal(checkContext._data.minOffset, 24);
+      assert.closeTo(checkContext._data.closestOffset, 23, 0.2);
+    });
+
+    it('returns undefined for targets not in the tab order', function () {
+      var checkArgs = checkSetup(
+        '<a href="#" id="target" tabindex="-1" style="' +
+          'display: inline-block; width:16px; height:16px; margin-right: 7px' +
+          '">x</a>' +
+          '<a href="#" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>'
+      );
+
+      assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
+      assert.isUndefined(checkContext._data.messageKey);
+      assert.equal(checkContext._data.minOffset, 24);
+      assert.closeTo(checkContext._data.closestOffset, 23, 0.2);
+    });
   });
 
   it('ignores non-widget elements as neighbors', function () {
@@ -66,7 +85,7 @@ describe('target-offset tests', function () {
     assert.closeTo(checkContext._data.closestOffset, 24, 0.2);
   });
 
-  it('ignores non-tabbable widget elements as neighbors', function () {
+  it('ignores non-focusable widget elements as neighbors', function () {
     var checkArgs = checkSetup(
       '<a href="#" id="target" style="' +
         'display: inline-block; width:16px; height:16px; margin-right: 7px' +
@@ -102,4 +121,52 @@ describe('target-offset tests', function () {
     });
     assert.deepEqual(relatedIds, ['#left', '#right']);
   });
+
+  describe('when neighbors are focusable but not tabbable', () => {
+    it('returns undefined if all neighbors are not tabbable', () => {
+      var checkArgs = checkSetup(
+        '<a href="#" id="left" tabindex="-1" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>' +
+          '<a href="#" id="target" style="' +
+          'display: inline-block; width:16px; height:16px; margin-right: 4px' +
+          '">x</a>' +
+          '<a href="#" id="right" tabindex="-1" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>'
+      );
+      assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
+      assert.equal(checkContext._data.messageKey, 'nonTabbableNeighbor');
+      assert.equal(checkContext._data.minOffset, 24);
+      assert.closeTo(checkContext._data.closestOffset, 16, 0.2);
+
+      var relatedIds = checkContext._relatedNodes.map(function (node) {
+        return '#' + node.id;
+      });
+      assert.deepEqual(relatedIds, ['#left', '#right']);
+    });
+
+    it('returns false if some but not all neighbors are not tabbable', () => {
+      var 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:16px; height:16px; margin-right: 4px' +
+          '">x</a>' +
+          '<a href="#" id="right" tabindex="-1" style="' +
+          'display: inline-block; width:16px; height:16px;' +
+          '">x</a>'
+      );
+      assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
+      assert.isUndefined(checkContext._data.messageKey);
+      assert.equal(checkContext._data.minOffset, 24);
+      assert.closeTo(checkContext._data.closestOffset, 16, 0.2);
+
+      var relatedIds = checkContext._relatedNodes.map(function (node) {
+        return '#' + node.id;
+      });
+      assert.deepEqual(relatedIds, ['#left', '#right']);
+    });
+  });
 });