diff --git a/lib/commons/aria/get-role.js b/lib/commons/aria/get-role.js index fb5fc7347f..3a3cc1a79b 100644 --- a/lib/commons/aria/get-role.js +++ b/lib/commons/aria/get-role.js @@ -3,7 +3,7 @@ import getImplicitRole from './implicit-role'; import getGlobalAriaAttrs from '../standards/get-global-aria-attrs'; import isFocusable from '../dom/is-focusable'; import { getNodeFromTree } from '../../core/utils'; -import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node'; +import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node'; // when an element inherits the presentational role from a parent // is not defined in the spec, but through testing it seems to be @@ -126,7 +126,7 @@ function hasConflictResolution(vNode) { */ function resolveRole(node, { noImplicit, ...roleOptions } = {}) { const vNode = - node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node); + node instanceof AbstractVirtualNode ? node : getNodeFromTree(node); if (vNode.props.nodeType !== 1) { return null; } diff --git a/lib/commons/aria/index.js b/lib/commons/aria/index.js index d9f7bd4805..9eb5bb3a88 100644 --- a/lib/commons/aria/index.js +++ b/lib/commons/aria/index.js @@ -19,6 +19,7 @@ export { default as implicitNodes } from './implicit-nodes'; export { default as implicitRole } from './implicit-role'; export { default as isAccessibleRef } from './is-accessible-ref'; export { default as isAriaRoleAllowedOnElement } from './is-aria-role-allowed-on-element'; +export { default as isComboboxPopup } from './is-combobox-popup'; export { default as isUnsupportedRole } from './is-unsupported-role'; export { default as isValidRole } from './is-valid-role'; export { default as labelVirtual } from './label-virtual'; diff --git a/lib/commons/aria/is-combobox-popup.js b/lib/commons/aria/is-combobox-popup.js new file mode 100644 index 0000000000..bef80f83be --- /dev/null +++ b/lib/commons/aria/is-combobox-popup.js @@ -0,0 +1,55 @@ +import getRole from './get-role'; +import ariaAttrs from '../../standards/aria-attrs'; +import { getRootNode } from '../../core/utils'; + +/** + * Whether an element is the popup for a combobox + * @method isComboboxPopup + * @memberof axe.commons.aria + * @instance + * @param {VirtualNode} virtualNode + * @param {Object} options + * @property {String[]} popupRoles Overrides which roles can be popup. Defaults to aria-haspopup values + * @returns {boolean} + */ +export default function isComboboxPopup(virtualNode, { popupRoles } = {}) { + const role = getRole(virtualNode); + popupRoles ??= ariaAttrs['aria-haspopup'].values; + if (!popupRoles.includes(role)) { + return false; + } + + // in ARIA 1.1 the container has role=combobox + const vParent = nearestParentWithRole(virtualNode); + if (isCombobox(vParent)) { + return true; + } + + const { id } = virtualNode.props; + if (!id) { + return false; + } + + if (!virtualNode.actualNode) { + throw new Error('Unable to determine combobox popup without an actualNode'); + } + const root = getRootNode(virtualNode.actualNode); + const ownedCombobox = root.querySelectorAll( + // aria-owns was from ARIA 1.0, aria-controls was from ARIA 1.2 + `[aria-owns~="${id}"][role~="combobox"]:not(select), + [aria-controls~="${id}"][role~="combobox"]:not(select)` + ); + + return Array.from(ownedCombobox).some(isCombobox); +} + +const isCombobox = node => node && getRole(node) === 'combobox'; + +function nearestParentWithRole(vNode) { + while ((vNode = vNode.parent)) { + if (getRole(vNode, { noPresentational: true }) !== null) { + return vNode; + } + } + return null; +} diff --git a/lib/rules/no-naming-method-matches.js b/lib/rules/no-naming-method-matches.js index 2986f8247b..cd8c973f3b 100644 --- a/lib/rules/no-naming-method-matches.js +++ b/lib/rules/no-naming-method-matches.js @@ -1,6 +1,7 @@ -import { getExplicitRole } from '../commons/aria'; -import { querySelectorAll } from '../core/utils'; +import getExplicitRole from '../commons/aria/get-explicit-role'; +import isComboboxPopup from '../commons/aria/is-combobox-popup'; import getElementSpec from '../commons/standards/get-element-spec'; +import querySelectorAll from '../core/utils/query-selector-all'; /** * Filter out elements that have a naming method (i.e. img[alt], table > caption, etc.) @@ -10,7 +11,6 @@ function noNamingMethodMatches(node, virtualNode) { if (namingMethods && namingMethods.length !== 0) { return false; } - // Additionally, ignore combobox that get their name from a descendant input: if ( getExplicitRole(virtualNode) === 'combobox' && @@ -18,6 +18,11 @@ function noNamingMethodMatches(node, virtualNode) { ) { return false; } + // Ignore listboxes that are referenced by a combobox + // Other roles don't require a name at all, or require one anyway + if (isComboboxPopup(virtualNode, { popupRoles: ['listbox'] })) { + return false; + } return true; } diff --git a/lib/rules/scrollable-region-focusable-matches.js b/lib/rules/scrollable-region-focusable-matches.js index 71acf09d24..34c4b08d77 100644 --- a/lib/rules/scrollable-region-focusable-matches.js +++ b/lib/rules/scrollable-region-focusable-matches.js @@ -1,75 +1,21 @@ -import { hasContentVirtual } from '../commons/dom'; -import { getExplicitRole } from '../commons/aria'; -import { - querySelectorAll, - getScroll, - closest, - getRootNode, - tokenList -} from '../core/utils'; -import ariaAttrs from '../standards/aria-attrs'; - -function scrollableRegionFocusableMatches(node, virtualNode) { - /** - * Note: - * `excludeHidden=true` for this rule, thus considering only elements in the accessibility tree. - */ - - /** - * if not scrollable -> `return` - */ - if (!!getScroll(node, 13) === false) { - return false; - } - - /** - * ignore scrollable regions owned by combobox. limit to roles - * ownable by combobox so we don't keep calling closest for every - * node (which would be slow) - * @see https://github.com/dequelabs/axe-core/issues/1763 - */ - const role = getExplicitRole(virtualNode); - if (ariaAttrs['aria-haspopup'].values.includes(role)) { - // in ARIA 1.1 the container has role=combobox - if (closest(virtualNode, '[role~="combobox"]')) { - return false; - } - - // in ARIA 1.0 and 1.2 the combobox owns (1.0) or controls (1.2) - // the listbox - const id = virtualNode.attr('id'); - if (id) { - const doc = getRootNode(node); - const owned = Array.from( - doc.querySelectorAll(`[aria-owns~="${id}"], [aria-controls~="${id}"]`) - ); - const comboboxOwned = owned.some(el => { - const roles = tokenList(el.getAttribute('role')); - return roles.includes('combobox'); - }); - - if (comboboxOwned) { - return false; - } - } - } - - /** - * check if node has visible contents - */ - const nodeAndDescendents = querySelectorAll(virtualNode, '*'); - const hasVisibleChildren = nodeAndDescendents.some(elm => - hasContentVirtual( - elm, - true, // noRecursion - true // ignoreAria - ) +import hasContentVirtual from '../commons/dom/has-content-virtual'; +import isComboboxPopup from '../commons/aria/is-combobox-popup'; +import { querySelectorAll, getScroll } from '../core/utils'; + +export default function scrollableRegionFocusableMatches(node, virtualNode) { + return ( + // The element scrolls + getScroll(node, 13) !== undefined && + // It's not a combobox popup, which commonly has keyboard focus added + isComboboxPopup(virtualNode) === false && + // And there's something actually worth scrolling to + isNoneEmptyElement(virtualNode) ); - if (!hasVisibleChildren) { - return false; - } - - return true; } -export default scrollableRegionFocusableMatches; +function isNoneEmptyElement(vNode) { + return querySelectorAll(vNode, '*').some(elm => + // (elm, noRecursion, ignoreAria) + hasContentVirtual(elm, true, true) + ); +} diff --git a/test/commons/aria/is-combobox-popup.js b/test/commons/aria/is-combobox-popup.js new file mode 100644 index 0000000000..65ff04ac2b --- /dev/null +++ b/test/commons/aria/is-combobox-popup.js @@ -0,0 +1,145 @@ +describe('isComboboxPopup', () => { + const { isComboboxPopup } = axe.commons.aria; + const { queryFixture } = axe.testUtils; + + it('does not match non-popup roles', () => { + const roles = ['main', 'combobox', 'textbox', 'button']; + for (const role of roles) { + const vNode = queryFixture( + `
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + } + }); + + for (const role of ['menu', 'listbox', 'tree', 'grid', 'dialog']) { + describe(role, () => { + it('is false when not related to the combobox', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + + describe('using aria-controls (ARIA 1.2 pattern)', () => { + it('is true when referenced', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isTrue(isComboboxPopup(vNode)); + }); + + it('is false when controlled by a select element', () => { + const vNode = queryFixture( + ` +
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + + it('is false when not controlled by a combobox', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + }); + + describe('using parent owned (ARIA 1.1 pattern)', () => { + it('is true when its a child of the combobox', () => { + const vNode = queryFixture( + `
+
+
` + ); + assert.isTrue(isComboboxPopup(vNode)); + }); + + it('is false when its not a child of a real combobox', () => { + const vNode = queryFixture( + `
+
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + + it('is false when its nearest parent with a role is not a combobox', () => { + const vNode = queryFixture( + `
+
+
+
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + + it('is true when its nearest parent with a role is not a combobox', () => { + const vNode = queryFixture( + `
+
+
+
+
+
+
+
+
` + ); + assert.isTrue(isComboboxPopup(vNode)); + }); + }); + + describe('when using aria-owns (ARIA 1.0 pattern)', () => { + it('is true when referenced', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isTrue(isComboboxPopup(vNode)); + }); + + it('is false when owned by a select element', () => { + const vNode = queryFixture( + ` +
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + + it('is false when not owned by a combobox', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + }); + }); + }); + } + + describe('options.popupRoles', () => { + it('allows custom popup roles', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isFalse(isComboboxPopup(vNode)); + assert.isTrue(isComboboxPopup(vNode, { popupRoles: ['button'] })); + }); + + it('overrides the default popup roles', () => { + const vNode = queryFixture( + `
+
` + ); + assert.isTrue(isComboboxPopup(vNode)); + assert.isFalse(isComboboxPopup(vNode, { popupRoles: ['button'] })); + }); + }); +}); diff --git a/test/integration/rules/aria-input-field-name/aria-input-field-name.html b/test/integration/rules/aria-input-field-name/aria-input-field-name.html index bfd15eb387..a83a78cd27 100644 --- a/test/integration/rules/aria-input-field-name/aria-input-field-name.html +++ b/test/integration/rules/aria-input-field-name/aria-input-field-name.html @@ -1,6 +1,21 @@ -
England
+
+ England +
+ + + +

Select a color:

@@ -87,13 +102,13 @@ - - + - + diff --git a/test/rule-matches/no-naming-method-matches.js b/test/rule-matches/no-naming-method-matches.js index fd00c1eed8..c92213817e 100644 --- a/test/rule-matches/no-naming-method-matches.js +++ b/test/rule-matches/no-naming-method-matches.js @@ -69,6 +69,24 @@ describe('no-naming-method-matches', function () { assert.isFalse(actual); }); + it('returns false for the listbox popup of a role=`combobox`', function () { + var vNode = queryFixture( + '
' + + '
' + ); + var actual = rule.matches(null, vNode); + assert.isFalse(actual); + }); + + it('returns true for the dialog popup of a role=`combobox`', function () { + var vNode = queryFixture( + '
' + + '' + ); + var actual = rule.matches(null, vNode); + assert.isTrue(actual); + }); + it('returns true for a div with role=`button`', function () { var vNode = queryFixture('
'); var actual = rule.matches(null, vNode);