From 0ddc00bb2d0eed457d9ce8ba5cd05606ef3bdc9e Mon Sep 17 00:00:00 2001
From: Dan Tripp <88439449+dan-tripp@users.noreply.github.com>
Date: Mon, 6 Dec 2021 11:57:15 -0500
Subject: [PATCH] fix(nested-interactive/aria-text): allow "tabindex=-1" on
 elements with no role (#3165)

* 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 428e01533eb561a5a2ee0a603014057337ba0177, reversing
changes made to 9f996bc6f54092541db77b9c3e62e2a5d9d9643e.

* Revert "test commit 1"

This reverts commit 9f996bc6f54092541db77b9c3e62e2a5d9d9643e.

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

Closes issue #2934

* work in progress

* work in progress

* test commit 1

* Revert "test commit 1"

This reverts commit 9f996bc6f54092541db77b9c3e62e2a5d9d9643e.

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

Closes issue #2934

* work in progress

* work in progress

* fix whitespace

* add new case to test test/checks/keyboard/no-focusable-content.js

* change "disabled" test case in test/checks/keyboard/no-focusable-content.js

* fix merge problem
---
 .../keyboard/no-focusable-content-evaluate.js |  4 +-
 test/checks/keyboard/no-focusable-content.js  | 56 +++++++++++++++++--
 .../rules/aria-text/aria-text.html            |  8 +++
 .../rules/aria-text/aria-text.json            |  4 +-
 .../nested-interactive.html                   | 24 ++++----
 .../nested-interactive.json                   | 10 ++--
 .../virtual-rules/nested-interactive.js       | 28 ++++++++--
 7 files changed, 106 insertions(+), 28 deletions(-)

diff --git a/lib/checks/keyboard/no-focusable-content-evaluate.js b/lib/checks/keyboard/no-focusable-content-evaluate.js
index 27824fc4f4..f72bad79d7 100644
--- a/lib/checks/keyboard/no-focusable-content-evaluate.js
+++ b/lib/checks/keyboard/no-focusable-content-evaluate.js
@@ -1,4 +1,5 @@
 import isFocusable from '../../commons/dom/is-focusable';
+import { getRole, getRoleType } from '../../commons/aria';
 
 export default function noFocusableContentEvaluate(node, options, virtualNode) {
   if (!virtualNode.children) {
@@ -40,7 +41,8 @@ function getFocusableDescendants(vNode) {
 
   const retVal = [];
   vNode.children.forEach(child => {
-    if (isFocusable(child)) {
+    const role = getRole(child);
+    if (getRoleType(role) === 'widget' && isFocusable(child)) {
       retVal.push(child);
     } else {
       retVal.push(...getFocusableDescendants(child));
diff --git a/test/checks/keyboard/no-focusable-content.js b/test/checks/keyboard/no-focusable-content.js
index 1389e0241a..d642bbd15e 100644
--- a/test/checks/keyboard/no-focusable-content.js
+++ b/test/checks/keyboard/no-focusable-content.js
@@ -28,17 +28,32 @@ describe('no-focusable-content tests', function() {
     assert.isTrue(noFocusableContent(null, null, vNode));
   });
 
-  it('should return false if element has focusable content', function() {
+  it('should return true if element has content which is focusable (tabindex=0) and does not have a widget role', function() {
     var params = checkSetup(
       '<button id="target"><span tabindex="0">Hello</span></button>'
     );
 
-    assert.isFalse(noFocusableContent.apply(checkContext, params));
-    assert.deepEqual(checkContext._data, null);
+    assert.isTrue(noFocusableContent.apply(checkContext, params));
+  });
+
+  it('should return true if element has content which has negative tabindex and non-widget role', function() {
+    var vNode = queryFixture(
+      '<button id="target"><span tabindex="-1">Hello</span></button>'
+    );
+    assert.isTrue(noFocusableContent(null, null, vNode));
+  });
+
+  it('should return false if element has content which has negative tabindex and an explicit widget role', function() {
+    var params = checkSetup(
+      '<button id="target"><span role="link" tabindex="-1">Hello</span></button>'
+    );
+    axe.utils.getFlattenedTree(document.documentElement);
+    assert.isFalse(check.evaluate.apply(checkContext, params));
+    assert.deepEqual(checkContext._data, { messageKey: 'notHidden' });
     assert.deepEqual(checkContext._relatedNodes, [params[2].children[0]]);
   });
 
-  it('should return false if element has natively focusable content', function() {
+  it('should return false if element has content which is natively focusable and has a widget role', function() {
     var params = checkSetup(
       '<button id="target"><a href="foo.html">Hello</a></button>'
     );
@@ -50,7 +65,7 @@ describe('no-focusable-content tests', function() {
 
   it('should add each focusable child as related nodes', function() {
     var params = checkSetup(
-      '<button id="target"><span tabindex="0">Hello</span><a href="foo.html">Hello</a></button>'
+      '<button id="target"><input type="checkbox"><a href="foo.html">Hello</a></button>'
     );
 
     assert.isFalse(noFocusableContent.apply(checkContext, params));
@@ -61,7 +76,7 @@ describe('no-focusable-content tests', function() {
     ]);
   });
 
-  it('should return false if element has natively focusable content with negative tabindex', function() {
+  it('should return false if element has natively focusable widget role content with negative tabindex', function() {
     var params = checkSetup(
       '<button id="target"><a href="foo.html" tabindex="-1">Hello</a></button>'
     );
@@ -70,4 +85,33 @@ describe('no-focusable-content tests', function() {
     assert.deepEqual(checkContext._data, { messageKey: 'notHidden' });
     assert.deepEqual(checkContext._relatedNodes, [params[2].children[0]]);
   });
+
+  it('should return true if element has content which is natively focusable and has a widget role but is disabled', function() {
+    var vNode = queryFixture(
+      '<button id="target"><input value="hello" disabled></button>'
+    );
+    assert.isTrue(noFocusableContent(null, null, vNode));
+  });
+
+  it('should return true on span with negative tabindex (focusable, does not have a widget role)', function() {
+    var vNode = queryFixture('<span id="target" role="text"> some text '
+        +'<span tabIndex="-1">JavaScript is able to focus this</span> '
+        +'</span>');
+    assert.isTrue(noFocusableContent(null, null, vNode));
+  });
+
+  it('should return true on aria-hidden span with negative tabindex (focusable, does not have a widget role)', function() {
+    var vNode = queryFixture('<span id="target" role="text"> some text '
+        +'<span tabIndex="-1" aria-hidden="true">JavaScript is able to focus this</span> '
+        +'</span>');
+    assert.isTrue(noFocusableContent(null, null, vNode));
+  });
+
+  it('should return true on nested span with tabindex=0 (focusable, does not have a widget role)', function() {
+    var vNode = queryFixture('<span id="target" role="text"> some text '
+        +'<span tabIndex="0">anyone is able to focus this</span> '
+        +'</span>');
+    assert.isTrue(noFocusableContent(null, null, vNode));
+  });
+
 });
diff --git a/test/integration/rules/aria-text/aria-text.html b/test/integration/rules/aria-text/aria-text.html
index 28252c0bce..54809b6bd5 100644
--- a/test/integration/rules/aria-text/aria-text.html
+++ b/test/integration/rules/aria-text/aria-text.html
@@ -20,3 +20,11 @@ <h1>
   explicit role.
 </div>
 <p role="text" id="fail4"><button>Hello</button></p>
+<p role="text" id="fail5"><button tabindex="-1">Hello</button></p>
+<p role="text" id="pass4"><span tabindex="-1">Hello</span></p>
+<div role="text" id="pass5">
+  <a tabindex="-1">passes because no href makes this not have the implicit role of 'link'</a>
+</div>
+<div role="text" id="pass6">
+  <a>passes because no href makes this not have the implicit role of 'link'</a>
+</div>
diff --git a/test/integration/rules/aria-text/aria-text.json b/test/integration/rules/aria-text/aria-text.json
index f6c3f1d2d9..9443502a16 100644
--- a/test/integration/rules/aria-text/aria-text.json
+++ b/test/integration/rules/aria-text/aria-text.json
@@ -1,6 +1,6 @@
 {
   "description": "aria-text tests",
   "rule": "aria-text",
-  "violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"]],
-  "passes": [["#pass1"], ["#pass2"], ["#pass3"]]
+  "violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"], ["#fail5"]],
+  "passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"], ["#pass5"], ["#pass6"]]
 }
diff --git a/test/integration/rules/nested-interactive/nested-interactive.html b/test/integration/rules/nested-interactive/nested-interactive.html
index fb42dd24dc..1af908b261 100644
--- a/test/integration/rules/nested-interactive/nested-interactive.html
+++ b/test/integration/rules/nested-interactive/nested-interactive.html
@@ -1,16 +1,18 @@
 <button id="pass1">pass</button>
-<div role="button" id="pass2">pass</div>
-<div role="tab" id="pass3">pass</div>
-<div role="checkbox" id="pass4">pass</div>
-<div role="radio" id="pass5"><span>pass</span></div>
+<button id="pass2"><span tabindex="-1">pass</span></button>
+<div role="button" id="pass3">pass</div>
+<div role="tab" id="pass4">pass</div>
+<div role="checkbox" id="pass5">pass</div>
+<div role="radio" id="pass6"><span>pass</span></div>
+<div role="radio" id="pass7"><span tabindex="-1">pass</span></div>
 
-<button id="fail1"><span tabindex="0">fail</span></button>
-<div role="button" id="fail2"><input /></div>
-<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>
+<button id="pass8"><span tabindex="0">pass</span></button>
+<div role="button" id="fail1"><input /></div>
+<div role="tab" id="fail2"><button id="pass9">div fails, button passes</button></div>
+<div role="checkbox" id="fail3"><a href="foo.html">fail</a></div>
+<div role="radio" id="pass10"><span tabindex="0">pass</span></div>
+<div role="radio" id="fail4"><button id="pass11" tabindex="-1">not really hidden</button></div>
+<div role="radio" id="fail5"><button aria-hidden="true" tabindex="-1">not really hidden</button></div>
 
 <a id="ignored1" href="foo.html">ignored</a>
 <span id="ignored2">ignored</span>
diff --git a/test/integration/rules/nested-interactive/nested-interactive.json b/test/integration/rules/nested-interactive/nested-interactive.json
index 434b22bb90..4db1a12cdb 100644
--- a/test/integration/rules/nested-interactive/nested-interactive.json
+++ b/test/integration/rules/nested-interactive/nested-interactive.json
@@ -6,9 +6,7 @@
     ["#fail2"], 
     ["#fail3"], 
     ["#fail4"], 
-    ["#fail5"],
-    ["#fail6"],
-    ["#fail7"]
+    ["#fail5"]
   ],
   "passes": [
     ["#pass1"],
@@ -17,6 +15,10 @@
     ["#pass4"],
     ["#pass5"],
     ["#pass6"],
-    ["#pass7"]
+    ["#pass7"],
+    ["#pass8"],
+    ["#pass9"],
+    ["#pass10"],
+    ["#pass11"]
   ]
 }
diff --git a/test/integration/virtual-rules/nested-interactive.js b/test/integration/virtual-rules/nested-interactive.js
index ba963b99b5..acdb54f389 100644
--- a/test/integration/virtual-rules/nested-interactive.js
+++ b/test/integration/virtual-rules/nested-interactive.js
@@ -38,6 +38,26 @@ describe('nested-interactive virtual-rule', function() {
     assert.lengthOf(results.incomplete, 0);
   });
 
+  it('should pass for element with non-widget content which has negative tabindex', function() {
+    var node = new axe.SerialVirtualNode({
+      nodeName: 'button'
+    });
+    var child = new axe.SerialVirtualNode({
+      nodeName: 'span',
+      attributes: {
+        tabindex: -1
+      }
+    });
+    child.children = [];
+    node.children = [child];
+
+    var results = axe.runVirtualRule('nested-interactive', node);
+
+    assert.lengthOf(results.passes, 1);
+    assert.lengthOf(results.violations, 0);
+    assert.lengthOf(results.incomplete, 0);
+  });
+
   it('should pass for empty element without', function() {
     var node = new axe.SerialVirtualNode({
       nodeName: 'div',
@@ -54,7 +74,7 @@ describe('nested-interactive virtual-rule', function() {
     assert.lengthOf(results.incomplete, 0);
   });
 
-  it('should fail for element with focusable content', function() {
+  it('should pass for element with non-widget content', function() {
     var node = new axe.SerialVirtualNode({
       nodeName: 'button'
     });
@@ -69,12 +89,12 @@ describe('nested-interactive virtual-rule', function() {
 
     var results = axe.runVirtualRule('nested-interactive', node);
 
-    assert.lengthOf(results.passes, 0);
-    assert.lengthOf(results.violations, 1);
+    assert.lengthOf(results.passes, 1);
+    assert.lengthOf(results.violations, 0);
     assert.lengthOf(results.incomplete, 0);
   });
 
-  it('should fail for element with natively focusable content', function() {
+  it('should fail for element with native widget content', function() {
     var node = new axe.SerialVirtualNode({
       nodeName: 'div',
       attributes: {