Skip to content

feat(utils): add glimmer-attr-presence + verified-behavior reference doc#2769

Draft
johanrd wants to merge 12 commits intoember-cli:masterfrom
johanrd:docs/glimmer-attribute-behavior
Draft

feat(utils): add glimmer-attr-presence + verified-behavior reference doc#2769
johanrd wants to merge 12 commits intoember-cli:masterfrom
johanrd:docs/glimmer-attribute-behavior

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 28, 2026

Summary

Adds docs/glimmer-attribute-behavior.md — an empirically-verified reference for how Glimmer renders HTML attributes with mustache values. Aimed at rule authors writing or modifying rules that classify attribute literals via GlimmerBooleanLiteral, GlimmerStringLiteral, GlimmerConcatStatement, or GlimmerTextNode. A short pointer is added to README's "Creating a New Rule" section.

Why

Several common mental models for Glimmer attribute rendering are wrong in subtle, attribute-specific ways. Two examples that have caused real bugs in this plugin's rules:

  1. attr={{"false"}} (bare-mustache string "false") is JS-truthy, so it renders as attr="false" (kept) — not falsy as the literal suggests.
  2. attr="{{false}}" (concat-mustache with boolean literal false) sets the IDL property to true regardless of the literal value inside. Verified against <video muted="{{false}}">videoEl.muted === true, even though outerHTML shows no muted attribute.

Without a written reference, rule authors (myself included) keep re-deriving the wrong model. This doc captures what's been empirically tested and includes a reproduction template so future readers can re-verify if Glimmer behavior changes.

Contents

  • TL;DR lint-truth table (one-shot answer for "is the attribute present at runtime?")
  • HTML serialization vs IDL property (they can disagree — IDL is what matters for accessibility rules)
  • Verified rendering tables, split by bare-mustache and concat-mustache
  • Per-attribute notes (reflecting boolean attrs, non-reflecting media boolean attrs, ARIA/string attrs, numeric attrs)
  • Reproduction template — paste into any Ember dev app, inspect rendered HTML and IDL properties
  • Open questions / unverified rows, marked explicitly

The Verification metadata section has a _TODO_ for the exact ember-source version of the dev app where t1–t4 were verified. I'll fill that in once I confirm with the testing environment.

johanrd added 6 commits April 28, 2026 10:33
Captures empirically-verified Glimmer rendering behavior for HTML
attributes with mustache values, so rule authors classifying
GlimmerBooleanLiteral / GlimmerStringLiteral / GlimmerConcatStatement
have a ground-truth reference instead of intuition.

Notable findings the doc pins down:
- attr={{"false"}} (bare string "false") renders as attr="false" — TRUTHY,
  not falsy as the literal suggests.
- attr="{{false}}" (concat) sets the IDL property to true regardless of
  the literal value inside, even when HTML serialization shows nothing.
  Verified against <video muted="{{false}}"> → videoEl.muted === true.
- Non-reflecting boolean attrs (muted, autoplay) and reflecting ones
  (disabled, hidden) diverge in HTML serialization but agree at the IDL
  property layer.

Includes a copy-pasteable reproduction template so future readers can
re-verify if Glimmer behavior changes.

Adds a pointer in README's "Creating a New Rule" section.
Replaces the prior tables (which mixed verified data with extrapolations
marked "(assumed)") with strictly-verified per-attribute tables. Every cell
populated from rendering and IDL inspection in ember-source 6.12.

Structure:
- One "Reference table" section, five per-attribute sub-tables
  (muted, disabled, aria-hidden, tabindex, autocomplete)
- One "To reproduce the reference table" section with the exact template
  and JS console snippet, inline (no separate fixture file)
- Cross-attribute observations summarizing the rules the data reveals

Findings the new tables make explicit:
- Bare-mustache falsy set is {{false}}/{{null}}/{{undefined}}/{{0}} for
  boolean-coerced attrs (boolean HTML, ARIA, numeric); {{""}} is kept as
  attr="".
- Bare-mustache string literals never coerce — attr={{"false"}} renders
  as attr="false".
- Concat-mustache for boolean HTML attrs sets the IDL property to true
  regardless of the literal value inside (verified for both reflecting
  and non-reflecting attrs).
- Concat-mustache for ARIA/string attrs renders the stringified value
  literally — no boolean coercion. aria-hidden="{{false}}" is visible.
- Plain string attrs (autocomplete) skip Glimmer's boolean coercion
  entirely; autocomplete={{false}} renders as autocomplete="false".

The video.muted snapshot reads IDL muted=false for static attribute forms
(m1-m4, m7-m8, m11) because the test runs before media load — the doc
explains how defaultMuted reflects to muted at load time, so the rule's
"At play time" column is the lint-truth column rule authors should use.
…les" guide

Adds a practical-implementation section between the reference table and
the reproduction template. It maps each AST shape (GlimmerTextNode /
GlimmerMustacheStatement with each path type / GlimmerConcatStatement)
to a verdict, citing the row IDs from the reference table so rule
authors can implement classification correctly without re-deriving the
model.

