Skip to content

Commit 12393fe

Browse files
johanrdjohanrd
committed
fix: rename messageId + honest comment wording + literal-type check refactor (Copilot review)
- messageId `dynamicFalseTitle` renamed to `invalidTitleLiteral` — the rule flags booleans, null, undefined, AND numbers, not just `false`. - Message text generalized and now includes the literal type (boolean/null/ undefined/number) via a {{literalType}} interpolation. - Comment at rule top no longer claims `true` coerces to an empty-ish string (it doesn't — String(true) === "true"); reworded to explain that non-string literals don't describe frame content even when they would coerce. - Comment inside empty-string branch reworded — "not a string" was wrong. - isInvalidTitleLiteral() replaced with a Set-backed lookup (INVALID_LITERAL_TYPES) for cleaner membership checks. - Test assertions for the renamed messageId updated (unit + audit fixture); hbs-tester text assertions updated for the new message format.
1 parent a3b4437 commit 12393fe

3 files changed

Lines changed: 82 additions & 46 deletions

File tree

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

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,37 @@
1-
// Mustache path nodes that produce no accessible name. Booleans, null, undefined
2-
// all coerce to empty-ish strings; numeric literals ("42") are accepted by HTML
3-
// but provide no useful title for assistive tech.
4-
function isInvalidTitleLiteral(path) {
5-
if (!path) {
6-
return false;
7-
}
8-
if (path.type === 'GlimmerBooleanLiteral') {
9-
return true;
10-
}
11-
if (path.type === 'GlimmerNullLiteral' || path.type === 'GlimmerUndefinedLiteral') {
12-
return true;
13-
}
14-
if (path.type === 'GlimmerNumberLiteral') {
15-
return true;
1+
// Non-string literal AST nodes (boolean/null/undefined/number) don't represent
2+
// a meaningful author-provided title. Even though they would coerce to strings
3+
// at runtime (e.g. `true` → "true", `42` → "42"), those strings do not describe
4+
// the frame's content — the rule rejects the literal forms.
5+
const INVALID_LITERAL_TYPES = new Set([
6+
'GlimmerBooleanLiteral',
7+
'GlimmerNullLiteral',
8+
'GlimmerUndefinedLiteral',
9+
'GlimmerNumberLiteral',
10+
]);
11+
12+
function isInvalidTitleLiteralPath(path) {
13+
return INVALID_LITERAL_TYPES.has(path?.type);
14+
}
15+
16+
function getInvalidLiteralType(path) {
17+
if (!path) {return undefined;}
18+
switch (path.type) {
19+
case 'GlimmerBooleanLiteral': {
20+
return 'boolean';
21+
}
22+
case 'GlimmerNullLiteral': {
23+
return 'null';
24+
}
25+
case 'GlimmerUndefinedLiteral': {
26+
return 'undefined';
27+
}
28+
case 'GlimmerNumberLiteral': {
29+
return 'number';
30+
}
31+
default: {
32+
return undefined;
33+
}
1634
}
17-
return false;
1835
}
1936

2037
/** @type {import('eslint').Rule.RuleModule} */
@@ -39,11 +56,12 @@ module.exports = {
3956
},
4057
],
4158
messages: {
42-
// Four messageIds (missingTitle, emptyTitle, dynamicFalseTitle,
59+
// Four messageIds (missingTitle, emptyTitle, invalidTitleLiteral,
4360
// duplicateTitle) for richer diagnostic detail.
4461
missingTitle: '<iframe> elements must have a unique title property.',
4562
emptyTitle: '<iframe> elements must have a unique title property.',
46-
dynamicFalseTitle: '<iframe> elements must have a unique title property.',
63+
invalidTitleLiteral:
64+
'<iframe title> must be a non-empty string. Got a {{literalType}} literal, which does not describe the frame contents.',
4765
duplicateTitleFirst: 'This title is not unique. #{{index}}',
4866
duplicateTitleOther:
4967
'<iframe> elements must have a unique title property. Value title="{{title}}" already used for different iframe. #{{index}}',
@@ -97,9 +115,9 @@ module.exports = {
97115
const raw = titleAttr.value.chars;
98116
const value = raw.trim();
99117
if (value.length === 0) {
100-
// Distinguish genuine empty-string (always flagged — not a
101-
// string) from whitespace-only (spec-compliant but hygiene
102-
// check). Whitespace-only case respects the schema option.
118+
// Empty-string title always fails: no accessible name for screen readers.
119+
// Whitespace-only titles are controlled by the `allowWhitespaceOnlyTitle`
120+
// option (default false).
103121
if (raw.length === 0 || !allowWhitespaceOnlyTitle) {
104122
context.report({ node, messageId: 'emptyTitle' });
105123
}
@@ -138,8 +156,12 @@ module.exports = {
138156
case 'GlimmerMustacheStatement': {
139157
// title={{false}} / title={{null}} / title={{undefined}} / title={{42}}
140158
// — any literal that doesn't produce a meaningful accessible name.
141-
if (isInvalidTitleLiteral(titleAttr.value.path)) {
142-
context.report({ node, messageId: 'dynamicFalseTitle' });
159+
if (isInvalidTitleLiteralPath(titleAttr.value.path)) {
160+
context.report({
161+
node,
162+
messageId: 'invalidTitleLiteral',
163+
data: { literalType: getInvalidLiteralType(titleAttr.value.path) },
164+
});
143165
}
144166
break;
145167
}
@@ -150,9 +172,13 @@ module.exports = {
150172
if (
151173
parts.length === 1 &&
152174
parts[0].type === 'GlimmerMustacheStatement' &&
153-
isInvalidTitleLiteral(parts[0].path)
175+
isInvalidTitleLiteralPath(parts[0].path)
154176
) {
155-
context.report({ node, messageId: 'dynamicFalseTitle' });
177+
context.report({
178+
node,
179+
messageId: 'invalidTitleLiteral',
180+
data: { literalType: getInvalidLiteralType(parts[0].path) },
181+
});
156182
}
157183
break;
158184
}

‎tests/audit/iframe-title/peer-parity.js‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,43 +96,43 @@ ruleTester.run('audit:iframe-title (gts)', rule, {
9696
{
9797
code: '<template><iframe title={{false}} /></template>',
9898
output: null,
99-
errors: [{ messageId: 'dynamicFalseTitle' }],
99+
errors: [{ messageId: 'invalidTitleLiteral' }],
100100
},
101101
{
102102
code: '<template><iframe title={{true}} /></template>',
103103
output: null,
104-
errors: [{ messageId: 'dynamicFalseTitle' }],
104+
errors: [{ messageId: 'invalidTitleLiteral' }],
105105
},
106106
{
107107
code: '<template><iframe title={{null}} /></template>',
108108
output: null,
109-
errors: [{ messageId: 'dynamicFalseTitle' }],
109+
errors: [{ messageId: 'invalidTitleLiteral' }],
110110
},
111111
{
112112
code: '<template><iframe title={{undefined}} /></template>',
113113
output: null,
114-
errors: [{ messageId: 'dynamicFalseTitle' }],
114+
errors: [{ messageId: 'invalidTitleLiteral' }],
115115
},
116116
{
117117
code: '<template><iframe title={{42}} /></template>',
118118
output: null,
119-
errors: [{ messageId: 'dynamicFalseTitle' }],
119+
errors: [{ messageId: 'invalidTitleLiteral' }],
120120
},
121121
// Concat form — `title="{{false}}"` / etc. also flagged.
122122
{
123123
code: '<template><iframe title="{{false}}" /></template>',
124124
output: null,
125-
errors: [{ messageId: 'dynamicFalseTitle' }],
125+
errors: [{ messageId: 'invalidTitleLiteral' }],
126126
},
127127
{
128128
code: '<template><iframe title="{{null}}" /></template>',
129129
output: null,
130-
errors: [{ messageId: 'dynamicFalseTitle' }],
130+
errors: [{ messageId: 'invalidTitleLiteral' }],
131131
},
132132
{
133133
code: '<template><iframe title="{{42}}" /></template>',
134134
output: null,
135-
errors: [{ messageId: 'dynamicFalseTitle' }],
135+
errors: [{ messageId: 'invalidTitleLiteral' }],
136136
},
137137

138138
// === DIVERGENCE — duplicate-title detection ===
@@ -181,17 +181,17 @@ hbsRuleTester.run('audit:iframe-title (hbs)', rule, {
181181
{
182182
code: '<iframe title={{false}} />',
183183
output: null,
184-
errors: [{ messageId: 'dynamicFalseTitle' }],
184+
errors: [{ messageId: 'invalidTitleLiteral' }],
185185
},
186186
{
187187
code: '<iframe title={{undefined}} />',
188188
output: null,
189-
errors: [{ messageId: 'dynamicFalseTitle' }],
189+
errors: [{ messageId: 'invalidTitleLiteral' }],
190190
},
191191
{
192192
code: '<iframe title={{42}} />',
193193
output: null,
194-
errors: [{ messageId: 'dynamicFalseTitle' }],
194+
errors: [{ messageId: 'invalidTitleLiteral' }],
195195
},
196196
// DIVERGENCE — duplicate detection (upstream does not check).
197197
{

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,12 @@ ruleTester.run('template-require-iframe-title', rule, {
9797
{
9898
code: '<template><iframe src="12" title={{false}} /></template>',
9999
output: null,
100-
errors: [{ messageId: 'dynamicFalseTitle' }],
100+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'boolean' } }],
101101
},
102102
{
103103
code: '<template><iframe src="12" title="{{false}}" /></template>',
104104
output: null,
105-
errors: [{ messageId: 'dynamicFalseTitle' }],
105+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'boolean' } }],
106106
},
107107
{
108108
code: '<template><iframe src="12" title="" /></template>',
@@ -114,32 +114,32 @@ ruleTester.run('template-require-iframe-title', rule, {
114114
{
115115
code: '<template><iframe title={{null}} /></template>',
116116
output: null,
117-
errors: [{ messageId: 'dynamicFalseTitle' }],
117+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'null' } }],
118118
},
119119
{
120120
code: '<template><iframe title={{undefined}} /></template>',
121121
output: null,
122-
errors: [{ messageId: 'dynamicFalseTitle' }],
122+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'undefined' } }],
123123
},
124124
{
125125
code: '<template><iframe title={{42}} /></template>',
126126
output: null,
127-
errors: [{ messageId: 'dynamicFalseTitle' }],
127+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'number' } }],
128128
},
129129
{
130130
code: '<template><iframe title="{{null}}" /></template>',
131131
output: null,
132-
errors: [{ messageId: 'dynamicFalseTitle' }],
132+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'null' } }],
133133
},
134134
{
135135
code: '<template><iframe title="{{undefined}}" /></template>',
136136
output: null,
137-
errors: [{ messageId: 'dynamicFalseTitle' }],
137+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'undefined' } }],
138138
},
139139
{
140140
code: '<template><iframe title="{{42}}" /></template>',
141141
output: null,
142-
errors: [{ messageId: 'dynamicFalseTitle' }],
142+
errors: [{ messageId: 'invalidTitleLiteral', data: { literalType: 'number' } }],
143143
},
144144

145145
// Whitespace-only title is flagged by default (authoring hygiene).
@@ -229,12 +229,22 @@ hbsRuleTester.run('template-require-iframe-title', rule, {
229229
{
230230
code: '<iframe src="12" title={{false}} />',
231231
output: null,
232-
errors: [{ message: '<iframe> elements must have a unique title property.' }],
232+
errors: [
233+
{
234+
message:
235+
'<iframe title> must be a non-empty string. Got a boolean literal, which does not describe the frame contents.',
236+
},
237+
],
233238
},
234239
{
235240
code: '<iframe src="12" title="{{false}}" />',
236241
output: null,
237-
errors: [{ message: '<iframe> elements must have a unique title property.' }],
242+
errors: [
243+
{
244+
message:
245+
'<iframe title> must be a non-empty string. Got a boolean literal, which does not describe the frame contents.',
246+
},
247+
],
238248
},
239249
{
240250
code: '<iframe src="12" title="" />',

0 commit comments

Comments
 (0)