fix(frame-focusable-content): don't fail for elements with negative t…

…abindex (#3493) * fix(frame-focsuable-content): don't fail for elements with negative tabindex * revert playground' * remove test case * Update lib/checks/keyboard/frame-focusable-content-evaluate.js Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Wilco Fiers <[email protected]>
dequelabs · Jul 13, 2022 · 3b0b678 · 3b0b678
1 parent fc17062
commit 3b0b678
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 29 deletions.
diff --git a/lib/checks/keyboard/frame-focusable-content-evaluate.js b/lib/checks/keyboard/frame-focusable-content-evaluate.js
@@ -1,7 +1,26 @@
 import isFocusable from '../../commons/dom/is-focusable';
 
+export default function frameFocusableContentEvaluate(
+  node,
+  options,
+  virtualNode
+) {
+  if (!virtualNode.children) {
+    return undefined;
+  }
+
+  try {
+    return !virtualNode.children.some(child => {
+      return focusableDescendants(child);
+    });
+  } catch (e) {
+    return undefined;
+  }
+}
+
 function focusableDescendants(vNode) {
-  if (isFocusable(vNode)) {
+  const tabIndex = parseInt(vNode.attr('tabindex'), 10);
+  if ((isNaN(tabIndex) || tabIndex > -1) && isFocusable(vNode)) {
     return true;
   }
 
@@ -17,19 +36,3 @@ function focusableDescendants(vNode) {
     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;
diff --git a/lib/commons/dom/is-focusable.js b/lib/commons/dom/is-focusable.js
@@ -4,15 +4,14 @@ import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-n
 import { getNodeFromTree } from '../../core/utils';
 
 /**
- * Determines if an element is focusable
+ * Determines if an element is keyboard or programmatically focusable.
  * @method isFocusable
  * @memberof axe.commons.dom
  * @instance
  * @param {HTMLElement} el The HTMLElement
  * @return {Boolean} The element's focusability status
  */
-
-function isFocusable(el) {
+export default function isFocusable(el) {
   const vNode = el instanceof AbstractVirtualNode ? el : getNodeFromTree(el);
 
   if (vNode.props.nodeType !== 1) {
@@ -32,5 +31,3 @@ function isFocusable(el) {
 
   return false;
 }
-
-export default isFocusable;
diff --git a/test/checks/keyboard/frame-focusable-content.js b/test/checks/keyboard/frame-focusable-content.js
@@ -10,32 +10,45 @@ describe('frame-focusable-content tests', function() {
   });
 
   it('should return true if element has no focusable content', function() {
-    var vNode = queryFixture('<button id="target"><span>Hello</span></button>');
+    var vNode = queryFixture('<div id="target"><span>Hello</span></div>');
     assert.isTrue(frameFocusableContent(null, null, vNode));
   });
 
   it('should return true if element is empty', function() {
-    var vNode = queryFixture('<button id="target"></button>');
+    var vNode = queryFixture('<div id="target"></div>');
     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>');
+    var vNode = queryFixture('<div id="target">Hello</div>');
     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>'
+      '<div id="target"><span tabindex="0">Hello</span></div>'
     );
     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>'
+      '<div id="target"><a href="foo.html">Hello</a></div>'
+    );
+    assert.isFalse(frameFocusableContent(null, null, vNode));
+  });
+
+  it('should return true if element is natively focusable but has tabindex=-1', function() {
+    var vNode = queryFixture(
+      '<div id="target"><button tabindex="-1">Hello</button></div>'
+    );
+    assert.isTrue(frameFocusableContent(null, null, vNode));
+  });
+
+  it('should return false if element is natively focusable but has tabindex=0', function() {
+    var vNode = queryFixture(
+      '<div id="target"><button tabindex="0">Hello</button></div>'
     );
     assert.isFalse(frameFocusableContent(null, null, vNode));
   });
-
 });
diff --git a/test/integration/rules/frame-focusable-content/frame-focusable-content.html b/test/integration/rules/frame-focusable-content/frame-focusable-content.html
@@ -3,6 +3,11 @@
   tabindex="-1"
   id="pass1"
 ></iframe>
+<iframe
+  src="/integration/rules/frame-focusable-content/frames/focusable-negative-tabindex.html"
+  tabindex="-1"
+  id="pass2"
+></iframe>
 
 <iframe
   src="/integration/rules/frame-focusable-content/frames/focusable.html"

diff --git a/test/integration/rules/frame-focusable-content/frame-focusable-content.json b/test/integration/rules/frame-focusable-content/frame-focusable-content.json
@@ -5,5 +5,8 @@
     ["#fail1", "#focusable"],
     ["#fail2", "#focusable"]
   ],
-  "passes": [["#pass1", "#not-focusable"]]
+  "passes": [
+    ["#pass1", "#not-focusable"],
+    ["#pass2", "#focusable"]
+  ]
 }
diff --git a/test/integration/rules/frame-focusable-content/frames/focusable-negative-tabindex.html b/test/integration/rules/frame-focusable-content/frames/focusable-negative-tabindex.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html id="focusable">
+  <head>
+    <title>Hello</title>
+    <meta charset="utf8" />
+    <script src="/axe.js"></script>
+  </head>
+  <body>
+    <button tabindex="-1">Click</button>
+  </body>
+</html>