Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: consistently parse tabindex, following HTML 5 spec #4637

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
ahmedhalac marked this conversation as resolved.
Show resolved Hide resolved
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) {
ahmedhalac marked this conversation as resolved.
Show resolved Hide resolved
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';
ahmedhalac marked this conversation as resolved.
Show resolved Hide resolved
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;
5 changes: 3 additions & 2 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) {
ahmedhalac marked this conversation as resolved.
Show resolved Hide resolved
const roleDef = standards.ariaRoles[role];
if (roleDef === undefined || roleDef.type !== 'widget') {
return false;
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);
});
ahmedhalac marked this conversation as resolved.
Show resolved Hide resolved
});