Inject placeholder `src`/`alt` on `<img ...attributes>` by johanrd · Pull Request #13 · johanrd/html-validate-ember

johanrd · 2026-05-07T10:50:50Z

Summary

When a thin wrapper component renders <img ...attributes> and the consumer supplies src/alt via the splat, the blanker erased ...attributes and html-validate saw an attribute-less <img>, FP-firing element-required-attributes (src) and wcag/h37 (alt).

This adds tryInjectImgRequiredAttrs (symmetric to the existing tryInjectInputType for <input>): when <img> has ...attributes, inject whitespace-valued src=' ' and alt=' ' placeholders into Glimmer-only blank regions in the open tag. processAttribute's DynamicValue conversion (>=3-char whitespace threshold) then surfaces them as "present, value unknowable". Skipped when the consumer already specified src=/alt=.

Slot-space trade-off

The minimal case <img ...attributes> has only one 13-char Glimmer-only slot (the splat), which fits one 9-char placeholder — only src gets injected. Real-world <img> invocations usually have additional Glimmer-only attrs/modifiers (@arg, {{on "load" …}}, etc.) providing enough total space for both. Tests cover both cases.

Currently scoped to <img>. Same shape applies to <source>/<track>/<area>/<iframe> if real-world FPs surface there.

Surfaced by

Ecosystem CI on ember-learn/super-rentals (<img ...attributes> in app/components/rental/image.gjs).

Test plan

New test: minimal single-slot case asserts src placeholder is injected
New test: multi-slot case asserts both src and alt are injected
npm test — 129/129 passing
npm run typecheck:tests — clean
npm run build — clean

@arg

When a thin wrapper component renders `<img ...attributes>` and the consumer supplies src/alt via the splat, the blanker erased `...attributes` and html-validate then saw an attribute-less <img>, FP-firing `element-required-attributes` (src) and `wcag/h37` (alt). At runtime the splat provides them. Symmetric to the existing `tryInjectInputType` (blank.ts:670) for `<input type=' '>`: when `<img>` has `...attributes`, inject whitespace-valued `src=' '` and `alt=' '` placeholders into Glimmer-only blank regions in the open tag. processAttribute's DynamicValue conversion (length>=3 + all-whitespace) then surfaces them as "present, value unknowable". Trade-off on slot space: minimal `<img ...attributes>` has only one 13-char Glimmer-only slot (the splat) which fits one 9-char placeholder. Real-world usually has additional Glimmer-only attrs/modifiers (an @arg, a {{on …}}) that provide enough room for both src and alt. Tests cover both cases. Surfaced by ecosystem CI on super-rentals (rental/image.gjs).

Copilot

Pull request overview

Adds placeholder attribute injection for <img ...attributes> so html-validate does not falsely report missing required attributes (src/alt) when those are expected to be supplied via splattributes at runtime.

Changes:

Implement tryInjectImgRequiredAttrs to inject whitespace placeholders for src/alt into Glimmer-only blank regions when <img> has ...attributes.
Call the injector during native element handling for img, before Glimmer-only regions are blanked.
Add unit tests covering the minimal single-slot case (only src fits) and a multi-slot case (both src and alt fit).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`blank.ts`	Adds `<img>` placeholder injection logic and wires it into the element handling flow.
`test/blank.test.ts`	Adds tests asserting placeholder injection behavior for `<img ...attributes>` scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Per Copilot review on #13: the comment claimed parity with tryInjectComponentAttrs's anti-starvation strategy, but the code only sorted the candidate slots, not the wanted attrs. Adjusted the comment to describe what the code actually does — slot-side widest-first. The wanted-side sort isn't needed because src/alt placeholders are both 9 chars, so neither attr can starve the other.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Per Copilot review on #13: the no-injection-when-already-present behavior was implemented but only positive cases were tested. Added a test asserting that <img src="..." alt="..." ...attributes> keeps the consumer's explicit values and produces no injected placeholders. Guards against a regression that would emit duplicate attributes.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

… threshold Per Copilot review on #13: \\s+ would pass with 1-2 spaces, but processAttribute requires >= 3 whitespace chars to convert to DynamicValue. Tightened all five toMatch/.not.toMatch checks for src/alt placeholders to \\s{3,} so a regression to a shorter placeholder fails the test instead of silently breaking the DynamicValue conversion.

@x

