fix(554): mute the channel preview, and name blocked autoplay only when that is the cause #910

Merged

timothy merged 5 commits from 554-preview-autoplay-blocked-hint into main 2026-09-05 04:23:11 +02:00

Conversation 1 Commits 5 Files Changed 6

+533 -8

timothy commented 2026-09-05 01:38:10 +02:00

Owner

Copy Link

Copy Source

fixes #554

Root cause

The channel preview panel's <video> element played unmuted. Browsers permit autoplay of
muted media without user activation but block or reject autoplay on unmuted media, so the
first play() attempt after selecting a channel would reject — and the panel gave no
indication why, leaving a stalled preview with no explanation.

The fix, as pushed

• HlsPlayer gained a muted?: boolean prop, opt-in and defaulting to false — not
  applied unconditionally. The shared player also backs the playback-troubleshooting screen,
  whose job includes verifying the audio side of an FFmpeg profile, and which the legacy
  Blazor player never muted; muting it unconditionally would have silently broken that.
  ChannelPreviewPanel is the one caller that opts in.
• onAutoplayBlocked fires only when the rejected video.play() throws a DOMException
  named NotAllowedError, on both the MSE and native-HLS (Safari) playback paths. A play()
  interrupted by teardown rejects with AbortError — which is exactly what the panel's own
  Retry button produces while an earlier MANIFEST_PARSED play() is still pending — so an
  unfiltered handler would mislabel that as "autoplay was blocked."
• The hint's visibility has a single guard: autoplayBlocked && state === 'starting'.
  There are exactly three paths back to starting, and each is responsible for clearing
  autoplayBlocked itself:
  1. Retry — clears it directly.
  2. Channel change — cleared in the render-phase reset.
  3. The mute opt-in button (onOptIn) — clears nothing, and that is correct: the
     button renders only while started is false, and started only goes false in the
     same render-phase reset that already clears the flag two lines later, so no player
     exists to have set the flag while the button is visible. Adding a clear here would be
     dead code no test could distinguish.

Measured (this push, npm test)

• Full suite: 121 test files, 1343 tests, all passed.
• The two touched suites: HlsPlayer.test.tsx — 15 passed; ChannelPreviewPanel.test.tsx
  — 27 passed.
• PlaybackTroubleshootingScreen.test.tsx, HlsPlayer.test.tsx, and
  ChannelPreviewPanel.test.tsx together: 55 passed.
• Mutation coverage (from the test-pinning commit): deleting the Retry-path clear reds only
  the Retry test; deleting the render-phase-reset clear reds only the channel-change test;
  widening the render guard to autoplayBlocked && (dropping state === 'starting') reds a
  third, distinct test. Unmutated: all pass.
• npm run check:api, lint, typecheck, build all clean; check-doc-narrative.py --diff origin/main reports 0 warnings; live-E2E (dotnet build + scripts/e2e-local.sh,
  port 8440) served /app/ with HTTP 200.

This branch is 4 commits ahead of the original fix (5 total): 02e1c583e (initial fix),
4e042fb7d, b6b4fb661, f119bfcb2, 350509f8b.

• 4e042fb7d — fixes the three defects round one's review found: filters
  onAutoplayBlocked to NotAllowedError only, makes muted an opt-in prop instead of
  applying it unconditionally, and removes the redundant onPlaying clear (the two clauses
  hiding the hint masked each other — either could be deleted with the suite green). Also
  adds coverage for the previously-untested native-HLS (Safari) branch.
• b6b4fb661 — pins both remaining clears (Retry, channel change) with one test each,
  each asserting the hint is gone and the player really re-mounted, so the assertions can't
  pass merely because the panel is unrendered. Includes the mutation-testing measurements
  above.
• f119bfcb2 — round two found the invariant statement (code comment, test comment,
  docs/spa-conventions.md §5b) was false: it enumerated only two paths back to starting
  when onOptIn is a third, and it clears nothing. Documents why that omission is correct
  and pins the reachability premise with a test that fails if the opt-in button ever outlives
  the mounted player.
• 350509f8b — tightens that same reachability explanation: the load-bearing fact is
  that the button renders only while started is false, not "before the player has ever
  mounted" (a channel switch to a forced channel can re-render the button after a player
  mounted for a previous channel).

Review

Three review rounds ran on this branch before this push:

