Skip to content

Commit 5dd3c75

Browse files
committed
fix(template-require-iframe-title): flag empty mustache string-literal titles
`<iframe title={{""}} />` and `<iframe title="{{""}}" />` previously slipped past the rule because the mustache/concat branches only flagged the named non-string literal types (boolean / null / undefined / number) and ignored GlimmerStringLiteral entirely. jsx-a11y's getLiteralPropValue resolves the empty-string literal and flags the case; this brings parity. Refactor the title-string handling (trim / empty / duplicate) into a small `processStaticTitle` helper, then call it from three places: - the existing GlimmerTextNode branch (no behavior change), - a new GlimmerStringLiteral sub-case in GlimmerMustacheStatement, - a new GlimmerStringLiteral sub-case in single-part GlimmerConcatStatement. The non-string-literal diagnostics (boolean/null/undefined/number → richer 'invalidTitleLiteral' message) are preserved unchanged. Added invalid regression tests for both the bare-mustache and concat-of-string-literal forms; replaced the previous valid pin of `title={{""}}` with a positive case (`title={{"My frame"}}`) to keep coverage of the resolution path.
1 parent 6b37a2b commit 5dd3c75

2 files changed

Lines changed: 80 additions & 50 deletions

File tree

lib/rules/template-require-iframe-title.js

Lines changed: 63 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,44 @@ module.exports = {
9494
const knownTitles = [];
9595
let nextDuplicateIndex = 1;
9696

97+
// Process a statically-known title string (from a text node OR a
98+
// mustache string literal OR a single-part concat). Handles the empty /
99+
// whitespace / duplicate logic that's shared across those AST shapes.
100+
function processStaticTitle(node, raw) {
101+
const value = raw.trim();
102+
if (value.length === 0) {
103+
// Empty-string title always fails: no accessible name for screen readers.
104+
// Whitespace-only titles are controlled by `allowWhitespaceOnlyTitle`.
105+
if (raw.length === 0 || !allowWhitespaceOnlyTitle) {
106+
context.report({ node, messageId: 'emptyTitle' });
107+
}
108+
return;
109+
}
110+
// Duplicate check — reports BOTH the first and the current occurrence
111+
// on every collision, sharing a `#N` index so users can correlate them.
112+
// For three or more duplicates the first occurrence is therefore
113+
// re-reported once per collision.
114+
const existing = knownTitles.find((entry) => entry.value === value);
115+
if (existing) {
116+
if (existing.index === null) {
117+
existing.index = nextDuplicateIndex++;
118+
}
119+
const index = existing.index;
120+
context.report({
121+
node: existing.node,
122+
messageId: 'duplicateTitleFirst',
123+
data: { index: String(index) },
124+
});
125+
context.report({
126+
node,
127+
messageId: 'duplicateTitleOther',
128+
data: { title: raw, index: String(index) },
129+
});
130+
} else {
131+
knownTitles.push({ value, node, index: null });
132+
}
133+
}
134+
97135
return {
98136
GlimmerElementNode(node) {
99137
if (node.tag !== 'iframe') {
@@ -117,63 +155,35 @@ module.exports = {
117155
if (titleAttr.value) {
118156
switch (titleAttr.value.type) {
119157
case 'GlimmerTextNode': {
120-
const raw = titleAttr.value.chars;
121-
const value = raw.trim();
122-
if (value.length === 0) {
123-
// Empty-string title always fails: no accessible name for screen readers.
124-
// Whitespace-only titles are controlled by the `allowWhitespaceOnlyTitle`
125-
// option (default false).
126-
if (raw.length === 0 || !allowWhitespaceOnlyTitle) {
127-
context.report({ node, messageId: 'emptyTitle' });
128-
}
129-
} else {
130-
// Check for duplicate titles. Reports BOTH the first and the
131-
// current occurrence on every collision, sharing a `#N` index
132-
// so users can correlate them. For three or more duplicates
133-
// the first occurrence is therefore re-reported once per
134-
// collision.
135-
const existing = knownTitles.find((entry) => entry.value === value);
136-
if (existing) {
137-
if (existing.index === null) {
138-
existing.index = nextDuplicateIndex++;
139-
}
140-
const index = existing.index;
141-
142-
// Report on the first occurrence on every collision.
143-
context.report({
144-
node: existing.node,
145-
messageId: 'duplicateTitleFirst',
146-
data: { index: String(index) },
147-
});
148-
149-
// Report on the current (duplicate) occurrence.
150-
context.report({
151-
node,
152-
messageId: 'duplicateTitleOther',
153-
data: { title: titleAttr.value.chars, index: String(index) },
154-
});
155-
} else {
156-
knownTitles.push({ value, node, index: null });
157-
}
158-
}
158+
processStaticTitle(node, titleAttr.value.chars);
159159
break;
160160
}
161161
case 'GlimmerMustacheStatement': {
162-
// title={{false}} / title={{null}} / title={{undefined}} / title={{42}}
163-
// — any literal that doesn't produce a meaningful accessible name.
162+
// Non-string literal mustaches — boolean / null / undefined /
163+
// number — get a specific "invalidTitleLiteral" diagnostic
164+
// because the literal coerces to a string at runtime that
165+
// doesn't describe the frame contents.
164166
if (isInvalidTitleLiteralPath(titleAttr.value.path)) {
165167
context.report({
166168
node,
167169
messageId: 'invalidTitleLiteral',
168170
data: { literalType: getInvalidLiteralType(titleAttr.value.path) },
169171
});
172+
break;
173+
}
174+
// String-literal mustaches resolve to their static value — a
175+
// non-empty literal supplies an accessible name the same as a
176+
// text node. Empty / whitespace literals are flagged the same
177+
// way as `title=""` / `title=" "`.
178+
if (titleAttr.value.path?.type === 'GlimmerStringLiteral') {
179+
processStaticTitle(node, titleAttr.value.path.value);
170180
}
171181
break;
172182
}
173183
case 'GlimmerConcatStatement': {
174-
// title="{{false}}" / "{{undefined}}" / etc. — ConcatStatement
175-
// with a single literal part that doesn't produce a name.
176184
const parts = titleAttr.value.parts || [];
185+
// Single-part concat wrapping a non-string literal — same
186+
// diagnostic as the bare mustache form.
177187
if (
178188
parts.length === 1 &&
179189
parts[0].type === 'GlimmerMustacheStatement' &&
@@ -184,6 +194,16 @@ module.exports = {
184194
messageId: 'invalidTitleLiteral',
185195
data: { literalType: getInvalidLiteralType(parts[0].path) },
186196
});
197+
break;
198+
}
199+
// Single-part concat wrapping a string literal — resolve to
200+
// the static value and apply the same checks as a text node.
201+
if (
202+
parts.length === 1 &&
203+
parts[0].type === 'GlimmerMustacheStatement' &&
204+
parts[0].path?.type === 'GlimmerStringLiteral'
205+
) {
206+
processStaticTitle(node, parts[0].path.value);
187207
}
188208
break;
189209
}

tests/lib/rules/template-require-iframe-title.js

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,9 @@ ruleTester.run('template-require-iframe-title', rule, {
1717
'<template><iframe title={{someValue}} /></template>',
1818
'<template><iframe title="" aria-hidden /></template>',
1919
'<template><iframe title="" hidden /></template>',
20-
// Documented divergence — `title={{""}}` is not flagged. jsx-a11y catches
21-
// empty-string literals via getLiteralPropValue; our rule inspects only
22-
// the named literal AST nodes (Boolean / Null / Undefined / Number) and
23-
// an empty-string mustache parses as a concat-of-nothing rather than a
24-
// GlimmerStringLiteral, so the under-flag is structural rather than
25-
// policy. Pinning the current behavior so any change is intentional.
26-
'<template><iframe title={{""}} /></template>',
20+
// Mustache string literals resolve to their static value — non-empty
21+
// literals supply an accessible name the same as a text node.
22+
'<template><iframe title={{"My frame"}} /></template>',
2723
'<template><iframe title="foo" /><iframe title="bar" /></template>',
2824
// allowWhitespaceOnlyTitle: true — whitespace-only accepted.
2925
{
@@ -42,6 +38,20 @@ ruleTester.run('template-require-iframe-title', rule, {
4238
output: null,
4339
errors: [{ messageId: 'emptyTitle' }],
4440
},
41+
// Empty string-literal mustaches and concat-with-empty-string-literal
42+
// resolve to "" via the static-attr-value handling and are flagged the
43+
// same as the text-node empty case. Closes a bypass jsx-a11y already
44+
// catches via getLiteralPropValue.
45+
{
46+
code: '<template><iframe title={{""}} /></template>',
47+
output: null,
48+
errors: [{ messageId: 'emptyTitle' }],
49+
},
50+
{
51+
code: '<template><iframe title="{{""}}" /></template>',
52+
output: null,
53+
errors: [{ messageId: 'emptyTitle' }],
54+
},
4555

4656
{
4757
// Both occurrences are reported with a shared `#N` index.

0 commit comments

Comments
 (0)