Skip to content

Commit f85c914

Browse files
committed
fix(template-require-mandatory-role-attributes): use classifyAttribute (rows i2, h6, h9, h10)
Two bugs fixed; both stem from AST-based classification ignoring runtime rendering. Bug 1 — bare-mustache string-literal role ignored (i2 analog). Before: getStaticRolesFromElement only recognized role attributes whose value was a `GlimmerTextNode`. The form `role={{"option"}}` (which renders `role="option"` per the doc's i2-analog row — bare-mustache string literals never coerce) was treated as dynamic and silently skipped, so the rule never validated required ARIA attributes for it. After: getStaticRolesFromElement uses classifyAttribute. Static text, bare-string-literal mustache, and concat with all-literal parts (i1, i2, i3 analogs) all resolve to the same string and feed into role-token splitting. Bug 2 — bare-mustache falsy aria attribute counted as satisfying required-aria (h6, h9, h10). Before: required-ARIA satisfaction was checked by AST attribute name presence — `<div role="option" aria-selected={{false}} />` was treated as having aria-selected even though Glimmer OMITS the attribute at runtime per doc rows h6 (`{{false}}`), h9 (`{{null}}`), h10 (`{{undefined}}`). This was *also* encoded as a passing valid fixture, locking the bug in. After: the aria-attribute filter excludes attributes whose `classifyAttribute(attr).presence === 'absent'`. The previously-passing fixture moved from valid to invalid; replaced with positive cases that use static aria values (`aria-selected="true"` / `"false"`) and the new i2 role classification. Test coverage: - Valid (new): `<div role={{"option"}} aria-selected="true" />` - Invalid (new): `<div role="option" aria-selected={{false}} />` (h6) - Invalid (new): `<div role="option" aria-selected={{null}} />` (h9) - Invalid (new): `<div role={{"option"}} />` (i2 + missing aria) - All variants in both GJS and HBS suites. - Replaced the bug-encoding fixture (valid `<div role="option" aria-selected={{false}} />`) with `aria-selected="true"`/`"false"` static-value cases preserving the original "aria-selected satisfies role=option" intent. 106 tests pass. Doc rows cited: i1, i2, i3 (role classification); h6, h9, h10 (aria-* falsy-coercion).
1 parent de1838b commit f85c914

2 files changed

Lines changed: 79 additions & 6 deletions

File tree

lib/rules/template-require-mandatory-role-attributes.js

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,25 @@
1+
'use strict';
2+
13
const { roles } = require('aria-query');
24
const { AXObjectRoles, elementAXObjects } = require('axobject-query');
5+
const { classifyAttribute } = require('../utils/glimmer-attr-presence');
36

47
// ARIA role values are whitespace-separated tokens compared ASCII-case-insensitively.
58
// Returns the list of normalised tokens, or undefined when the attribute is
69
// missing or dynamic.
10+
//
11+
// `role` is a plain string attribute (no boolean coercion — see
12+
// docs/glimmer-attribute-behavior.md cross-attribute observations).
13+
// Recognise every source form that renders a statically-known role string:
14+
// - GlimmerTextNode (i1): `role="button"`
15+
// - bare-mustache string literal (i2 analog): `role={{"button"}}`
16+
// - concat with all-literal parts (i3 analog): `role="{{'button'}}"`
17+
// classifyAttribute resolves all three to the same string value.
718
function getStaticRolesFromElement(node) {
819
const roleAttr = node.attributes?.find((attr) => attr.name === 'role');
9-
10-
if (roleAttr?.value?.type === 'GlimmerTextNode') {
11-
return splitRoleTokens(roleAttr.value.chars);
20+
const { presence, value } = classifyAttribute(roleAttr);
21+
if (presence === 'present' && value !== null) {
22+
return splitRoleTokens(value);
1223
}
1324

1425
return undefined;
@@ -231,8 +242,19 @@ module.exports = {
231242
return;
232243
}
233244

245+
// Per docs/glimmer-attribute-behavior.md cross-attribute observations,
246+
// bare-mustache falsy literals on aria-* attributes (rows h6, h9, h10)
247+
// cause Glimmer to OMIT the attribute at runtime. AST-presence is not
248+
// a proxy for runtime-presence here: an element written as
249+
// <div role="option" aria-selected={{false}}> renders without any
250+
// aria-selected attribute and should NOT be treated as satisfying
251+
// role="option"'s required ARIA state.
234252
const foundAriaAttributes = (node.attributes ?? [])
235-
.filter((attribute) => attribute.name?.startsWith('aria-'))
253+
.filter(
254+
(attribute) =>
255+
attribute.name?.startsWith('aria-') &&
256+
classifyAttribute(attribute).presence !== 'absent'
257+
)
236258
.map((attribute) => attribute.name);
237259

238260
const result = getMissingRequiredAttributes(roleTokens, foundAriaAttributes, node, context);

tests/lib/rules/template-require-mandatory-role-attributes.js

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ ruleTester.run('template-require-mandatory-role-attributes', rule, {
1212
'<template><div aria-disabled="true" /></template>',
1313
'<template><div role="complementary" /></template>',
1414
'<template><div role="combobox" aria-expanded="false" aria-controls="ctrlId" /></template>',
15-
'<template><div role="option" aria-selected={{false}} /></template>',
15+
// Static aria-selected satisfies role="option"'s required ARIA. Both empty-
16+
// value and any literal value are recognized — what matters is that the
17+
// attribute is rendered at runtime.
18+
'<template><div role="option" aria-selected="true" /></template>',
19+
'<template><div role="option" aria-selected="false" /></template>',
20+
// Bare-mustache string-literal role (i2 analog) renders `role="option"`
21+
// and is now recognized as a static role token. The required aria-selected
22+
// is satisfied by its static text value.
23+
'<template><div role={{"option"}} aria-selected="true" /></template>',
1624
'<template><FakeComponent /></template>',
1725
'<template><FakeComponent role="fakerole" /></template>',
1826
'<template><CustomComponent role="checkbox" aria-checked="false" /></template>',
@@ -74,6 +82,29 @@ ruleTester.run('template-require-mandatory-role-attributes', rule, {
7482
output: null,
7583
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
7684
},
85+
// Bare-mustache falsy aria attribute (h6) — Glimmer omits the attribute at
86+
// runtime, so the required-ARIA check should NOT consider it satisfied.
87+
// Previously this case was silently treated as valid (encoded as a passing
88+
// fixture) because the rule counted AST attribute names, not runtime
89+
// presence. Per docs/glimmer-attribute-behavior.md cross-attribute
90+
// observations, aria-* is in the falsy-coercion list.
91+
{
92+
code: '<template><div role="option" aria-selected={{false}} /></template>',
93+
output: null,
94+
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
95+
},
96+
{
97+
code: '<template><div role="option" aria-selected={{null}} /></template>',
98+
output: null,
99+
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
100+
},
101+
// Bare-mustache string-literal role (i2 analog) — now recognized; missing
102+
// aria-selected is correctly flagged.
103+
{
104+
code: '<template><div role={{"option"}} /></template>',
105+
output: null,
106+
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
107+
},
77108
// Plain widget roles missing all required attrs — basic coverage that
78109
// peer plugins (jsx-a11y / vue-a11y / angular-eslint) also flag.
79110
{
@@ -246,7 +277,10 @@ hbsRuleTester.run('template-require-mandatory-role-attributes', rule, {
246277
'<div aria-disabled="true" />',
247278
'<div role="complementary" />',
248279
'<div role="combobox" aria-expanded="false" aria-controls="ctrlId" />',
249-
'<div role="option" aria-selected={{false}} />',
280+
'<div role="option" aria-selected="true" />',
281+
'<div role="option" aria-selected="false" />',
282+
// i2 analog: bare-mustache string-literal role renders `role="option"`.
283+
'<div role={{"option"}} aria-selected="true" />',
250284
'<FakeComponent />',
251285
'<FakeComponent role="fakerole" />',
252286
'<CustomComponent role="checkbox" aria-checked="false" />',
@@ -285,6 +319,23 @@ hbsRuleTester.run('template-require-mandatory-role-attributes', rule, {
285319
output: null,
286320
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
287321
},
322+
// Bare-mustache falsy aria attribute (h6, h9) — Glimmer omits at runtime.
323+
{
324+
code: '<div role="option" aria-selected={{false}} />',
325+
output: null,
326+
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
327+
},
328+
{
329+
code: '<div role="option" aria-selected={{null}} />',
330+
output: null,
331+
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
332+
},
333+
// Bare-mustache string-literal role (i2 analog) — now recognized.
334+
{
335+
code: '<div role={{"option"}} />',
336+
output: null,
337+
errors: [{ message: 'The attribute aria-selected is required by the role option' }],
338+
},
288339
{
289340
code: '<CustomComponent role="checkbox" aria-required="true" />',
290341
output: null,

0 commit comments

Comments
 (0)