Skip to content

Commit 35a38d8

Browse files
namespaceMarcelloclaudechutchins25
authored
fix(aria-required-children): explain presentational children that cannot be ignored (#5407)
Makes the `aria-required-children` failure name the attribute that stops a `role="presentation"`/`role="none"` child from being ignored. As #4833 describes, this fails correctly, because `tabindex="-1"` means the presentational role is not applied: ```html <div role="grid"> <div role="presentation" tabindex="-1"> <div role="row"><div role="columnheader">foo</div></div> </div> </div> ``` but the message blamed the role instead of the attribute: ``` before: Element has children which are not allowed: [role=presentation] after: Element has presentational children which cannot be ignored because of [tabindex] ``` Changes in `aria-required-children-evaluate.js`: - `isPresentationalConflict()`: the owned element has an explicit `presentation`/`none` role, plus the global ARIA attribute or `tabindex` that `getOwnedRoles()` already records as the reason it was kept - when every unallowed child is such a conflict, the new `unallowedPresentational` message lists those attributes (`[tabindex]`, `[aria-live]`, …) - when they are mixed with other unallowed children, `unallowed` is kept and their selector includes the attribute: `[role=presentation][tabindex], [role=tabpanel]` - `aria-busy-fail` keeps priority, and a natively focusable child such as `<button role="presentation">` keeps the current message, also when it has `tabindex` or a global ARIA attribute: removing the attribute would not make its role apply `locales/_template.json` is regenerated in the same commit. Tests (`test/checks/aria/required-children.js`), asserting `messageKey` and `values` rather than the message text (#5357): - the issue's grid example: `unallowedPresentational`, `[tabindex]` - `role="none"` with `aria-live`: `unallowedPresentational`, `[aria-live]` - a presentational child with `tabindex` next to a `tabpanel`: `unallowed`, `[role=presentation][tabindex], [role=tabpanel]` - `<button role="presentation">`: `unallowed`, `[role=presentation]` - `<button role="presentation" tabindex="-1">` and `<button role="none" aria-label="x">`: `unallowed`, `[role=presentation]` / `[role=none]` Verified locally: the first three fail on `develop` (`[role=presentation]` / `[role=none]` with `unallowed`) and pass with the fix. Dropping the attribute guard in `isPresentationalConflict()` makes the fourth fail. Prettier, eslint, build (generated files committed), tsc, the unit, integration and virtual-rule suites, `test:act` and `test:apg` in Chrome pass. AI disclosure: this PR was written with Claude Code. Closes: issue #4833 _Written by Claude Code (AI assistant) on behalf of @namespaceMarcello, who directs this work._ 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Chris Hutchins <chris.hutchins@deque.com>
1 parent 2ba0812 commit 35a38d8

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

‎lib/checks/aria/aria-required-children-evaluate.js‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@ import { getGlobalAriaAttrs } from '../../commons/standards';
1010
import {
1111
hasContentVirtual,
1212
isFocusable,
13+
isNativelyFocusable,
1314
isVisibleToScreenReaders
1415
} from '../../commons/dom';
1516

17+
const PRESENTATIONAL_ROLES = ['presentation', 'none'];
18+
1619
/**
1720
* Check that an element owns all required children for its explicit role.
1821
*
@@ -46,15 +49,22 @@ export default function ariaRequiredChildrenEvaluate(
4649

4750
if (unallowed.length) {
4851
this.relatedNodes(unallowed.map(({ vNode }) => vNode));
49-
const messageKey =
50-
getAriaValue(virtualNode, 'aria-busy').value === 'true'
51-
? 'aria-busy-fail'
52-
: 'unallowed';
52+
let messageKey = 'unallowed';
53+
let selectors = unallowed.map(({ vNode, attr }) =>
54+
getUnallowedSelector(vNode, attr)
55+
);
56+
if (getAriaValue(virtualNode, 'aria-busy').value === 'true') {
57+
messageKey = 'aria-busy-fail';
58+
} else if (unallowed.every(isPresentationalConflict)) {
59+
// the children are only unallowed because an attribute stops their
60+
// presentational role from being applied
61+
messageKey = 'unallowedPresentational';
62+
selectors = unallowed.map(({ attr }) => `[${attr}]`);
63+
}
5364

5465
this.data({
5566
messageKey,
56-
values: unallowed
57-
.map(({ vNode, attr }) => getUnallowedSelector(vNode, attr))
67+
values: selectors
5868
.filter((selector, index, array) => array.indexOf(selector) === index)
5969
.join(', ')
6070
});
@@ -148,6 +158,23 @@ function getGlobalAriaAttr(vNode) {
148158
return getGlobalAriaAttrs().find(attr => vNode.hasAttr(attr));
149159
}
150160

161+
/**
162+
* Check if an owned element has a presentational role that is ignored because
163+
* of a global ARIA attribute or tabindex. A natively focusable element is not,
164+
* since removing the attribute would not make its role apply
165+
* @param {Object} ownedRole
166+
* @param {VirtualNode} ownedRole.vNode
167+
* @param {String} [ownedRole.attr] - Attribute which made the element unallowed
168+
* @return {Boolean}
169+
*/
170+
function isPresentationalConflict({ vNode, attr }) {
171+
return (
172+
!!attr &&
173+
!isNativelyFocusable(vNode) &&
174+
PRESENTATIONAL_ROLES.includes(getExplicitRole(vNode))
175+
);
176+
}
177+
151178
/**
152179
* Return a simple selector for an unallowed element.
153180
* @param {VirtualNode} vNode
@@ -161,6 +188,9 @@ function getUnallowedSelector(vNode, attr) {
161188
}
162189

163190
const role = getExplicitRole(vNode, { dpub: true });
191+
if (role && isPresentationalConflict({ vNode, attr })) {
192+
return `[role=${role}][${attr}]`;
193+
}
164194
if (role) {
165195
return `[role=${role}]`;
166196
}

‎lib/checks/aria/aria-required-children.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"singular": "Required ARIA child role not present: ${data.values}",
2929
"plural": "Required ARIA children role not present: ${data.values}",
3030
"unallowed": "Element has children which are not allowed: ${data.values}",
31+
"unallowedPresentational": "Element has presentational children which cannot be ignored because of ${data.values}",
3132
"aria-busy-fail": "Element has children which are not allowed: ${data.values}; Having aria-busy=\"true\" does not allow children with roles that are not allowed"
3233
},
3334
"incomplete": {

‎locales/_template.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@
536536
"singular": "Required ARIA child role not present: ${data.values}",
537537
"plural": "Required ARIA children role not present: ${data.values}",
538538
"unallowed": "Element has children which are not allowed: ${data.values}",
539+
"unallowedPresentational": "Element has presentational children which cannot be ignored because of ${data.values}",
539540
"aria-busy-fail": "Element has children which are not allowed: ${data.values}; Having aria-busy=\"true\" does not allow children with roles that are not allowed"
540541
},
541542
"incomplete": {

‎test/checks/aria/required-children.js‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,87 @@ describe('aria-required-children', () => {
203203
assert.deepEqual(checkContext._relatedNodes, [unallowed]);
204204
});
205205

206+
it('should explain why a presentational child with tabindex is not allowed', () => {
207+
const params = checkSetup(html`
208+
<div id="target" role="grid">
209+
<div role="presentation" tabindex="-1">
210+
<div role="row"><div role="columnheader">foo</div></div>
211+
</div>
212+
</div>
213+
`);
214+
assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
215+
216+
const unallowed = axe.utils.querySelectorAll(
217+
axe._tree,
218+
'[role="presentation"]'
219+
)[0];
220+
assert.deepEqual(checkContext._data, {
221+
messageKey: 'unallowedPresentational',
222+
values: '[tabindex]'
223+
});
224+
assert.deepEqual(checkContext._relatedNodes, [unallowed]);
225+
});
226+
227+
it('should explain why a presentational child with a global ARIA attribute is not allowed', () => {
228+
const params = checkSetup(
229+
'<div id="target" role="list"><div role="none" aria-live="polite"><div role="listitem">List item 1</div></div></div>'
230+
);
231+
assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
232+
assert.deepEqual(checkContext._data, {
233+
messageKey: 'unallowedPresentational',
234+
values: '[aria-live]'
235+
});
236+
});
237+
238+
it('should list presentational children with their attribute when other children are not allowed', () => {
239+
const params = checkSetup(html`
240+
<div id="target" role="list">
241+
<div role="presentation" tabindex="-1">
242+
<div role="listitem">List item 1</div>
243+
</div>
244+
<div role="tabpanel"></div>
245+
</div>
246+
`);
247+
assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
248+
assert.deepEqual(checkContext._data, {
249+
messageKey: 'unallowed',
250+
values: '[role=presentation][tabindex], [role=tabpanel]'
251+
});
252+
});
253+
254+
it('should not blame an attribute for a natively focusable presentational child', () => {
255+
const params = checkSetup(
256+
'<div id="target" role="list"><button role="presentation">Hello</button></div>'
257+
);
258+
assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
259+
assert.deepEqual(checkContext._data, {
260+
messageKey: 'unallowed',
261+
values: '[role=presentation]'
262+
});
263+
});
264+
265+
it('should not blame tabindex for a natively focusable presentational child', () => {
266+
const params = checkSetup(
267+
'<div id="target" role="list"><button role="presentation" tabindex="-1">Hello</button></div>'
268+
);
269+
assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
270+
assert.deepEqual(checkContext._data, {
271+
messageKey: 'unallowed',
272+
values: '[role=presentation]'
273+
});
274+
});
275+
276+
it('should not blame a global ARIA attribute for a natively focusable presentational child', () => {
277+
const params = checkSetup(
278+
'<div id="target" role="list"><button role="none" aria-label="x">Hello</button></div>'
279+
);
280+
assert.isFalse(requiredChildrenCheck.apply(checkContext, params));
281+
assert.deepEqual(checkContext._data, {
282+
messageKey: 'unallowed',
283+
values: '[role=none]'
284+
});
285+
});
286+
206287
it('should remove duplicate unallowed selectors', () => {
207288
const params = checkSetup(html`
208289
<div id="target" role="list">

0 commit comments

Comments
 (0)