Includes:
- AST-shape verdict table — direct mapping rule authors can copy from
- Six common mistakes section, each tied to specific row IDs
- Pointer to the (forthcoming) lib/utils/glimmer-attr-presence.js
  utility that will encode the verdict table once and let rules consume
  the resolved kind + value rather than re-walking the AST

The audit of master rules and the open feature PRs found 18 REAL_BUG
findings (12 in PRs, 6 in master) — all classifiable into the bullet-1
through bullet-4 footguns this guide enumerates.
…ng model

Adds lib/utils/glimmer-attr-presence.js exporting:

- classifyAttribute(attrNode, options?) → { presence, value }
  Maps every AST shape (valueless / GlimmerTextNode / GlimmerMustacheStatement
  with each path type / GlimmerConcatStatement) to a (presence, value) pair
  per the verified model in docs/glimmer-attribute-behavior.md. Each branch
  cites the relevant doc row IDs (m1–m19, h1–h15, d1–d10, t1–t7, i1–i5).
- inferAttrKind(name) → 'boolean' | 'aria' | 'numeric' | 'plain-string'
  Used when classifyAttribute callers don't pass options.kind explicitly.
- BOOLEAN_HTML_ATTRS, NUMERIC_ATTRS — exported sets, useful for callers
  that want to extend the kind model.

Key empirical asymmetries this util encodes correctly (and that audit
findings show several rules currently misclassify):

- Bare {{false}} / {{null}} / {{undefined}} on falsy-coerced kinds
  (boolean / aria / numeric) → presence='absent' (Glimmer omits attribute).
  Same forms on plain-string → presence='present', value='false' / etc.
- Bare {{"false"}} (StringLiteral) is JS-truthy, never coerced — renders
  the literal value across all attribute kinds.
- aria-hidden={{true}} renders aria-hidden="" (h5, contested), not
  aria-hidden="true" — the util surfaces value='' here so callers
  comparing value === 'true' don't false-match.
- Concat is never falsy: any concat form is presence='present'; the
  resolved value comes from the existing getStaticAttrValue helper.

Tests: 35 unit tests covering every doc row + the kind-override option.

Updates docs/glimmer-attribute-behavior.md to reference the actual file
and replaces the "(forthcoming)" sketch with a working example.
@johanrd johanrd changed the title docs: add glimmer-attribute-behavior reference for rule authors feat(utils): add glimmer-attr-presence + verified-behavior reference doc Apr 28, 2026
…ation / html-void-elements

Correctness fixes from PR ember-cli#2769 review:

- Boolean concat now returns canonical `value: 'true'` instead of leaking
  the inner literal. Per doc rows m13-m19, d7-d10 the IDL is set true
  regardless of inner value, so callers checking `value === 'false'` to
  detect "off" no longer get a wrong answer for `<video muted="{{false}}">`.
- {{true}} on numeric / plain-string now returns `unknown` (untested in
  doc; was previously claiming `value: 'true'` by extrapolation).
- `inferAttrKind` is now case-insensitive (`Disabled`, `ARIA-Hidden`, etc.).

Drop hand-rolled spec lists in favor of upstream packages:

- `property-information` for boolean / overloadedBoolean / number attribute
  classification, replacing the 24-entry BOOLEAN_HTML_ATTRS and 3-entry
  NUMERIC_ATTRS Sets. `colspan` is added as a small NUMERIC_OVERRIDES Set
  to compensate for an upstream gap in property-information 7.1.0 (rowspan
  and cols are marked `number: true`, colspan isn't).
- `html-void-elements` in template-block-indentation.js and
  template-self-closing-void-elements.js, deduplicating two parallel
  16-entry VOID_TAGS Sets.

Internal API change: BOOLEAN_HTML_ATTRS and NUMERIC_ATTRS are no longer
exported from glimmer-attr-presence. The util's public surface is now
`classifyAttribute` and `inferAttrKind`. Callers wanting the underlying
classification can use property-information directly.
Comment thread docs/glimmer-attribute-behavior.md Outdated
| h8 | `<div aria-hidden={{"false"}}></div>` | `<div aria-hidden="false"></div>` | `"false"` | `true` | **visible** |
| h9 | `<div aria-hidden={{null}}></div>` | `<div></div>` | `null` | `false` | **visible** |
| h10 | `<div aria-hidden={{undefined}}></div>` | `<div></div>` | `null` | `false` | **visible** |
| h11 | `<div aria-hidden={{""}}></div>` | `<div aria-hidden=""></div>` | `""` | `true` | **contested** |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contested? that's not a thing. it's either present or not present.

this whole doc can be distilled to:

  • setting to a falsey non-string value hides the attribute
  • wrapping {{}} in a string gives you a string, "{{false}}" is output as "false", which is truthy, refer to HTML for what string values do

Copy link
Copy Markdown
Contributor Author

@johanrd johanrd Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On contested — thanks, fixed (was more related to how peer linters interpreted it).

On distillation — I don't think this destillation holds across attribute kinds (the whole reason the table was needed). E.g:

  • <input autocomplete={{false}} /><input autocomplete="false"> (row i4) — falsey-non-string does not hide for plain string attrs
  • <input disabled="{{false}}" /><input disabled=""> with IDL disabled = true (row d7) — concat on a boolean attr sets the IDL property regardless of the inner value, not "string passed through to HTML"

Concat-mustache seems to fork by attribute kind (boolean HTML vs. ARIA vs. numeric vs. plain string), which the two-rule model collapses. (May even count as a bug in glimmer?)

E.g. h5: <div aria-hidden={{true}}></div><div aria-hidden=""></div>. The author wrote the literal true and got an empty string instead of "true". aria-hidden="" isn't a valid enumerated value. It looks like Glimmer is applying its boolean-HTML-attr coercion (true → present-with-empty-value) to an ARIA attribute where the spec wants string "true" / "false".

Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…oss-attr m6/d3 analog)

