fix(nested-interactive): add message for negative tabindex (#3194)

* refactor(checks/navigation): improve `internal-link-present-evaluate`

Make `internal-link-present-evaluate` work with virtualNode rather than actualNode.

Closes issue #2466

* test commit 1

* test commit 2

* test commit 3

* Revert "Merge branch 'dan-test-branch-1' into develop"

This reverts commit 428e015, reversing
changes made to 9f996bc.

* Revert "test commit 1"

This reverts commit 9f996bc.

* fix(rule): allow "tabindex=-1" for rules "aria-text" and "nested-interactive"

Closes issue #2934

* Revert "fix(rule): allow "tabindex=-1" for rules "aria-text" and "nested-interactive""

This reverts commit 30f0e01.

* refactor(check): split no-focusable-content rule into three.

That rule is now:
no-focusable-content-for-aria-text,
no-focusable-content-for-nested-interactive, and
no-focusable-content-for-frame

These three are all copy-and-pastes of each other, so far.

* fix(rule): add custom message for nested-interactive.

aria-hidden and negative tabindex will trigger messageKey.

* style(rule): fix lint errors

Errors were in parent commit.

* fix(no-focusable-content-for-frame): fix locale files

Was broken by recent rename of this check.

* refactor(check): undo recent check rename

Rename check "no-focusable-content-for-frame" back to "frame-focusable-content".

* refactor(check): undo recent split of checks

Recombine checks "no-focusable-content-for-aria-text" and "no-focusable-content-for-nested-interactive" back into "no-focusable-content".

* refactor(check): rename local variable

* fix(rule): make no-focusable-content not consult aria-hidden

no-focusable content now consults only tabindex (for the 'not a reliable way of hiding interactive elements' messageKey check)

* fix(checks/keyboard): make no-focusable-content use messageKey the right way

* refactor(check): misc. renaming and refactoring

* Updating description / messageKey text as per #3163 (comment)

* add unit test for frame-focusable-content check

* update the no-focusable-content unit tests to add a test to capture negative tabindex

* update the nested-interactive integration test to add a failure for elements with negative tabindex values

* fix(no-focusable-content-for-frame): fix locale files

Was broken by recent rename of this check.

* Update lib/rules/nested-interactive.json

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

* Update lib/checks/keyboard/no-focusable-content.json

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

* fixing botched merge

* update rule-descriptions.md

* fix locale files

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

lib/checks/keyboard/frame-focusable-content-evaluate.js

@@ -0,0 +1,35 @@
+import isFocusable from '../../commons/dom/is-focusable';
+
+function focusableDescendants(vNode) {
+  if (isFocusable(vNode)) {
+    return true;
+  }
+
+  if (!vNode.children) {
+    if (vNode.props.nodeType === 1) {
+      throw new Error('Cannot determine children');
+    }
+
+    return false;
+  }
+
+  return vNode.children.some(child => {
+    return focusableDescendants(child);
+  });
+}
+
+function frameFocusableContentEvaluate(node, options, virtualNode) {
+  if (!virtualNode.children) {
+    return undefined;
+  }
+
+  try {
+    return !virtualNode.children.some(child => {
+      return focusableDescendants(child);
+    });
+  } catch (e) {
+    return undefined;
+  }
+}
+
+export default frameFocusableContentEvaluate;

lib/checks/keyboard/frame-focusable-content.json

@@ -1,6 +1,6 @@
 {
   "id": "frame-focusable-content",
-  "evaluate": "no-focusable-content-evaluate",
+  "evaluate": "frame-focusable-content-evaluate",
   "metadata": {
     "impact": "serious",
     "messages": {

lib/checks/keyboard/no-focusable-content-evaluate.js

@@ -1,35 +1,47 @@
 import isFocusable from '../../commons/dom/is-focusable';
 
-function focusableDescendants(vNode) {
-  if (isFocusable(vNode)) {
-    return true;
-  }
-
+function getFocusableDescendants(vNode) {
   if (!vNode.children) {
     if (vNode.props.nodeType === 1) {
       throw new Error('Cannot determine children');
     }
 
-    return false;
+    return [];
   }
 
-  return vNode.children.some(child => {
-    return focusableDescendants(child);
+  const retVal = [];
+  vNode.children.forEach(child => {
+    if(isFocusable(child)) {
+      retVal.push(child);
+    } else {
+      retVal.push(...getFocusableDescendants(child));
+    }
   });
+  return retVal;
 }
 
-function noFocusbleContentEvaluate(node, options, virtualNode) {
+function usesUnreliableHidingStrategy(vNode) {
+  const tabIndex = parseInt(vNode.attr('tabindex'), 10);
+  return !isNaN(tabIndex) && tabIndex < 0;
+}
+
+function noFocusableContentEvaluate(node, options, virtualNode) {
   if (!virtualNode.children) {
     return undefined;
   }
-
   try {
-    return !virtualNode.children.some(child => {
-      return focusableDescendants(child);
-    });
+    const focusableDescendants = getFocusableDescendants(virtualNode);
+    if(focusableDescendants.length > 0) {
+      const notHiddenElements = focusableDescendants.filter(usesUnreliableHidingStrategy);
+      if(notHiddenElements.length > 0) {
+        this.data({ messageKey: 'notHidden' });
+        this.relatedNodes(notHiddenElements);
+      }
+    }
+    return focusableDescendants.length === 0;
   } catch (e) {
     return undefined;
   }
 }
 
-export default noFocusbleContentEvaluate;
+export default noFocusableContentEvaluate;

lib/checks/keyboard/no-focusable-content.json

@@ -5,7 +5,10 @@
     "impact": "serious",
     "messages": {
       "pass": "Element does not have focusable descendants",
-      "fail": "Element has focusable descendants",
+      "fail": {
+        "default": "Element has focusable descendants",
+        "notHidden": "Using a negative tabindex on an element inside an interactive control does not prevent a screen reader from focusing the element (even with 'aria-hidden=true')"
+      },
       "incomplete": "Could not determine if element has descendants"
     }
   }

lib/core/base/metadata-function-map.js

@@ -96,6 +96,7 @@ import focusableModalOpenEvaluate from '../../checks/keyboard/focusable-modal-op
 import focusableNoNameEvaluate from '../../checks/keyboard/focusable-no-name-evaluate';
 import focusableNotTabbableEvaluate from '../../checks/keyboard/focusable-not-tabbable-evaluate';
 import landmarkIsTopLevelEvaluate from '../../checks/keyboard/landmark-is-top-level-evaluate';
+import frameFocusableContentEvaluate from '../../checks/keyboard/frame-focusable-content-evaluate';
 import noFocusableContentEvaluate from '../../checks/keyboard/no-focusable-content-evaluate';
 import tabindexEvaluate from '../../checks/keyboard/tabindex-evaluate';
 
@@ -273,6 +274,7 @@ const metadataFunctionMap = {
   'focusable-no-name-evaluate': focusableNoNameEvaluate,
   'focusable-not-tabbable-evaluate': focusableNotTabbableEvaluate,
   'landmark-is-top-level-evaluate': landmarkIsTopLevelEvaluate,
+  'frame-focusable-content-evaluate': frameFocusableContentEvaluate,
   'no-focusable-content-evaluate': noFocusableContentEvaluate,
   'tabindex-evaluate': tabindexEvaluate,
 

lib/rules/nested-interactive.json

@@ -4,7 +4,7 @@
   "tags": ["cat.keyboard", "wcag2a", "wcag412"],
   "actIds": ["307n5z"],
   "metadata": {
-    "description": "Ensure controls are not nested as they are not announced by screen readers",
+    "description": "Ensures interactive controls are not nested as they are not always announced by screen readers or can cause focus problems for assistive technologies",
     "help": "Interactive controls must not be nested"
   },
   "all": [],

test/checks/keyboard/frame-focusable-content.js

@@ -0,0 +1,41 @@
+describe('frame-focusable-content tests', function() {
+  var fixture = document.querySelector('#fixture');
+  var queryFixture = axe.testUtils.queryFixture;
+  var frameFocusableContent = axe.testUtils.getCheckEvaluate(
+    'frame-focusable-content'
+  );
+
+  afterEach(function() {
+    fixture.innerHTML = '';
+  });
+
+  it('should return true if element has no focusable content', function() {
+    var vNode = queryFixture('<button id="target"><span>Hello</span></button>');
+    assert.isTrue(frameFocusableContent(null, null, vNode));
+  });
+
+  it('should return true if element is empty', function() {
+    var vNode = queryFixture('<button id="target"></button>');
+    assert.isTrue(frameFocusableContent(null, null, vNode));
+  });
+
+  it('should return true if element only has text content', function() {
+    var vNode = queryFixture('<button id="target">Hello</button>');
+    assert.isTrue(frameFocusableContent(null, null, vNode));
+  });
+
+  it('should return false if element has focusable content', function() {
+    var vNode = queryFixture(
+      '<button id="target"><span tabindex="0">Hello</span></button>'
+    );
+    assert.isFalse(frameFocusableContent(null, null, vNode));
+  });
+
+  it('should return false if element has natively focusable content', function() {
+    var vNode = queryFixture(
+      '<button id="target"><a href="foo.html">Hello</a></button>'
+    );
+    assert.isFalse(frameFocusableContent(null, null, vNode));
+  });
+
+});

test/checks/keyboard/no-focusable-content.js

@@ -4,9 +4,13 @@ describe('no-focusable-content tests', function() {
   var noFocusableContent = axe.testUtils.getCheckEvaluate(
     'no-focusable-content'
   );
+  var checkSetup = axe.testUtils.checkSetup;
+  var check = checks['no-focusable-content'];
+  var checkContext = new axe.testUtils.MockCheckContext();
 
   afterEach(function() {
     fixture.innerHTML = '';
+    checkContext.reset();
   });
 
   it('should return true if element has no focusable content', function() {
@@ -37,4 +41,14 @@ describe('no-focusable-content tests', function() {
     );
     assert.isFalse(noFocusableContent(null, null, vNode));
   });
+
+  it('should return false if element has natively focusable content with negative tabindex', function() {
+    var params = checkSetup(
+      '<button id="target"><a href="foo.html" tabindex="-1">Hello</a></button>'
+    );
+    axe.utils.getFlattenedTree(document.documentElement);
+    assert.isFalse(check.evaluate.apply(checkContext, params));
+    assert.deepEqual(checkContext._data, { messageKey: 'notHidden' });
+  });
+
 });

test/integration/rules/nested-interactive/nested-interactive.html

@@ -9,6 +9,8 @@
 <div role="tab" id="fail3"><button id="pass6">fail</button></div>
 <div role="checkbox" id="fail4"><a href="foo.html">fail</a></div>
 <div role="radio" id="fail5"><span tabindex="0">fail</span></div>
+<div role="radio" id="fail6"><button id="pass7" tabindex="-1">not really hidden</button></div>
+<div role="radio" id="fail7"><button aria-hidden="true" tabindex="-1">not really hidden</button></div>
 
 <a id="ignored1" href="foo.html">ignored</a>
 <span id="ignored2">ignored</span>

test/integration/rules/nested-interactive/nested-interactive.json

@@ -1,13 +1,22 @@
 {
   "description": "nested-interactive tests",
   "rule": "nested-interactive",
-  "violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"], ["#fail5"]],
+  "violations": [
+    ["#fail1"], 
+    ["#fail2"], 
+    ["#fail3"], 
+    ["#fail4"], 
+    ["#fail5"],
+    ["#fail6"],
+    ["#fail7"]
+  ],
   "passes": [
     ["#pass1"],
     ["#pass2"],
     ["#pass3"],
     ["#pass4"],
     ["#pass5"],
-    ["#pass6"]
+    ["#pass6"],
+    ["#pass7"]
   ]
 }