Skip to content

Commit 3dcdd42

Browse files
authored
fix(aria-input-field-name): skip combobox popups (#3886)
* fix(aria-input-field-name): skip combobox popups * skip popups on no-naming-method-matches * Test isComboboxPopup * Improve to run better on virtual trees * modals require a name though * Resolve feedback
1 parent da19946 commit 3dcdd42

8 files changed

Lines changed: 266 additions & 81 deletions

File tree

lib/commons/aria/get-role.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import getImplicitRole from './implicit-role';
33
import getGlobalAriaAttrs from '../standards/get-global-aria-attrs';
44
import isFocusable from '../dom/is-focusable';
55
import { getNodeFromTree } from '../../core/utils';
6-
import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node';
6+
import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node';
77

88
// when an element inherits the presentational role from a parent
99
// is not defined in the spec, but through testing it seems to be
@@ -126,7 +126,7 @@ function hasConflictResolution(vNode) {
126126
*/
127127
function resolveRole(node, { noImplicit, ...roleOptions } = {}) {
128128
const vNode =
129-
node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node);
129+
node instanceof AbstractVirtualNode ? node : getNodeFromTree(node);
130130
if (vNode.props.nodeType !== 1) {
131131
return null;
132132
}

lib/commons/aria/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export { default as implicitNodes } from './implicit-nodes';
1919
export { default as implicitRole } from './implicit-role';
2020
export { default as isAccessibleRef } from './is-accessible-ref';
2121
export { default as isAriaRoleAllowedOnElement } from './is-aria-role-allowed-on-element';
22+
export { default as isComboboxPopup } from './is-combobox-popup';
2223
export { default as isUnsupportedRole } from './is-unsupported-role';
2324
export { default as isValidRole } from './is-valid-role';
2425
export { default as labelVirtual } from './label-virtual';
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import getRole from './get-role';
2+
import ariaAttrs from '../../standards/aria-attrs';
3+
import { getRootNode } from '../../core/utils';
4+
5+
/**
6+
* Whether an element is the popup for a combobox
7+
* @method isComboboxPopup
8+
* @memberof axe.commons.aria
9+
* @instance
10+
* @param {VirtualNode} virtualNode
11+
* @param {Object} options
12+
* @property {String[]} popupRoles Overrides which roles can be popup. Defaults to aria-haspopup values
13+
* @returns {boolean}
14+
*/
15+
export default function isComboboxPopup(virtualNode, { popupRoles } = {}) {
16+
const role = getRole(virtualNode);
17+
popupRoles ??= ariaAttrs['aria-haspopup'].values;
18+
if (!popupRoles.includes(role)) {
19+
return false;
20+
}
21+
22+
// in ARIA 1.1 the container has role=combobox
23+
const vParent = nearestParentWithRole(virtualNode);
24+
if (isCombobox(vParent)) {
25+
return true;
26+
}
27+
28+
const { id } = virtualNode.props;
29+
if (!id) {
30+
return false;
31+
}
32+
33+
if (!virtualNode.actualNode) {
34+
throw new Error('Unable to determine combobox popup without an actualNode');
35+
}
36+
const root = getRootNode(virtualNode.actualNode);
37+
const ownedCombobox = root.querySelectorAll(
38+
// aria-owns was from ARIA 1.0, aria-controls was from ARIA 1.2
39+
`[aria-owns~="${id}"][role~="combobox"]:not(select),
40+
[aria-controls~="${id}"][role~="combobox"]:not(select)`
41+
);
42+
43+
return Array.from(ownedCombobox).some(isCombobox);
44+
}
45+
46+
const isCombobox = node => node && getRole(node) === 'combobox';
47+
48+
function nearestParentWithRole(vNode) {
49+
while ((vNode = vNode.parent)) {
50+
if (getRole(vNode, { noPresentational: true }) !== null) {
51+
return vNode;
52+
}
53+
}
54+
return null;
55+
}

lib/rules/no-naming-method-matches.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { getExplicitRole } from '../commons/aria';
2-
import { querySelectorAll } from '../core/utils';
1+
import getExplicitRole from '../commons/aria/get-explicit-role';
2+
import isComboboxPopup from '../commons/aria/is-combobox-popup';
33
import getElementSpec from '../commons/standards/get-element-spec';
4+
import querySelectorAll from '../core/utils/query-selector-all';
45

56
/**
67
* Filter out elements that have a naming method (i.e. img[alt], table > caption, etc.)
@@ -10,14 +11,18 @@ function noNamingMethodMatches(node, virtualNode) {
1011
if (namingMethods && namingMethods.length !== 0) {
1112
return false;
1213
}
13-
1414
// Additionally, ignore combobox that get their name from a descendant input:
1515
if (
1616
getExplicitRole(virtualNode) === 'combobox' &&
1717
querySelectorAll(virtualNode, 'input:not([type="hidden"])').length
1818
) {
1919
return false;
2020
}
21+
// Ignore listboxes that are referenced by a combobox
22+
// Other roles don't require a name at all, or require one anyway
23+
if (isComboboxPopup(virtualNode, { popupRoles: ['listbox'] })) {
24+
return false;
25+
}
2126
return true;
2227
}
2328

Lines changed: 18 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,21 @@
1-
import { hasContentVirtual } from '../commons/dom';
2-
import { getExplicitRole } from '../commons/aria';
3-
import {
4-
querySelectorAll,
5-
getScroll,
6-
closest,
7-
getRootNode,
8-
tokenList
9-
} from '../core/utils';
10-
import ariaAttrs from '../standards/aria-attrs';
11-
12-
function scrollableRegionFocusableMatches(node, virtualNode) {
13-
/**
14-
* Note:
15-
* `excludeHidden=true` for this rule, thus considering only elements in the accessibility tree.
16-
*/
17-
18-
/**
19-
* if not scrollable -> `return`
20-
*/
21-
if (!!getScroll(node, 13) === false) {
22-
return false;
23-
}
24-
25-
/**
26-
* ignore scrollable regions owned by combobox. limit to roles
27-
* ownable by combobox so we don't keep calling closest for every
28-
* node (which would be slow)
29-
* @see https://github.com/dequelabs/axe-core/issues/1763
30-
*/
31-
const role = getExplicitRole(virtualNode);
32-
if (ariaAttrs['aria-haspopup'].values.includes(role)) {
33-
// in ARIA 1.1 the container has role=combobox
34-
if (closest(virtualNode, '[role~="combobox"]')) {
35-
return false;
36-
}
37-
38-
// in ARIA 1.0 and 1.2 the combobox owns (1.0) or controls (1.2)
39-
// the listbox
40-
const id = virtualNode.attr('id');
41-
if (id) {
42-
const doc = getRootNode(node);
43-
const owned = Array.from(
44-
doc.querySelectorAll(`[aria-owns~="${id}"], [aria-controls~="${id}"]`)
45-
);
46-
const comboboxOwned = owned.some(el => {
47-
const roles = tokenList(el.getAttribute('role'));
48-
return roles.includes('combobox');
49-
});
50-
51-
if (comboboxOwned) {
52-
return false;
53-
}
54-
}
55-
}
56-
57-
/**
58-
* check if node has visible contents
59-
*/
60-
const nodeAndDescendents = querySelectorAll(virtualNode, '*');
61-
const hasVisibleChildren = nodeAndDescendents.some(elm =>
62-
hasContentVirtual(
63-
elm,
64-
true, // noRecursion
65-
true // ignoreAria
66-
)
1+
import hasContentVirtual from '../commons/dom/has-content-virtual';
2+
import isComboboxPopup from '../commons/aria/is-combobox-popup';
3+
import { querySelectorAll, getScroll } from '../core/utils';
4+
5+
export default function scrollableRegionFocusableMatches(node, virtualNode) {
6+
return (
7+
// The element scrolls
8+
getScroll(node, 13) !== undefined &&
9+
// It's not a combobox popup, which commonly has keyboard focus added
10+
isComboboxPopup(virtualNode) === false &&
11+
// And there's something actually worth scrolling to
12+
isNoneEmptyElement(virtualNode)
6713
);
68-
if (!hasVisibleChildren) {
69-
return false;
70-
}
71-
72-
return true;
7314
}
7415

75-
export default scrollableRegionFocusableMatches;
16+
function isNoneEmptyElement(vNode) {
17+
return querySelectorAll(vNode, '*').some(elm =>
18+
// (elm, noRecursion, ignoreAria)
19+
hasContentVirtual(elm, true, true)
20+
);
21+
}
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
describe('isComboboxPopup', () => {
2+
const { isComboboxPopup } = axe.commons.aria;
3+
const { queryFixture } = axe.testUtils;
4+
5+
it('does not match non-popup roles', () => {
6+
const roles = ['main', 'combobox', 'textbox', 'button'];
7+
for (const role of roles) {
8+
const vNode = queryFixture(
9+
`<div role="combobox" aria-controls="target"></div>
10+
<div role="${role}" id="target"></div>`
11+
);
12+
assert.isFalse(isComboboxPopup(vNode));
13+
}
14+
});
15+
16+
for (const role of ['menu', 'listbox', 'tree', 'grid', 'dialog']) {
17+
describe(role, () => {
18+
it('is false when not related to the combobox', () => {
19+
const vNode = queryFixture(
20+
`<div role="combobox"></div>
21+
<div role="${role}" id="target"></div>`
22+
);
23+
assert.isFalse(isComboboxPopup(vNode));
24+
});
25+
26+
describe('using aria-controls (ARIA 1.2 pattern)', () => {
27+
it('is true when referenced', () => {
28+
const vNode = queryFixture(
29+
`<div role="combobox" aria-controls="target"></div>
30+
<div role="${role}" id="target"></div>`
31+
);
32+
assert.isTrue(isComboboxPopup(vNode));
33+
});
34+
35+
it('is false when controlled by a select element', () => {
36+
const vNode = queryFixture(
37+
`<select role="combobox" aria-controls="target"></select>
38+
<div role="${role}" id="target"></div>`
39+
);
40+
assert.isFalse(isComboboxPopup(vNode));
41+
});
42+
43+
it('is false when not controlled by a combobox', () => {
44+
const vNode = queryFixture(
45+
`<div role="button combobox" aria-controls="target"></div>
46+
<div role="${role}" id="target"></div>`
47+
);
48+
assert.isFalse(isComboboxPopup(vNode));
49+
});
50+
});
51+
52+
describe('using parent owned (ARIA 1.1 pattern)', () => {
53+
it('is true when its a child of the combobox', () => {
54+
const vNode = queryFixture(
55+
`<div role="combobox">
56+
<div role="${role}" id="target"></div>
57+
</div>`
58+
);
59+
assert.isTrue(isComboboxPopup(vNode));
60+
});
61+
62+
it('is false when its not a child of a real combobox', () => {
63+
const vNode = queryFixture(
64+
`<div role="button combobox">
65+
<div role="${role}" id="target"></div>
66+
</div>`
67+
);
68+
assert.isFalse(isComboboxPopup(vNode));
69+
});
70+
71+
it('is false when its nearest parent with a role is not a combobox', () => {
72+
const vNode = queryFixture(
73+
`<div role="combobox">
74+
<div role="region">
75+
<div role="${role}" id="target"></div>
76+
</div>
77+
</div>`
78+
);
79+
assert.isFalse(isComboboxPopup(vNode));
80+
});
81+
82+
it('is true when its nearest parent with a role is not a combobox', () => {
83+
const vNode = queryFixture(
84+
`<div role="combobox">
85+
<div>
86+
<div role="none">
87+
<div role="presentation">
88+
<div role="${role}" id="target"></div>
89+
</div>
90+
</div>
91+
</div>
92+
</div>`
93+
);
94+
assert.isTrue(isComboboxPopup(vNode));
95+
});
96+
});
97+
98+
describe('when using aria-owns (ARIA 1.0 pattern)', () => {
99+
it('is true when referenced', () => {
100+
const vNode = queryFixture(
101+
`<div role="combobox" aria-owns="target"></div>
102+
<div role="${role}" id="target"></div>`
103+
);
104+
assert.isTrue(isComboboxPopup(vNode));
105+
});
106+
107+
it('is false when owned by a select element', () => {
108+
const vNode = queryFixture(
109+
`<select role="combobox" aria-owns="target"></select>
110+
<div role="${role}" id="target"></div>`
111+
);
112+
assert.isFalse(isComboboxPopup(vNode));
113+
});
114+
115+
it('is false when not owned by a combobox', () => {
116+
const vNode = queryFixture(
117+
`<div role="button combobox" aria-owns="target"></div>
118+
<div role="${role}" id="target"></div>`
119+
);
120+
assert.isFalse(isComboboxPopup(vNode));
121+
});
122+
});
123+
});
124+
}
125+
126+
describe('options.popupRoles', () => {
127+
it('allows custom popup roles', () => {
128+
const vNode = queryFixture(
129+
`<div role="combobox" aria-controls="target"></div>
130+
<div role="button" id="target"></div>`
131+
);
132+
assert.isFalse(isComboboxPopup(vNode));
133+
assert.isTrue(isComboboxPopup(vNode, { popupRoles: ['button'] }));
134+
});
135+
136+
it('overrides the default popup roles', () => {
137+
const vNode = queryFixture(
138+
`<div role="combobox" aria-controls="target"></div>
139+
<div role="listbox" id="target"></div>`
140+
);
141+
assert.isTrue(isComboboxPopup(vNode));
142+
assert.isFalse(isComboboxPopup(vNode, { popupRoles: ['button'] }));
143+
});
144+
});
145+
});

test/integration/rules/aria-input-field-name/aria-input-field-name.html

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,21 @@
11
<!-- PASS -->
22
<!-- combobox -->
3-
<div id="pass1" aria-label="country" role="combobox">England</div>
3+
<div
4+
id="pass1"
5+
aria-label="country"
6+
role="combobox"
7+
aria-expanded="true"
8+
aria-controls="inapplicable1"
9+
>
10+
England
11+
</div>
12+
13+
<!-- Controlled by combobox: -->
14+
<ul role="listbox" id="inapplicable1">
15+
<li role="option">Zebra</li>
16+
<li role="option" id="selected_option">Zoom</li>
17+
</ul>
18+
419
<!-- listbox -->
520
<p id="pass2Label">Select a color:</p>
621
<div id="pass2" role="listbox" aria-labelledby="pass2Label">
@@ -87,13 +102,13 @@
87102
</label>
88103

89104
<!-- INAPPLICABLE -->
90-
<input id="inapplicable1" />
91-
<select id="inapplicable2">
105+
<input id="inapplicable2" />
106+
<select id="inapplicable3">
92107
<option value="volvo">Volvo</option>
93108
<option value="saab">Saab</option>
94109
<option value="opel">Opel</option>
95110
</select>
96-
<textarea id="inapplicable3" title="Label"></textarea>
111+
<textarea id="inapplicable4" title="Label"></textarea>
97112

98113
<!-- INCOMPLETE -->
99114
<!-- Implicit label -->

0 commit comments

Comments
 (0)