From 84bb7d2b2675c49b31f6d7de12f9e1bc67ab5c94 Mon Sep 17 00:00:00 2001 From: ahmedhalac Date: Mon, 11 Nov 2024 21:58:20 +0100 Subject: [PATCH 1/4] refactor(#4632): correct tabindex parsing logic Ensures tabindex is correctly parsed as an integer using regex instead of parseInt to comply with HTML standards. Related to #4632 --- lib/commons/dom/is-focusable.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/commons/dom/is-focusable.js b/lib/commons/dom/is-focusable.js index b9411c9c0d..a79c5ea35d 100644 --- a/lib/commons/dom/is-focusable.js +++ b/lib/commons/dom/is-focusable.js @@ -24,9 +24,5 @@ export default function isFocusable(el) { } // check if the tabindex is specified and a parseable number const tabindex = vNode.attr('tabindex'); - if (tabindex && !isNaN(parseInt(tabindex, 10))) { - return true; - } - - return false; + return tabindex && /^\s*([-+]?\d+)/.test(tabindex); } From fcf78994436148eb52b763fe08d5d4fc6de1cfcf Mon Sep 17 00:00:00 2001 From: ahmedhalac Date: Wed, 13 Nov 2024 20:04:47 +0100 Subject: [PATCH 2/4] feat(#4632): add parseTabindex reusable method Add parseTabindex method in lib/core/utils and reused it in multiple places across the codebase Related to #4632 --- .../keyboard/focusable-no-name-evaluate.js | 6 ++-- .../keyboard/no-focusable-content-evaluate.js | 3 +- lib/checks/keyboard/tabindex-evaluate.js | 4 ++- lib/commons/dom/get-tabbable-elements.js | 5 ++- lib/commons/dom/inserted-into-focus-order.js | 3 +- lib/commons/dom/is-focusable.js | 5 +-- lib/commons/dom/is-in-tab-order.js | 3 +- lib/core/base/context/create-frame-context.js | 4 ++- lib/core/utils/index.js | 1 + lib/core/utils/parse-tabindex.js | 22 ++++++++++++ lib/rules/autocomplete-matches.js | 3 +- lib/rules/no-negative-tabindex-matches.js | 4 ++- test/core/utils/parse-tabindex.js | 35 +++++++++++++++++++ 13 files changed, 84 insertions(+), 14 deletions(-) create mode 100644 lib/core/utils/parse-tabindex.js create mode 100644 test/core/utils/parse-tabindex.js diff --git a/lib/checks/keyboard/focusable-no-name-evaluate.js b/lib/checks/keyboard/focusable-no-name-evaluate.js index 490470a67c..0b1e759a7c 100644 --- a/lib/checks/keyboard/focusable-no-name-evaluate.js +++ b/lib/checks/keyboard/focusable-no-name-evaluate.js @@ -1,9 +1,11 @@ import { isFocusable } from '../../commons/dom'; import { accessibleTextVirtual } from '../../commons/text'; +import { parseTabindex } from '../../core/utils'; function focusableNoNameEvaluate(node, options, virtualNode) { - const tabIndex = virtualNode.attr('tabindex'); - const inFocusOrder = isFocusable(virtualNode) && tabIndex > -1; + const tabIndex = parseTabindex(virtualNode.attr('tabindex')); + const inFocusOrder = + isFocusable(virtualNode) && tabIndex !== null && tabIndex > -1; if (!inFocusOrder) { return false; } diff --git a/lib/checks/keyboard/no-focusable-content-evaluate.js b/lib/checks/keyboard/no-focusable-content-evaluate.js index dd338a1dec..4be9dad804 100644 --- a/lib/checks/keyboard/no-focusable-content-evaluate.js +++ b/lib/checks/keyboard/no-focusable-content-evaluate.js @@ -1,5 +1,6 @@ import isFocusable from '../../commons/dom/is-focusable'; import { getRoleType } from '../../commons/aria'; +import { parseTabindex } from '../../core/utils'; export default function noFocusableContentEvaluate(node, options, virtualNode) { if (!virtualNode.children) { @@ -51,6 +52,6 @@ function getFocusableDescendants(vNode) { } function usesUnreliableHidingStrategy(vNode) { - const tabIndex = parseInt(vNode.attr('tabindex'), 10); + const tabIndex = parseTabindex(vNode.attr('tabindex')); return !isNaN(tabIndex) && tabIndex < 0; } diff --git a/lib/checks/keyboard/tabindex-evaluate.js b/lib/checks/keyboard/tabindex-evaluate.js index ffeeec5203..b0d60e5578 100644 --- a/lib/checks/keyboard/tabindex-evaluate.js +++ b/lib/checks/keyboard/tabindex-evaluate.js @@ -1,5 +1,7 @@ +import { parseTabindex } from '../../core/utils'; + function tabindexEvaluate(node, options, virtualNode) { - const tabIndex = parseInt(virtualNode.attr('tabindex'), 10); + const tabIndex = parseTabindex(virtualNode.attr('tabindex')); // an invalid tabindex will either return 0 or -1 (based on the element) so // will never be above 0 diff --git a/lib/commons/dom/get-tabbable-elements.js b/lib/commons/dom/get-tabbable-elements.js index 2836635643..85247bb0c1 100644 --- a/lib/commons/dom/get-tabbable-elements.js +++ b/lib/commons/dom/get-tabbable-elements.js @@ -1,4 +1,5 @@ import { querySelectorAll } from '../../core/utils'; +import { parseTabindex } from '../../core/utils'; /** * Get all elements (including given node) that are part of the tab order @@ -13,9 +14,7 @@ function getTabbableElements(virtualNode) { const tabbableElements = nodeAndDescendents.filter(vNode => { const isFocusable = vNode.isFocusable; - let tabIndex = vNode.actualNode.getAttribute('tabindex'); - tabIndex = - tabIndex && !isNaN(parseInt(tabIndex, 10)) ? parseInt(tabIndex) : null; + const tabIndex = parseTabindex(vNode.actualNode.getAttribute('tabindex')); return tabIndex ? isFocusable && tabIndex >= 0 : isFocusable; }); diff --git a/lib/commons/dom/inserted-into-focus-order.js b/lib/commons/dom/inserted-into-focus-order.js index 1eb927a891..4f6107d560 100644 --- a/lib/commons/dom/inserted-into-focus-order.js +++ b/lib/commons/dom/inserted-into-focus-order.js @@ -1,5 +1,6 @@ import isFocusable from './is-focusable'; import isNativelyFocusable from './is-natively-focusable'; +import { parseTabindex } from '../../core/utils'; /** * Determines if an element is in the focus order, but would not be if its @@ -12,7 +13,7 @@ import isNativelyFocusable from './is-natively-focusable'; * if its tabindex were removed. Else, false. */ function insertedIntoFocusOrder(el) { - const tabIndex = parseInt(el.getAttribute('tabindex'), 10); + const tabIndex = parseTabindex(el.getAttribute('tabindex')); // an element that has an invalid tabindex will return 0 or -1 based on // if it is natively focusable or not, which will always be false for this diff --git a/lib/commons/dom/is-focusable.js b/lib/commons/dom/is-focusable.js index a79c5ea35d..03be037864 100644 --- a/lib/commons/dom/is-focusable.js +++ b/lib/commons/dom/is-focusable.js @@ -1,6 +1,7 @@ import focusDisabled from './focus-disabled'; import isNativelyFocusable from './is-natively-focusable'; import { nodeLookup } from '../../core/utils'; +import { parseTabindex } from '../../core/utils'; /** * Determines if an element is keyboard or programmatically focusable. @@ -23,6 +24,6 @@ export default function isFocusable(el) { return true; } // check if the tabindex is specified and a parseable number - const tabindex = vNode.attr('tabindex'); - return tabindex && /^\s*([-+]?\d+)/.test(tabindex); + const tabindex = parseTabindex(vNode.attr('tabindex')); + return tabindex !== null; } diff --git a/lib/commons/dom/is-in-tab-order.js b/lib/commons/dom/is-in-tab-order.js index 7291b06e01..0baa67c2fe 100644 --- a/lib/commons/dom/is-in-tab-order.js +++ b/lib/commons/dom/is-in-tab-order.js @@ -1,5 +1,6 @@ import { nodeLookup } from '../../core/utils'; import isFocusable from './is-focusable'; +import { parseTabindex } from '../../core/utils'; /** * Determines if an element is focusable and able to be tabbed to. @@ -16,7 +17,7 @@ export default function isInTabOrder(el) { return false; } - const tabindex = parseInt(vNode.attr('tabindex', 10)); + const tabindex = parseTabindex(vNode.attr('tabindex')); if (tabindex <= -1) { return false; // Elements with tabindex=-1 are never in the tab order } diff --git a/lib/core/base/context/create-frame-context.js b/lib/core/base/context/create-frame-context.js index abb396ecf1..b0acf81731 100644 --- a/lib/core/base/context/create-frame-context.js +++ b/lib/core/base/context/create-frame-context.js @@ -1,3 +1,5 @@ +import { parseTabindex } from '../../utils'; + export function createFrameContext(frame, { focusable, page }) { return { node: frame, @@ -11,7 +13,7 @@ export function createFrameContext(frame, { focusable, page }) { } function frameFocusable(frame) { - const tabIndex = frame.getAttribute('tabindex'); + const tabIndex = parseTabindex(frame.getAttribute('tabindex')); if (!tabIndex) { return true; } diff --git a/lib/core/utils/index.js b/lib/core/utils/index.js index ac1286d6cf..81164587e2 100644 --- a/lib/core/utils/index.js +++ b/lib/core/utils/index.js @@ -71,6 +71,7 @@ export { default as objectHasOwn } from './object-has-own'; export { default as parseCrossOriginStylesheet } from './parse-crossorigin-stylesheet'; export { default as parseSameOriginStylesheet } from './parse-sameorigin-stylesheet'; export { default as parseStylesheet } from './parse-stylesheet'; +export { default as parseTabindex } from './parse-tabindex'; export { default as performanceTimer } from './performance-timer'; export { pollyfillElementsFromPoint } from './pollyfill-elements-from-point'; export { default as preloadCssom } from './preload-cssom'; diff --git a/lib/core/utils/parse-tabindex.js b/lib/core/utils/parse-tabindex.js new file mode 100644 index 0000000000..e9865f27f5 --- /dev/null +++ b/lib/core/utils/parse-tabindex.js @@ -0,0 +1,22 @@ +/** + * Parses a tabindex value to return an integer if valid, or null if invalid. + * @method parseTabindex + * @memberof axe.utils + * @param {string|null} str + * @return {number|null} + */ +function parseTabindex(value) { + if (value === null || typeof value !== 'string') { + return null; + } + + // Trim whitespace and check if it’s a valid integer format + const match = value.trim().match(/^([-+]?\d+)$/); + if (match) { + return Number(match[1]); + } + + return null; +} + +export default parseTabindex; diff --git a/lib/rules/autocomplete-matches.js b/lib/rules/autocomplete-matches.js index 23b622a846..69dda0a2d6 100644 --- a/lib/rules/autocomplete-matches.js +++ b/lib/rules/autocomplete-matches.js @@ -1,6 +1,7 @@ import { sanitize } from '../commons/text'; import standards from '../standards'; import { isVisibleToScreenReaders, isVisibleOnScreen } from '../commons/dom'; +import { parseTabindex } from '../core/utils'; function autocompleteMatches(node, virtualNode) { const autocomplete = virtualNode.attr('autocomplete'); @@ -34,7 +35,7 @@ function autocompleteMatches(node, virtualNode) { // The element has `tabindex="-1"` and has a [[semantic role]] that is // not a [widget](https://www.w3.org/TR/wai-aria-1.1/#widget_roles) const role = virtualNode.attr('role'); - const tabIndex = virtualNode.attr('tabindex'); + const tabIndex = parseTabindex(virtualNode.attr('tabindex')); if (tabIndex === '-1' && role) { const roleDef = standards.ariaRoles[role]; if (roleDef === undefined || roleDef.type !== 'widget') { diff --git a/lib/rules/no-negative-tabindex-matches.js b/lib/rules/no-negative-tabindex-matches.js index 1cc34736c0..ac8fe0a49e 100644 --- a/lib/rules/no-negative-tabindex-matches.js +++ b/lib/rules/no-negative-tabindex-matches.js @@ -1,5 +1,7 @@ +import { parseTabindex } from '../core/utils'; + function noNegativeTabindexMatches(node, virtualNode) { - const tabindex = parseInt(virtualNode.attr('tabindex'), 10); + const tabindex = parseTabindex(virtualNode.attr('tabindex')); return isNaN(tabindex) || tabindex >= 0; } diff --git a/test/core/utils/parse-tabindex.js b/test/core/utils/parse-tabindex.js new file mode 100644 index 0000000000..e21eacc780 --- /dev/null +++ b/test/core/utils/parse-tabindex.js @@ -0,0 +1,35 @@ +describe('axe.utils.parseTabindex', function () { + 'use strict'; + + it('should return 0 for "0"', function () { + assert.strictEqual(axe.utils.parseTabindex('0'), 0); + }); + + it('should return 1 for "+1"', function () { + assert.strictEqual(axe.utils.parseTabindex('+1'), 1); + }); + + it('should return -1 for "-1"', function () { + assert.strictEqual(axe.utils.parseTabindex('-1'), -1); + }); + + it('should return null for null', function () { + assert.strictEqual(axe.utils.parseTabindex(null), null); + }); + + it('should return null for an empty string', function () { + assert.strictEqual(axe.utils.parseTabindex(''), null); + }); + + it('should return null for a whitespace string', function () { + assert.strictEqual(axe.utils.parseTabindex(' '), null); + }); + + it('should return null for non-numeric strings', function () { + assert.strictEqual(axe.utils.parseTabindex('abc'), null); + }); + + it('should return null for decimal numbers', function () { + assert.strictEqual(axe.utils.parseTabindex('2.5'), null); + }); +}); From 8ba3f66c31d90c8a29d6947cbbacf5b2143c81fc Mon Sep 17 00:00:00 2001 From: ahmedhalac Date: Fri, 15 Nov 2024 20:19:57 +0100 Subject: [PATCH 3/4] refactor(#4637): resolving PR comments Resolving pull request comments Related to #4637 --- lib/checks/keyboard/focusable-no-name-evaluate.js | 8 ++------ lib/checks/keyboard/no-focusable-content-evaluate.js | 2 +- lib/checks/keyboard/tabindex-evaluate.js | 2 +- lib/commons/dom/get-tabbable-elements.js | 2 +- lib/core/base/context/create-frame-context.js | 6 +----- lib/core/utils/parse-tabindex.js | 6 +++--- lib/rules/autocomplete-matches.js | 2 +- lib/rules/no-negative-tabindex-matches.js | 2 +- test/core/utils/parse-tabindex.js | 8 ++++++-- 9 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/checks/keyboard/focusable-no-name-evaluate.js b/lib/checks/keyboard/focusable-no-name-evaluate.js index 0b1e759a7c..49ee28b887 100644 --- a/lib/checks/keyboard/focusable-no-name-evaluate.js +++ b/lib/checks/keyboard/focusable-no-name-evaluate.js @@ -1,12 +1,8 @@ -import { isFocusable } from '../../commons/dom'; +import { isInTabOrder } from '../../commons/dom'; import { accessibleTextVirtual } from '../../commons/text'; -import { parseTabindex } from '../../core/utils'; function focusableNoNameEvaluate(node, options, virtualNode) { - const tabIndex = parseTabindex(virtualNode.attr('tabindex')); - const inFocusOrder = - isFocusable(virtualNode) && tabIndex !== null && tabIndex > -1; - if (!inFocusOrder) { + if (!isInTabOrder(virtualNode)) { return false; } diff --git a/lib/checks/keyboard/no-focusable-content-evaluate.js b/lib/checks/keyboard/no-focusable-content-evaluate.js index 4be9dad804..17a6ff7ba2 100644 --- a/lib/checks/keyboard/no-focusable-content-evaluate.js +++ b/lib/checks/keyboard/no-focusable-content-evaluate.js @@ -53,5 +53,5 @@ function getFocusableDescendants(vNode) { function usesUnreliableHidingStrategy(vNode) { const tabIndex = parseTabindex(vNode.attr('tabindex')); - return !isNaN(tabIndex) && tabIndex < 0; + return tabIndex !== null && tabIndex < 0; } diff --git a/lib/checks/keyboard/tabindex-evaluate.js b/lib/checks/keyboard/tabindex-evaluate.js index b0d60e5578..5380345c2b 100644 --- a/lib/checks/keyboard/tabindex-evaluate.js +++ b/lib/checks/keyboard/tabindex-evaluate.js @@ -6,7 +6,7 @@ function tabindexEvaluate(node, options, virtualNode) { // an invalid tabindex will either return 0 or -1 (based on the element) so // will never be above 0 // @see https://www.w3.org/TR/html51/editing.html#the-tabindex-attribute - return isNaN(tabIndex) ? true : tabIndex <= 0; + return tabIndex === null || tabIndex <= 0; } export default tabindexEvaluate; diff --git a/lib/commons/dom/get-tabbable-elements.js b/lib/commons/dom/get-tabbable-elements.js index 85247bb0c1..dfa2925f8a 100644 --- a/lib/commons/dom/get-tabbable-elements.js +++ b/lib/commons/dom/get-tabbable-elements.js @@ -16,7 +16,7 @@ function getTabbableElements(virtualNode) { const isFocusable = vNode.isFocusable; const tabIndex = parseTabindex(vNode.actualNode.getAttribute('tabindex')); - return tabIndex ? isFocusable && tabIndex >= 0 : isFocusable; + return tabIndex !== null ? isFocusable && tabIndex >= 0 : isFocusable; }); return tabbableElements; diff --git a/lib/core/base/context/create-frame-context.js b/lib/core/base/context/create-frame-context.js index b0acf81731..c348f58483 100644 --- a/lib/core/base/context/create-frame-context.js +++ b/lib/core/base/context/create-frame-context.js @@ -14,11 +14,7 @@ export function createFrameContext(frame, { focusable, page }) { function frameFocusable(frame) { const tabIndex = parseTabindex(frame.getAttribute('tabindex')); - if (!tabIndex) { - return true; - } - const int = parseInt(tabIndex, 10); - return isNaN(int) || int >= 0; + return tabIndex === null || tabIndex >= 0; } function getBoundingSize(domNode) { diff --git a/lib/core/utils/parse-tabindex.js b/lib/core/utils/parse-tabindex.js index e9865f27f5..c62cc5c517 100644 --- a/lib/core/utils/parse-tabindex.js +++ b/lib/core/utils/parse-tabindex.js @@ -6,12 +6,12 @@ * @return {number|null} */ function parseTabindex(value) { - if (value === null || typeof value !== 'string') { + if (typeof value !== 'string') { return null; } - // Trim whitespace and check if it’s a valid integer format - const match = value.trim().match(/^([-+]?\d+)$/); + // spec: https://html.spec.whatwg.org/#rules-for-parsing-integers + const match = value.trim().match(/^([-+]?\d+)/); if (match) { return Number(match[1]); } diff --git a/lib/rules/autocomplete-matches.js b/lib/rules/autocomplete-matches.js index 69dda0a2d6..9ae286df2c 100644 --- a/lib/rules/autocomplete-matches.js +++ b/lib/rules/autocomplete-matches.js @@ -36,7 +36,7 @@ function autocompleteMatches(node, virtualNode) { // not a [widget](https://www.w3.org/TR/wai-aria-1.1/#widget_roles) const role = virtualNode.attr('role'); const tabIndex = parseTabindex(virtualNode.attr('tabindex')); - if (tabIndex === '-1' && role) { + if (tabIndex < 0 && role) { const roleDef = standards.ariaRoles[role]; if (roleDef === undefined || roleDef.type !== 'widget') { return false; diff --git a/lib/rules/no-negative-tabindex-matches.js b/lib/rules/no-negative-tabindex-matches.js index ac8fe0a49e..72816ecd78 100644 --- a/lib/rules/no-negative-tabindex-matches.js +++ b/lib/rules/no-negative-tabindex-matches.js @@ -2,7 +2,7 @@ import { parseTabindex } from '../core/utils'; function noNegativeTabindexMatches(node, virtualNode) { const tabindex = parseTabindex(virtualNode.attr('tabindex')); - return isNaN(tabindex) || tabindex >= 0; + return tabindex === null || tabindex >= 0; } export default noNegativeTabindexMatches; diff --git a/test/core/utils/parse-tabindex.js b/test/core/utils/parse-tabindex.js index e21eacc780..4dca8e99d0 100644 --- a/test/core/utils/parse-tabindex.js +++ b/test/core/utils/parse-tabindex.js @@ -29,7 +29,11 @@ describe('axe.utils.parseTabindex', function () { assert.strictEqual(axe.utils.parseTabindex('abc'), null); }); - it('should return null for decimal numbers', function () { - assert.strictEqual(axe.utils.parseTabindex('2.5'), null); + it('should return the first valid digit(s) for decimal numbers', function () { + assert.strictEqual(axe.utils.parseTabindex('2.5'), 2); + }); + + it('should return 123 for "123abc"', function () { + assert.strictEqual(axe.utils.parseTabindex('123abc'), 123); }); }); From 91210e9ab2b5989235588c17ed3bc8863485051a Mon Sep 17 00:00:00 2001 From: ahmedhalac Date: Tue, 19 Nov 2024 22:44:53 +0100 Subject: [PATCH 4/4] fix(#4632): Fixed tabindex condition Related to #4632 --- lib/rules/autocomplete-matches.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/autocomplete-matches.js b/lib/rules/autocomplete-matches.js index 9ae286df2c..69f678e73f 100644 --- a/lib/rules/autocomplete-matches.js +++ b/lib/rules/autocomplete-matches.js @@ -45,7 +45,7 @@ function autocompleteMatches(node, virtualNode) { // The element is **not** visible on the page or exposed to assistive technologies if ( - tabIndex === '-1' && + tabIndex < 0 && virtualNode.actualNode && !isVisibleOnScreen(virtualNode) && !isVisibleToScreenReaders(virtualNode)