• Round one — 1 blocking finding (the muted-everywhere design change and the
  unfiltered onAutoplayBlocked handler), 2 should-fix, 1 nit. Answered by 4e042fb7d.
• Round two — 1 should-fix: the opt-in button is a third path back to starting that the
  stated invariant omitted. Answered by f119bfcb2 and 350509f8b.
• Round three — measurement staleness only (this PR body and the issue's closing-record
  comment described an earlier, 1-commit state of the branch after later commits had landed).
  Answered by this rewrite.

The PR as originally opened was written before any of the fix commits above existed, against
just 02e1c583e, and so misdescribed the branch that has since landed.

Cross-family review: not required for this change (routine risk class under
process.independent-review-rubric).

🤖 Generated with Claude Code

https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

fixes #554 ## Root cause The channel preview panel's `<video>` element played unmuted. Browsers permit autoplay of muted media without user activation but block or reject autoplay on unmuted media, so the first play() attempt after selecting a channel would reject — and the panel gave no indication why, leaving a stalled preview with no explanation. ## The fix, as pushed - `HlsPlayer` gained a `muted?: boolean` prop, **opt-in and defaulting to `false`** — not applied unconditionally. The shared player also backs the playback-troubleshooting screen, whose job includes verifying the audio side of an FFmpeg profile, and which the legacy Blazor player never muted; muting it unconditionally would have silently broken that. `ChannelPreviewPanel` is the one caller that opts in. - `onAutoplayBlocked` fires **only** when the rejected `video.play()` throws a `DOMException` named `NotAllowedError`, on both the MSE and native-HLS (Safari) playback paths. A play() interrupted by teardown rejects with `AbortError` — which is exactly what the panel's own Retry button produces while an earlier `MANIFEST_PARSED` play() is still pending — so an unfiltered handler would mislabel that as "autoplay was blocked." - The hint's visibility has a **single guard**: `autoplayBlocked && state === 'starting'`. There are exactly three paths back to `starting`, and each is responsible for clearing `autoplayBlocked` itself: 1. **Retry** — clears it directly. 2. **Channel change** — cleared in the render-phase reset. 3. **The mute opt-in button (`onOptIn`)** — clears nothing, and that is correct: the button renders only while `started` is `false`, and `started` only goes `false` in the same render-phase reset that already clears the flag two lines later, so no player exists to have set the flag while the button is visible. Adding a clear here would be dead code no test could distinguish. ## Measured (this push, `npm test`) - Full suite: **121 test files, 1343 tests, all passed**. - The two touched suites: `HlsPlayer.test.tsx` — **15 passed**; `ChannelPreviewPanel.test.tsx` — **27 passed**. - `PlaybackTroubleshootingScreen.test.tsx`, `HlsPlayer.test.tsx`, and `ChannelPreviewPanel.test.tsx` together: **55 passed**. - Mutation coverage (from the test-pinning commit): deleting the Retry-path clear reds only the Retry test; deleting the render-phase-reset clear reds only the channel-change test; widening the render guard to `autoplayBlocked &&` (dropping `state === 'starting'`) reds a third, distinct test. Unmutated: all pass. - `npm run check:api`, `lint`, `typecheck`, `build` all clean; `check-doc-narrative.py --diff origin/main` reports 0 warnings; live-E2E (`dotnet build` + `scripts/e2e-local.sh`, port 8440) served `/app/` with HTTP 200. This branch is 4 commits ahead of the original fix (5 total): `02e1c583e` (initial fix), `4e042fb7d`, `b6b4fb661`, `f119bfcb2`, `350509f8b`. - **`4e042fb7d`** — fixes the three defects round one's review found: filters `onAutoplayBlocked` to `NotAllowedError` only, makes `muted` an opt-in prop instead of applying it unconditionally, and removes the redundant `onPlaying` clear (the two clauses hiding the hint masked each other — either could be deleted with the suite green). Also adds coverage for the previously-untested native-HLS (Safari) branch. - **`b6b4fb661`** — pins both remaining clears (Retry, channel change) with one test each, each asserting the hint is gone *and* the player really re-mounted, so the assertions can't pass merely because the panel is unrendered. Includes the mutation-testing measurements above. - **`f119bfcb2`** — round two found the invariant statement (code comment, test comment, `docs/spa-conventions.md` §5b) was false: it enumerated only two paths back to `starting` when `onOptIn` is a third, and it clears nothing. Documents why that omission is correct and pins the reachability premise with a test that fails if the opt-in button ever outlives the mounted player. - **`350509f8b`** — tightens that same reachability explanation: the load-bearing fact is that the button renders only while `started` is `false`, not "before the player has ever mounted" (a channel switch to a forced channel can re-render the button after a player mounted for a previous channel). ## Review Three review rounds ran on this branch before this push: - **Round one** — 1 blocking finding (the `muted`-everywhere design change and the unfiltered `onAutoplayBlocked` handler), 2 should-fix, 1 nit. Answered by `4e042fb7d`. - **Round two** — 1 should-fix: the opt-in button is a third path back to `starting` that the stated invariant omitted. Answered by `f119bfcb2` and `350509f8b`. - **Round three** — measurement staleness only (this PR body and the issue's closing-record comment described an earlier, 1-commit state of the branch after later commits had landed). Answered by this rewrite. The PR as originally opened was written before any of the fix commits above existed, against just `02e1c583e`, and so misdescribed the branch that has since landed. Cross-family review: not required for this change (routine risk class under `process.independent-review-rubric`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