`controls` on `<audio>`/`<video>` is an HTML boolean attribute, so per the
cross-attribute observation in docs/glimmer-attribute-behavior.md, bare
`{{false}}` / `{{null}}` / `{{undefined}}` cause Glimmer to omit the
attribute at runtime. The helper's previous AST-presence check
(hasAttribute) treated `<video controls={{false}}>` as still having
controls — a false positive that propagated to every rule using
isHtmlInteractiveContent: nested-interactive, no-noninteractive-tabindex,
interactive-supports-focus, click-events-have-key-events, etc.

After:
- `controls` flows through classifyAttribute. Bare-mustache falsy literals
  now correctly classify as 'absent' → element is NOT interactive at
  runtime.
- `href` and `usemap` continue to use AST-presence — those are plain
  string attributes that don't falsy-coerce (i4 analog: bare `{{false}}`
  on a plain string renders as the literal `"false"`, attribute kept).

Concat forms (`controls="{{X}}"`) still classify as 'present' because
concat is never falsy at runtime — so the existing pass-through behavior
for concat is preserved.

Tests:
- New: `<video controls={{false}}>` → not interactive (regression-locking).
- New: `<audio controls={{null}}>` → not interactive.
- New: `<video controls="{{false}}">` → IS interactive (concat sets IDL
  true regardless of inner literal).
- All 9555 existing tests pass — no rule was relying on the buggy
  behavior, so this fix lands cleanly across all consumers.
johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 28, 2026
johanrd added 4 commits April 28, 2026 14:56
…+ spec cite + empirical verification path

Per @NullVoxPopuli's review on PR ember-cli#2769: "contested" was hand-waving — an
element either is or isn't in the AT. The four cells previously labelled
"contested" (h1, h2, h5, h11) all render `aria-hidden=""`, which per
WAI-ARIA 1.2 §6.6 is an invalid enumerated value that falls back to the
default "undefined" state — the element is **visible**.

Changes:
- Replace "contested" with "visible" + ARIA §6.6 citation in h1, h2, h5, h11
  rows and the surrounding prose.
- Keep the Glimmer-coercion note for h5: `{{true}}` rendering empty is
  Glimmer-specific behaviour worth flagging — it diverges from what the
  spec wants for the literal token.
- Add a "Verifying the visible verdict empirically" subsection with two
  paths:
    1. DevTools Accessibility panel inspection (browser-implementation
       authority).
    2. A scriptable approximation that encodes §6.6 directly in JS, useful
       when the DevTools panel isn't convenient. The two converge in
       Chrome/Firefox/Safari — divergence would be a browser bug and is
       caught by the panel check.
…al check

CI self-lint flagged the JS code block with three eslint errors:
- curly violations on bare `if (…) return true` and `if (!el) continue` (eslint
  enforces always-blocks via the curly rule)
- unicorn/prefer-query-selector on `getElementById('id')`

Fixes:
- Wrap the bare-expression `if`s in blocks.
- Replace `getElementById(id)` with `querySelector('#${id}')`.

Also: empirical update for the "visible" verdict. `Element.computedRole`
returns the actual computed AX-tree role (or null if hidden) in Chrome
135+/Safari 18.4+. Verified empirically — `document.querySelector('#h5')
.computedRole` returns `"generic"` for the h5 row (`<div aria-hidden={{true}}>`),
confirming the element IS exposed to AT and thus visible. Promoted
computedRole to the primary console check, demoted DevTools Accessibility
panel to a fallback when the API isn't available, and kept the spec-encoded
JS as a third option that works everywhere.
computedRole returns the element's role regardless of aria-hidden state —
it's not a visibility check. Verified: `<div aria-hidden="true">` (h3)
still returns `computedRole === 'generic'` even though it's removed from
the AX tree. aria-hidden controls *exposure*, not *role*.

Restored DevTools Accessibility panel as the browser-authoritative check
and added a one-line note explaining why computedRole isn't usable for
this question. The spec-encoded JS walker remains the scriptable fallback.

Earlier commit (f2ecc0d) introduced the wrong claim — this corrects it.
@johanrd johanrd marked this pull request as draft April 30, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants