forked from ember-cli/eslint-plugin-ember
-
Notifications
You must be signed in to change notification settings - Fork 0
BUGFIX: template-require-valid-alt-text — reject empty-string aria-label/labelledby/alt on <input type=image>, <object>, <area> #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+404
−59
Closed
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
2337221
fix(template-require-valid-alt-text): reject empty-string aria-label/…
johanrd 7488d8d
chore: drop temporal 'previously accepted' comment
johanrd 56567ef
test: add Phase 3 audit fixture translating alt-text peer cases
johanrd 98e022d
fix+docs+test: trim whitespace, tighten JSDoc, add whitespace-only co…
johanrd 82022bb
docs: correct audit-fixture CI-run claim (Copilot review)
johanrd bc64218
fix(template-require-valid-alt-text): address Copilot review — JSDoc …
johanrd e7c6a6a
fix(#56): address round-2 Copilot review (drop unused hasAnyAttr helper)
johanrd 2abfbea
chore(alt-text/peer-parity): drop reference to non-existent docs/audi…
johanrd 67de7c7
fix(template-require-valid-alt-text): unwrap mustache literals via sh…
johanrd 6101f28
test(template-require-valid-alt-text): absorb audit-fixture cases, dr…
johanrd 19e44c4
BUGFIX: template-require-iframe-title — flag invalid title literals +…
johanrd 6cc136f
docs(template-require-valid-alt-text): add WCAG SC 4.1.2 citation to …
johanrd 30b3fe9
chore(deps): update wyvox/action-setup-pnpm action to v4 (#2742)
renovate[bot] 634af79
refactor(template-require-iframe-title): remove allowWhitespaceOnlyTi…
johanrd e170971
Merge pull request #2731 from johanrd/fix/iframe-title-value-checks
NullVoxPopuli 5c900c3
Merge branch 'master' into fix/alt-text-empty-aria-label
johanrd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| 'use strict'; | ||
|
|
||
| /** | ||
| * Return the statically-known string value of a Glimmer attribute value node, | ||
| * or `undefined` when the value is dynamic (cannot be resolved at lint time). | ||
| * | ||
| * Unwraps: | ||
| * - GlimmerTextNode → chars | ||
| * - GlimmerMustacheStatement with a literal path (boolean/string/number) → stringified value | ||
| * - GlimmerConcatStatement whose parts are all statically resolvable → joined string | ||
| * | ||
| * A missing/undefined value (valueless attribute, e.g. `<input disabled>`) | ||
| * returns the empty string. Pass `attr.value` — not the attribute itself. | ||
| */ | ||
| function getStaticAttrValue(value) { | ||
| if (value === null || value === undefined) { | ||
| return ''; | ||
| } | ||
| if (value.type === 'GlimmerTextNode') { | ||
| return value.chars; | ||
| } | ||
| if (value.type === 'GlimmerMustacheStatement') { | ||
| return extractLiteral(value.path); | ||
| } | ||
| if (value.type === 'GlimmerConcatStatement') { | ||
| const parts = value.parts || []; | ||
| let out = ''; | ||
| for (const part of parts) { | ||
| if (part.type === 'GlimmerTextNode') { | ||
| out += part.chars; | ||
| continue; | ||
| } | ||
| if (part.type === 'GlimmerMustacheStatement') { | ||
| const literal = extractLiteral(part.path); | ||
| if (literal === undefined) { | ||
| return undefined; | ||
| } | ||
| out += literal; | ||
| continue; | ||
| } | ||
| return undefined; | ||
| } | ||
| return out; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| function extractLiteral(path) { | ||
| if (!path) { | ||
| return undefined; | ||
| } | ||
| if (path.type === 'GlimmerBooleanLiteral') { | ||
| return path.value ? 'true' : 'false'; | ||
| } | ||
| if (path.type === 'GlimmerStringLiteral') { | ||
| return path.value; | ||
| } | ||
| if (path.type === 'GlimmerNumberLiteral') { | ||
| return String(path.value); | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| module.exports = { getStaticAttrValue }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| // Audit fixture — translates peer-plugin test cases into assertions against | ||
| // our rule (`ember/template-require-valid-alt-text`). Runs as part of the | ||
| // default Vitest suite (via the `tests/**/*.js` include glob) and serves | ||
| // double-duty: (1) auditable record of peer-parity divergences, | ||
|
johanrd marked this conversation as resolved.
Outdated
|
||
| // (2) regression coverage pinning CURRENT behavior. Each case encodes what | ||
| // OUR rule does today; divergences from upstream plugins are annotated as | ||
| // `DIVERGENCE —`. Peer-only constructs that can't be translated to Ember | ||
| // templates (JSX spread props, Vue v-bind, Angular `$event`, undefined-handler | ||
| // expression analysis) are marked `AUDIT-SKIP`. Divergences are also captured | ||
| // in PR descriptions / commit messages; grepping the repo for `DIVERGENCE —` | ||
| // surfaces the full list. | ||
| // | ||
| // Source files: | ||
| // - context/eslint-plugin-jsx-a11y-main/__tests__/src/rules/alt-text-test.js | ||
| // - context/eslint-plugin-vuejs-accessibility-main/src/rules/__tests__/alt-text.test.ts | ||
| // - context/eslint-plugin-lit-a11y/tests/lib/rules/alt-text.js | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const rule = require('../../../lib/rules/template-require-valid-alt-text'); | ||
| const RuleTester = require('eslint').RuleTester; | ||
|
|
||
| const ruleTester = new RuleTester({ | ||
| parser: require.resolve('ember-eslint-parser'), | ||
| parserOptions: { ecmaVersion: 2022, sourceType: 'module' }, | ||
| }); | ||
|
|
||
| ruleTester.run('audit:alt-text (gts)', rule, { | ||
| valid: [ | ||
| // === Upstream parity (valid in jsx-a11y + ours) === | ||
| '<template><img alt="foo" /></template>', | ||
| '<template><img alt="" /></template>', | ||
| '<template><img alt=" " /></template>', | ||
| '<template><img alt="" role="presentation" /></template>', | ||
| '<template><img alt="" role="none" /></template>', | ||
| // DIVERGENCE — moved to invalid below: | ||
| // '<template><img alt="this is lit..." role="presentation" /></template>', | ||
| '<template><img alt={{@dynamicAlt}} /></template>', | ||
| // object with label/children | ||
| '<template><object aria-label="foo" /></template>', | ||
| '<template><object aria-labelledby="id1" /></template>', | ||
| '<template><object>Foo</object></template>', | ||
| '<template><object title="An object" /></template>', | ||
| // area with label | ||
| '<template><area aria-label="foo" /></template>', | ||
| '<template><area aria-labelledby="id1" /></template>', | ||
| '<template><area alt="foo" /></template>', | ||
| // input[type=image] | ||
| '<template><input type="image" alt="foo" /></template>', | ||
| '<template><input type="image" aria-label="foo" /></template>', | ||
| '<template><input type="image" aria-labelledby="id1" /></template>', | ||
|
|
||
| // === DIVERGENCE — aria-label/aria-labelledby on <img> without alt === | ||
| // jsx-a11y: VALID — `<img aria-label="foo" />` is accepted. | ||
| // vue-a11y: VALID — same. | ||
| // Our rule: INVALID — requires `alt` attribute on <img>, full stop. | ||
| // Spec reading: the HTML spec mandates alt on <img>. WAI-ARIA accepts | ||
| // aria-label/aria-labelledby as alternative accessible-name sources. The | ||
| // two specs disagree; we side with HTML-strict. | ||
| // No valid test here — we flag; see invalid section. | ||
|
|
||
| // === Edge cases we handle === | ||
| // alt === src (we flag) | ||
| // numeric alt (we flag) | ||
| // redundant words (we flag) | ||
| ], | ||
| invalid: [ | ||
| // === Upstream parity (invalid in jsx-a11y + ours) === | ||
| { | ||
| code: '<template><img /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'imgMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><input type="image" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'inputImage' }], | ||
| }, | ||
| { | ||
| code: '<template><object /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'objectMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><area /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'areaMissing' }], | ||
| }, | ||
|
|
||
| // === DIVERGENCE — <img aria-label> without alt === | ||
| // jsx-a11y: VALID. Ours: INVALID (imgMissing). | ||
| // Behavior captured here; potential false positive per WAI-ARIA. | ||
| { | ||
| code: '<template><img aria-label="foo" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'imgMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><img aria-labelledby="id1" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'imgMissing' }], | ||
| }, | ||
|
|
||
| // === DIVERGENCE — non-empty alt with role=presentation on img === | ||
| // jsx-a11y: VALID — accepts `<img alt="this is lit..." role="presentation" />`. | ||
| // Ours: INVALID — imgRolePresentation. We're spec-strict: if role is | ||
| // "none"/"presentation", the image is decorative and alt should be empty. | ||
| { | ||
| code: '<template><img alt="this is lit..." role="presentation" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'imgRolePresentation' }], | ||
| }, | ||
|
|
||
| // === Parity — empty-string aria-label/aria-labelledby === | ||
| // jsx-a11y / vuejs-accessibility flag empty-string fallbacks on the | ||
| // "accessible-name-required" elements (<object>, <area>, <input | ||
| // type=image>). Our rule now reuses the existing messageIds. | ||
| { | ||
| code: '<template><object aria-label="" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'objectMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><object aria-labelledby="" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'objectMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><area aria-label="" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'areaMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><area aria-labelledby="" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'areaMissing' }], | ||
| }, | ||
| { | ||
| code: '<template><input type="image" aria-label="" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'inputImage' }], | ||
| }, | ||
| { | ||
| code: '<template><input type="image" aria-labelledby="" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'inputImage' }], | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const hbsRuleTester = new RuleTester({ | ||
| parser: require.resolve('ember-eslint-parser/hbs'), | ||
| parserOptions: { ecmaVersion: 2022, sourceType: 'module' }, | ||
| }); | ||
|
|
||
| hbsRuleTester.run('audit:alt-text (hbs)', rule, { | ||
| valid: [ | ||
| '<img alt="foo" />', | ||
| '<img alt="" />', | ||
| '<img alt="" role="presentation" />', | ||
| '<object aria-label="foo" />', | ||
| '<area aria-label="foo" />', | ||
| '<input type="image" aria-label="foo" />', | ||
| ], | ||
| invalid: [ | ||
| { | ||
| code: '<img />', | ||
| output: null, | ||
| errors: [{ messageId: 'imgMissing' }], | ||
| }, | ||
| { | ||
| code: '<input type="image" />', | ||
| output: null, | ||
| errors: [{ messageId: 'inputImage' }], | ||
| }, | ||
| { | ||
| code: '<object />', | ||
| output: null, | ||
| errors: [{ messageId: 'objectMissing' }], | ||
| }, | ||
| { | ||
| code: '<area />', | ||
| output: null, | ||
| errors: [{ messageId: 'areaMissing' }], | ||
| }, | ||
| // DIVERGENCE captured — we flag img-with-aria-label (jsx-a11y/vue-a11y don't) | ||
| { | ||
| code: '<img aria-label="foo" />', | ||
| output: null, | ||
| errors: [{ messageId: 'imgMissing' }], | ||
| }, | ||
| // Parity — empty-string label on accessible-name-required elements. | ||
| { | ||
| code: '<object aria-label="" />', | ||
| output: null, | ||
| errors: [{ messageId: 'objectMissing' }], | ||
| }, | ||
| ], | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.