timothy added 1 commit 2026-09-05 01:38:10 +02:00

fix(554): mute the preview player so autoplay is never blocked ... 02e1c583e9

HlsPlayer's manifest GET can block until segments exist (unbounded
maxTimeToFirstByteMs), so MANIFEST_PARSED can arrive past the
browser's transient user-activation window and video.play() gets
rejected as blocked autoplay — the channel preview panel then sat at
"starting" over a black frame with no hint the operator just needed
to press play.

Render the <video> element muted (browsers permit autoplay of muted
media without user activation) so the common case starts on its own,
and add an optional onAutoplayBlocked callback for the residual case
(stricter policy/extension) that the channel preview panel wires to a
"press play" hint shown only while still starting.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

timothy referenced this pull request 2026-09-05 01:39:03 +02:00

Channel preview can sit at &quot;starting&quot; when autoplay is blocked, with no hint to press play #554

timothy referenced this pull request 2026-09-05 01:40:20 +02:00

Orchestration workflows: should-fix findings never reach the fixer, and the finisher then attests they were folded in #911

timothy changed title from fix(554): mute the channel preview so autoplay is never blocked to fix(554): mute the channel preview, and name blocked autoplay only when that is the cause 2026-09-05 02:13:14 +02:00

timothy referenced this pull request 2026-09-05 02:13:52 +02:00

Channel preview can sit at &quot;starting&quot; when autoplay is blocked, with no hint to press play #554

timothy added 4 commits 2026-09-05 03:01:41 +02:00

fix(554): the hint's cause must be measured, and muting is the consumer's call ... 4e042fb7de

Round one measured three defects in the first commit.

onAutoplayBlocked fired on ANY rejected video.play(), so the panel could
say "autoplay was blocked" when it was not. A play() interrupted by
teardown rejects with AbortError — which is exactly what the panel's own
Retry produces while the MANIFEST_PARSED play() is still pending — and
because the element was muted, a genuine NotAllowedError is the rare
case, so the realistic firings were the mislabelled ones. Report only a
DOMException named NotAllowedError, on both the MSE and native paths.

`muted` was applied to the shared player unconditionally, which silently
muted the playback-troubleshooting screen — the tool whose job includes
verifying the audio side of an FFmpeg profile, and which the legacy
Blazor player never muted. Make it an opt-in `muted` prop defaulting to
false; the channel preview passes it, troubleshooting does not, and a
test on each side pins its own value.

The two clauses hiding the hint once playback starts masked each other:
removing either alone left the panel suite green. Every path back to
'starting' (Retry, a channel change) already clears the flag itself, so
the clear in onPlaying could never be the load-bearing guard — drop it
and let the `state === 'starting'` render guard be the single pinned one.

Also cover the native-HLS (Safari) branch, which no test had ever
executed: its play() kick, its playing/error wiring, and both autoplay
rejection names.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

test(554): pin both clears the hint's single guard depends on ... b6b4fb6619

Dropping the onPlaying clear made the `state === 'starting'` render guard the
hint's only guard, which moves the burden onto the two paths back to
`starting`: each has to clear `autoplayBlocked` itself. Both were assertable
but unasserted — either `setAutoplayBlocked(false)` could be deleted with the
whole panel suite green, so the invariant the code comment and
docs/spa-conventions.md §5b both state was unpinned in both of its named paths.