Per Copilot review on #15: 1. Updated lib/dynamic-value.ts header comment to drop the reference to tryInjectImgRequiredAttrs — that helper lives on a sibling branch (#13), not this one. The list now generalizes ("multiple injection sites") rather than naming each function. 2. Boolean attrs (`disabled`, `required`, `selected`, `checked`, …) now emit presence-only in BOTH tryInjectComponentAttrs and substituteSelfClosingComponent regardless of the recorded value (literal `''`, the DynamicValue placeholder, or any literal-safe string). Per HTML5 any value on a boolean attr is equivalent to "true"; emitting `disabled=' '` (or any explicit-value form) unnecessarily triggers `attribute-boolean-style`. - tryInjectComponentAttrs: changed the gate from `literal === '' && isBooleanAttr(...)` to just `isBooleanAttr(...)`. This now covers the new arg-bound case (`disabled={{@x}}` recorded as the placeholder) which used to fall through to literal-emit. - substituteSelfClosingComponent: added the same isBooleanAttr branch when emitting splatted-root attrs into the rewritten `<RESOLVED ...></RESOLVED>` open tag. New regression test in blank.test.ts covers the placeholder boolean case explicitly.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

The previous source-rewrite approach in `tryInjectImgRequiredAttrs` could only inject ONE 9-char `attr=' '` placeholder per Glimmer-only slot. For the minimal pattern `<img ...attributes>` (only one slot, 13 chars wide) that meant `src=' '` was injected but `alt` was not — and wcag/h37 FP-fired on the missing alt. Surfaced by ecosystem CI on super-rentals' `rental/image.gjs`. No two-attr form fits in 13 chars that html-validate accepts: - bare `src alt` triggers attribute-allowed-values-missing-value - empty quoted `src=''` triggers attribute-allowed-values-invalid - the wide whitespace form `src=' ' alt=' '` is 19+ chars Switched to hook-time injection. The blanker now records each `<img ...attributes>` element's start offset (when src or alt is not consumer-written) on a new `BlankResult.imgSplatOffsets` field. The transformer's `processElement` hook reads that set and calls `setAttribute('src'/'alt', new DynamicValue(''), location, location)` on the parsed element, sidestepping source-side slot widths entirely. Per-attr presence is checked in the hook so a partially consumer-written invocation (e.g. only `src=` written) still synthesizes the missing one. New regression fixture `examples/img-splat-thin-wrapper.gjs` mirrors the super-rentals pattern; integration test asserts neither wcag/h37 nor element-required-attributes fires. Existing unit tests rewritten to assert the offset-recording behavior (instead of the old source-rewrite output). 132/132 pass.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Re-merged combined-fp-fixes into ecosystem-ci with all latest fix tips (PR #13 narrow-slot, PR #15 stale-comment cleanup, PR #17 Thread B + fieldset-with-component, PR #19 native-tag guard, cache-invalidation-on-source-change). Cleared all target caches before re-baseline so cache-stale findings (e.g., the abbr-FP from power-select pre-PR-#12) re-resolve correctly. Per-target deltas: super-rentals -2 errors (img wcag/h37, form wcag/h32 cleared by Thread A + Thread B) ember-primitives -1 error (2 wcag/h71 cleared by fieldset-with- component-content; +1 no-implicit-button-type newly visible on tabs.gts:247 — TabButton now resolves to <button>) ember-power-select -1 error -1 warning (abbr-FP cleared by cache fix re-resolving via PR #12; one trigger.gts prefer-native warning gone for the same reason) hds-design-system -10 errors (much shuffling: -39 element- required-attributes from img-splat fix, -19 element-permitted-content, +31 aria-label-misuse newly visible, +6 no-implicit-button-type) limber +1 error (Editor iframe missing title — newly visible after better resolution; needs investigation as potential plugin gap) Total errors across all targets: −13.

@src

When the classic-by-name resolver substitutes `<MyImg>` to `<img>` because the addon's template has `<img src={{this.src}} ...attributes />`, the consumer's narrow Glimmer-attr blank slots (`@src="…"`) can't fit the projected `src=' '` placeholder via tryInjectComponentAttrs's source-side rewrite, and `element-required-attributes` FP-fires. Mirror PR #13's narrow-slot fix: when resolved=='img' and the addon's attrCtx records `src`/`alt` (literal OR mustache-bound), push the consumer's offset to `imgSplatOffsets`. The processElement hook then calls setAttribute at parse time with a DynamicValue, sidestepping source-width entirely. Caught by ecosystem CI on PR #21 baseline diff: ember-website's `<ResponsiveImage @src="…" alt="" />` started FP-firing because the new by-name resolver mapped it to `<img>` but didn't carry the mustache-bound `src` from the addon's template through to the consumer-side substitution.

johanrd added the bug Something isn't working label May 7, 2026

johanrd requested a review from Copilot May 7, 2026 11:05

Copilot started reviewing on behalf of johanrd May 7, 2026 11:05 View session