Skip to content

Post-merge-review: Fix template-require-media-caption: skip caption check when muted is dynamic#2700

Merged
NullVoxPopuli merged 3 commits intoember-cli:masterfrom
johanrd:day_fix/template-require-media-caption
Apr 14, 2026
Merged

Post-merge-review: Fix template-require-media-caption: skip caption check when muted is dynamic#2700
NullVoxPopuli merged 3 commits intoember-cli:masterfrom
johanrd:day_fix/template-require-media-caption

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 13, 2026

Summary

  • muted="{{isMuted}}" (ConcatStatement) was not handled → false positive requiring captions
  • Any non-text/non-mustache muted value is now treated as exempt (matches upstream)

Test plan

  • <video muted="{{isMuted}}"> → valid (no caption required)
  • <audio muted="{{this.isMuted}}"> → valid
  • <video muted={{false}}> → still requires captions (explicit false)

johanrd added 2 commits April 13, 2026 14:43
…dynamic

Port had no GlimmerConcatStatement handling for the muted attribute,
so <video muted="{{isMuted}}"> false-positively required captions.
Treat ConcatStatement (and other non-text values) as muted/exempt
to match upstream.
@johanrd johanrd marked this pull request as ready for review April 13, 2026 17:10
@johanrd johanrd force-pushed the day_fix/template-require-media-caption branch from d7d8f84 to e3a8d8b Compare April 13, 2026 17:39
@johanrd johanrd changed the title Fix template-require-media-caption: skip caption check when muted is dynamic Post-merge-review: Fix template-require-media-caption: skip caption check when muted is dynamic Apr 13, 2026

// Any other dynamic value (e.g. muted="{{isMuted}}" → ConcatStatement,
// or muted={{#if ...}}...{{/if}} → BlockStatement) → treat as exempt.
// Matches upstream ember-template-lint behavior where MustacheStatement,
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.

can we stop adding comments about "matches upstram" this means nothing post PR merge 🙈

@NullVoxPopuli NullVoxPopuli enabled auto-merge April 14, 2026 13:15
@NullVoxPopuli NullVoxPopuli merged commit cb738d6 into ember-cli:master Apr 14, 2026
10 checks passed
@johanrd johanrd deleted the day_fix/template-require-media-caption branch April 14, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants