Skip to content

Commit

Permalink
fix: consistently parse tabindex, following HTML 5 spec (#4637)
Browse files Browse the repository at this point in the history
Ensures tabindex is correctly parsed as an integer using regex instead
of parseInt to comply with HTML standards.

Closes #4632
  • Loading branch information
ahmedhalac authored Nov 25, 2024
1 parent ec7c6c8 commit 645a850
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 31 deletions.
6 changes: 2 additions & 4 deletions lib/checks/keyboard/focusable-no-name-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { isFocusable } from '../../commons/dom';
import { isInTabOrder } from '../../commons/dom';
import { accessibleTextVirtual } from '../../commons/text';

function focusableNoNameEvaluate(node, options, virtualNode) {
const tabIndex = virtualNode.attr('tabindex');
const inFocusOrder = isFocusable(virtualNode) && tabIndex > -1;
if (!inFocusOrder) {
if (!isInTabOrder(virtualNode)) {
return false;
}

Expand Down
5 changes: 3 additions & 2 deletions lib/checks/keyboard/no-focusable-content-evaluate.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -51,6 +52,6 @@ function getFocusableDescendants(vNode) {
}

function usesUnreliableHidingStrategy(vNode) {
const tabIndex = parseInt(vNode.attr('tabindex'), 10);
return !isNaN(tabIndex) && tabIndex < 0;
const tabIndex = parseTabindex(vNode.attr('tabindex'));
return tabIndex !== null && tabIndex < 0;
}
6 changes: 4 additions & 2 deletions lib/checks/keyboard/tabindex-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
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
// @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;
7 changes: 3 additions & 4 deletions lib/commons/dom/get-tabbable-elements.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -13,11 +14,9 @@ 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;
return tabIndex !== null ? isFocusable && tabIndex >= 0 : isFocusable;
});

return tabbableElements;
Expand Down
3 changes: 2 additions & 1 deletion lib/commons/dom/inserted-into-focus-order.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
9 changes: 3 additions & 6 deletions lib/commons/dom/is-focusable.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -23,10 +24,6 @@ export default function isFocusable(el) {
return true;
}
// 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;
const tabindex = parseTabindex(vNode.attr('tabindex'));
return tabindex !== null;
}
3 changes: 2 additions & 1 deletion lib/commons/dom/is-in-tab-order.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
}
Expand Down
10 changes: 4 additions & 6 deletions lib/core/base/context/create-frame-context.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { parseTabindex } from '../../utils';

export function createFrameContext(frame, { focusable, page }) {
return {
node: frame,
Expand All @@ -11,12 +13,8 @@ export function createFrameContext(frame, { focusable, page }) {
}

function frameFocusable(frame) {
const tabIndex = frame.getAttribute('tabindex');
if (!tabIndex) {
return true;
}
const int = parseInt(tabIndex, 10);
return isNaN(int) || int >= 0;
const tabIndex = parseTabindex(frame.getAttribute('tabindex'));
return tabIndex === null || tabIndex >= 0;
}

function getBoundingSize(domNode) {
Expand Down
1 change: 1 addition & 0 deletions lib/core/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
22 changes: 22 additions & 0 deletions lib/core/utils/parse-tabindex.js
Original file line number Diff line number Diff line change
@@ -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 (typeof value !== 'string') {
return null;
}

// spec: https://html.spec.whatwg.org/#rules-for-parsing-integers
const match = value.trim().match(/^([-+]?\d+)/);
if (match) {
return Number(match[1]);
}

return null;
}

export default parseTabindex;
7 changes: 4 additions & 3 deletions lib/rules/autocomplete-matches.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -34,8 +35,8 @@ 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');
if (tabIndex === '-1' && role) {
const tabIndex = parseTabindex(virtualNode.attr('tabindex'));
if (tabIndex < 0 && role) {
const roleDef = standards.ariaRoles[role];
if (roleDef === undefined || roleDef.type !== 'widget') {
return false;
Expand All @@ -44,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)
Expand Down
6 changes: 4 additions & 2 deletions lib/rules/no-negative-tabindex-matches.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { parseTabindex } from '../core/utils';

function noNegativeTabindexMatches(node, virtualNode) {
const tabindex = parseInt(virtualNode.attr('tabindex'), 10);
return isNaN(tabindex) || tabindex >= 0;
const tabindex = parseTabindex(virtualNode.attr('tabindex'));
return tabindex === null || tabindex >= 0;
}

export default noNegativeTabindexMatches;
39 changes: 39 additions & 0 deletions test/core/utils/parse-tabindex.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
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 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);
});
});

0 comments on commit 645a850

Please sign in to comment.