Repository navigation
fix(player): keep new controls in sync with playback - #6373
Conversation
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
miguel-heygen
left a comment
There was a problem hiding this comment.
Independent review of 83ea696089d745413325e353dda879f19559affa against 84b63193463bac2e3e7f8500d03fb7da9fbf8846. No blocking finding.
packages/player/src/hyperframes-player.ts:1617 initializes newly created controls through the same time/playing/speed APIs used by ongoing updates. This fixes the internal controls' play/pause toggle state as well as their labels; the existing removal/disconnect destroy paths remain intact. The four-file API/git census agrees, and the docs describe the implemented behavior. No prior substantive review or inline finding was present.
Audited: both _setupControls callers, attribute ordering, video/proxy state updates, play/pause/seek, control recreation, controls' update methods, hide timer and destroy cleanup. The three added cases cover preconfigured speed and both playing/paused recreation paths.
Executed independently: Chromium with an offline synthetic muted 10-second MP4 and a browser bundle built from this exact player's/core's/parsers' source using existing esbuild (no install). Playing and paused controls show the correct button, 3s/10s time, progress, 2x label/accessible name/active option; one click toggles playback. Remove/re-add and playback-rate-before-controls pass. A precise baseline mutation removing only the three initialization calls reproduces Play/1x/zero-time and the ineffective first pause click. No browser page errors; the only failed local request was the harness's absent favicon (404).
CI independently read: exact-head support job 121359102128 executed 560 player tests in 17 files, all passed (including 252 in hyperframes-player.test.ts). Docs job 121358993676 executed validation and broken-links successfully. Final bounded snapshot: 47 successful, 15 skipped, one neutral and six running checks, no failed check. Fork preview approval is external and was not mutated.
Trusting / limits: author's screenshots/repro remain their execution; my control uses a local source bundle and synthetic video, not a deployed release or authenticated backend. No Safari/Firefox, native app, provider, or full local workspace-suite claim. No new worktree/install/product edit, merge or deployment; task browser and server stopped.
Verdict: APPROVE
Reasoning: The change restores the full current controls state at creation, and the independent real-media baseline/fix witness discriminates the reported regression. Relevant hosted player/docs gates passed; pending broader checks are not claimed green.
— Magi (desktop-review)
Merge 8 upstream commits (85dcc19..7d6c02f: heygen-com#6418, heygen-com#5273, heygen-com#6245, heygen-com#5593, heygen-com#5447, heygen-com#6415, heygen-com#6373, heygen-com#6375). Keep the fork's Windows-only ci.yml. For cancellableProcess.ts take upstream's heygen-com#5273, which supersedes the fork's own 744a620 port (same StringDecoder design, but the output limit counts raw bytes). Record the batch in docs/DECISIONS.md, mark heygen-com#6373/heygen-com#6375/heygen-com#6415/heygen-com#6418 as merged, and advance the commit watermark to 7d6c02f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
What
Showing controls on an existing player now reflects its current playback state, position and speed. A video playing at 2x gets a Pause button and a 2x speed label immediately; clicking Pause stops it.
Why
New controls kept their default Play/1x/0:00 state. The first playback-button click could leave a video running, and a paused player's time stayed at zero until another update.
Related work
Fixes #6372.
#1407 handles controls cleanup, #2152 connection cleanup, and #6032 the speed button's accessible name. This addresses the current state when creating the controls.
How
Initialize the controls through the existing time, playing and speed update methods. The regression tests cover the initial attribute order, adding/recreating controls while playing or paused, and the resulting playback-button action. Player documentation now describes showing controls during playback.
Before
Chromium 156, 1000 x 700 viewport, 800 x 450 player. The existing synthetic test video is at 3 seconds. Baseline
535317115; player source unchanged at the PR base84b631934.After
Same browser, video, viewport, seek and playback rate. Reproduction script and recorded state. Captures are public immutable files in the fork's evidence branch and are excluded from this PR.
Test plan
Tested commit:
83ea696089d745413325e353dda879f19559affa.bun run --cwd packages/player test --maxWorkers=1.Also passed player build/runtime-pin verification, source and perf typechecks, changed-source oxlint, oxfmt for supported changed files, comment checks and
git diff --check. The full workspace build passed at535317115; after updating to84b631934, core/player builds and player tests/typechecks were repeated. Mintlifyvalidateandbroken-linkscould not run becausemintis not installed. Browser verification was limited to Chromium.