Add one test per path (Retry; a channel switch), each asserting the hint is
gone while the panel is back at `starting` — so the render guard cannot be
what hid it. Each also asserts the player really re-mounted (loadSource count
/ last URL, plus a non-null <video>), so the hint cannot be absent merely
because the `resolvedSrc` block is unrendered.

Measured on this tree, each mutation caught by exactly one test:
deleting the onRetry clear reds only 'clicking Retry clears the
autoplay-blocked hint' (Tests 1 failed | 25 passed); deleting the
render-phase reset clear reds only 'clears the autoplay-blocked hint when
switching to a different channel' (1 failed | 25 passed); replacing
`autoplayBlocked && state === 'starting' &&` with `autoplayBlocked &&` reds
one test too. Unmutated: 26 passed.

Refs #554

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

docs(554): the opt-in is a third path back to starting, and the invariant must say so ... f119bfcb29

The hint's single-guard invariant was stated as "every path back to `starting` clears
the flag itself", enumerating Retry and a channel change. `onOptIn` is a third such
path and clears nothing, so the sentence was false as written — in the code comment,
in the Retry test's comment, and in docs/spa-conventions.md §5b.

Adding a clear to `onOptIn` would be dead code no test could distinguish, which is the
exact shape this branch removed from `onPlaying`. The omission is correct for a reason
none of the three places stated: the opt-in button renders only while `started` is
false, `started` only goes false in the render-phase reset that clears the flag two
lines later, and no player exists to set the flag while `started` is false. State that
exception, and pin the reachability premise it rests on with a test that fails if the
opt-in button outlives the mounted player.

Refs #554

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

docs(554): the opt-in's unreachability is about started, not about mount order ... 350509f8bb

"its button renders only before the player has ever mounted" is loose: a channel
switch to a forced channel re-renders the button after a player has mounted for the
previous channel. The load-bearing fact is the one the code comment states — the
button renders only while `started` is false, and the only thing that un-starts the
panel is the channel reset that clears the flag in the same batch.

Refs #554

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

timothy referenced this pull request 2026-09-05 03:04:12 +02:00

Channel preview can sit at &quot;starting&quot; when autoplay is blocked, with no hint to press play #554

timothy commented 2026-09-05 03:04:53 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ 350509f

Orchestrated-session workflow: three cold review rounds in the worktree before the push (correctness Opus high in its own worktree, conformance sonnet high); round one's design findings (muted-everywhere, NotAllowedError filtering, masking clauses) and round two's third path back to starting were answered by four fix commits, each clear pinned by a test that reddens alone; round three carried only measurement staleness, resynced at push. Cross-family review: not required (routine risk class; SPA + docs). Verdict on the pushed head after a plain fast-forward push; patch unchanged.

Review-verdict: MERGEABLE @ 350509f Orchestrated-session workflow: three cold review rounds in the worktree before the push (correctness Opus high in its own worktree, conformance sonnet high); round one's design findings (muted-everywhere, NotAllowedError filtering, masking clauses) and round two's third path back to starting were answered by four fix commits, each clear pinned by a test that reddens alone; round three carried only measurement staleness, resynced at push. Cross-family review: not required (routine risk class; SPA + docs). Verdict on the pushed head after a plain fast-forward push; patch unchanged.

timothy referenced this pull request 2026-09-05 03:23:04 +02:00

fix(911): a should-fix finding sends the round back, a failed-lens round is an error, and a harness executes the loop #912

timothy referenced this issue from a commit 2026-09-05 03:23:05 +02:00

fix(911): a should-fix finding sends the round back, and the finisher reports the recorded review history

timothy referenced this issue from a commit 2026-09-05 03:23:05 +02:00

fix(911): in a resume, the existing PR's stale head is the expected state until the push, not a finding

timothy referenced this pull request 2026-09-05 03:23:42 +02:00

Orchestration workflows: should-fix findings never reach the fixer, and the finisher then attests they were folded in #911

timothy merged commit fd58b1df33 into main 2026-09-05 04:23:11 +02:00

timothy deleted branch 554-preview-autoplay-blocked-hint 2026-09-05 04:23:12 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#910