From 692a41fda94fc1d9230516d9c69875cf37e69085 Mon Sep 17 00:00:00 2001 From: Preston Mantel Date: Sat, 19 Sep 2026 15:24:01 -0700 Subject: [PATCH 1/8] fix(debates): a refused play() is not a playing video (GEO-2978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A debate card on a phone showed a still frame and needed two taps to start: the first only revealed a play control, the second played it. Measured on the device with both videos fully buffered, muted and inline: card0 · ready=true active=true playing=true blocked=false playBtn=0 v0 · 100% shown · PAUSED · t=0.0 · calls=[play REJECTED:NotAllowedError] The app believed it was playing while the elements sat paused at zero, having been refused — iOS declines every autoplay in Low Power Mode, and when auto-play is turned off for a site. `playBothWithMutedFallback` judges success by polling `paused` shortly after calling `play()` rather than trusting the promise, because a resolved `play()` can precede the element leaving `paused` — that is what stopped the feed claiming "Could not play both videos" on every scroll. Its comment then said a rejection needs no special case, "a rejection leaves the element paused, so it fails the same check". It does not, or not in time: `play()` sets `paused` false *synchronously* and only then rejects, and the user agent re-pauses afterwards. The first confirm poll sees both elements running and reports 'playing' for a play being refused. Nothing recovers from there. The autoplay effect will not retry a card it believes is playing, no refusal is recorded, and the paused glyph never renders — so the viewer's first tap only pauses what was never running. Three things follow: * A refusal invalidates the confirm reading, on the first attempt and on the muted retry. `isRefusal` tells `NotAllowedError` — the browser declining — from the `AbortError` our own `pause()` raises. * A refusal outlives the attempt that found it, recorded above the ownership check: it describes the device, while that check exists to stop a superseded attempt writing playback state. Superseded is the norm, since `resumeBoth` bumps its generation on entry and the effect re-enters while `playing` is false. * The card offers the play control instead of "Could not play both videos. Try Play again." — copy that named a control which was not on screen and a failure that had not happened. `awaitingTap` names the state the control renders from: the viewer paused, or the browser refused, which are different facts and the same question. Co-Authored-By: Claude Opus 5 (1M context) --- .../browse/debate-feed-player.test.tsx | 54 ++++++++- .../debates/browse/debate-feed-player.tsx | 41 ++++++- apps/web/core/debates/playback-utils.test.ts | 89 +++++++++++++++ apps/web/core/debates/playback-utils.ts | 74 ++++++++++-- .../core/debates/use-debate-playback.test.tsx | 107 +++++++++++++++++- apps/web/core/debates/use-debate-playback.ts | 49 +++++++- 6 files changed, 392 insertions(+), 22 deletions(-) diff --git a/apps/web/core/debates/browse/debate-feed-player.test.tsx b/apps/web/core/debates/browse/debate-feed-player.test.tsx index 4492f4fab6..0cdc578da9 100644 --- a/apps/web/core/debates/browse/debate-feed-player.test.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.test.tsx @@ -44,7 +44,14 @@ const votes: DebateVotesResult = { * A controller in the one state that matters here: playing, with a turn in progress, both * recordings loaded. `mutedByUser` and `turnState` are what the audio gating reads. */ -function controllerFixture(overrides: { mutedByUser: boolean; turnSlot: 1 | 2; isResuming?: boolean }) { +function controllerFixture(overrides: { + mutedByUser: boolean; + turnSlot: 1 | 2; + isResuming?: boolean; + /** The browser refused to autoplay — see `useDebatePlayback`. */ + autoplayBlocked?: boolean; + playing?: boolean; +}) { return { slot1VideoRef: { current: null }, slot2VideoRef: { current: null }, @@ -53,7 +60,8 @@ function controllerFixture(overrides: { mutedByUser: boolean; turnSlot: 1 | 2; i urls: { slot1: 'https://cdn.test/slot1.webm', slot2: 'https://cdn.test/slot2.webm' }, ready: true, error: null, - playing: true, + playing: overrides.playing ?? true, + autoplayBlocked: overrides.autoplayBlocked ?? false, userPaused: false, isScrubbing: false, isResuming: overrides.isResuming ?? false, @@ -77,7 +85,13 @@ function controllerFixture(overrides: { mutedByUser: boolean; turnSlot: 1 | 2; i } function renderPlayer( - overrides: { mutedByUser: boolean; turnSlot: 1 | 2; isResuming?: boolean }, + overrides: { + mutedByUser: boolean; + turnSlot: 1 | 2; + isResuming?: boolean; + autoplayBlocked?: boolean; + playing?: boolean; + }, reactStrictMode = false ) { mocks.controller = controllerFixture(overrides); @@ -196,3 +210,37 @@ describe('DebateFeedPlayer media release (GEO-2963)', () => { expect(slot2.hasAttribute('src')).toBe(false); }); }); + +/** + * A refused autoplay has to reach the screen (GEO-2978). + * + * Three fixes went in without the control ever appearing, and every attempt to + * check it went through a browser — which measured the hero carousel at the top + * of Explore rather than a debate card, twice. This asks the component + * directly: given a controller that says the browser refused, is there + * something to tap? + */ +describe('a refused autoplay', () => { + it('shows the play control', () => { + const { container } = (() => { + mocks.controller = controllerFixture({ + mutedByUser: true, + turnSlot: 1, + autoplayBlocked: true, + playing: false, + }); + return render(); + })(); + + expect(container.querySelector('[aria-label="Resume debate"]')).not.toBeNull(); + }); + + it('shows nothing extra while playback is running normally', () => { + mocks.controller = controllerFixture({ mutedByUser: true, turnSlot: 1 }); + const { container } = render( + + ); + + expect(container.querySelector('[aria-label="Resume debate"]')).toBeNull(); + }); +}); diff --git a/apps/web/core/debates/browse/debate-feed-player.tsx b/apps/web/core/debates/browse/debate-feed-player.tsx index 4d18c52596..7baefdf209 100644 --- a/apps/web/core/debates/browse/debate-feed-player.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.tsx @@ -51,6 +51,7 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb error, playing, userPaused, + autoplayBlocked, isScrubbing, isResuming, playbackEnded, @@ -88,20 +89,48 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb // mid-scrub. React.useEffect(() => { if (!ready) return; - if (active && !userPaused && !isScrubbing && !playing && !playbackEnded) { + // A refusal is not retried: the browser gives the same answer every time, and + // only the viewer's tap is a gesture it will accept. + if (active && !awaitingTap && !isScrubbing && !playing && !playbackEnded) { void resumeBoth(); } else if (!active && playing) { suspend(); } - }, [active, isScrubbing, playbackEnded, playing, ready, resumeBoth, suspend, userPaused]); + }, [active, autoplayBlocked, isScrubbing, playbackEnded, playing, ready, resumeBoth, suspend, userPaused]); - const showControls = ready && (userPaused || (playbackEnded && !hasVoted)); - // End of an unvoted debate offers a replay; a user pause shows the paused glyph. + /** + * Stopped, and only a tap will start it. + * + * The two ways in are different facts — the viewer paused, or the browser + * refused — and identical from here: the video is not running and the control + * is the only answer either accepts. + */ + const awaitingTap = userPaused || autoplayBlocked; + + const showControls = ready && (awaitingTap || (playbackEnded && !hasVoted)); + // End of an unvoted debate offers a replay; a stopped one shows the paused glyph. const showReplay = ready && playbackEnded && !hasVoted; - const showPausedGlyph = ready && userPaused && !playbackEnded; + const showPausedGlyph = ready && awaitingTap && !playbackEnded; return ( -
+
{ * in between. A caller that checks ownership only once this returns is too late: `play()` has * already been called, and no state check can take it back. */ + /** + * The element iOS actually gives you when it refuses (GEO-2978). + * + * `play()` sets `paused` false synchronously and only then rejects; the user agent pauses it + * again afterwards. So the confirm poll's first pass sees both elements un-paused and, before + * this fix, reported 'playing' for a play that was being refused — which left the card + * believing it was playing while both videos sat at `t=0.0`, with no retry and no play button, + * and needing two taps to start. + */ + function refusingVideo() { + const video = { + muted: true, + paused: true, + plays: 0, + async play() { + video.plays += 1; + // Synchronous, exactly as the spec has it. + video.paused = false; + await Promise.resolve(); + // And the user agent takes it back. + video.paused = true; + throw Object.assign(new Error('refused'), { name: 'NotAllowedError' }); + }, + }; + return video; + } + + it('does not report playing when the refusal un-pauses the element first', async () => { + const a = refusingVideo(); + const b = refusingVideo(); + + expect(await playBothWithMutedFallback(a, b)).toBe('blocked'); + }); + + /** + * The same shape where `paused` never comes back — a stricter reading of the same race. The + * browser's answer decides it either way. + */ + it('trusts the refusal over a stale un-paused reading', async () => { + const stuck = () => { + const video = { + muted: true, + paused: true, + plays: 0, + async play() { + video.plays += 1; + video.paused = false; + throw Object.assign(new Error('refused'), { name: 'NotAllowedError' }); + }, + }; + return video; + }; + + expect(await playBothWithMutedFallback(stuck(), stuck())).toBe('blocked'); + }); + + /** + * A refusal is the browser's answer and survives the cancellation check, because + * `resumeBoth` re-enters while `playing` is false and so cancels its own previous + * attempt as a matter of course. Reporting 'cancelled' there lost the answer every + * time and the card never learned it had been refused (GEO-2978). + * + * Muted on both, so there is no retry left that could turn this into playback. + */ + it('reports a refusal even when the attempt was cancelled while confirming', async () => { + const a = fakeVideo({ muted: true, blockUnmuted: false }); + const b = fakeVideo({ muted: true, blockUnmuted: false }); + a.play = async function refuse() { + a.plays += 1; + throw Object.assign(new Error('refused'), { name: 'NotAllowedError' }); + }; + b.play = a.play.bind(b); + + expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('blocked'); + }); + + /** But an interruption of ours is still a cancellation, not a refusal. */ + it('still reports a cancellation when our own pause interrupted the attempt', async () => { + const a = fakeVideo({ muted: true, blockUnmuted: false }); + const b = fakeVideo({ muted: true, blockUnmuted: false }); + a.play = async function abort() { + a.plays += 1; + throw Object.assign(new Error('interrupted by a call to pause()'), { name: 'AbortError' }); + }; + b.play = a.play.bind(b); + + expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('cancelled'); + }); + it('does not retry when the attempt was cancelled while confirming', async () => { const a = fakeVideo({ muted: false }); const b = fakeVideo({ muted: false }); diff --git a/apps/web/core/debates/playback-utils.ts b/apps/web/core/debates/playback-utils.ts index 6bbba3ad3a..125904fd2e 100644 --- a/apps/web/core/debates/playback-utils.ts +++ b/apps/web/core/debates/playback-utils.ts @@ -277,8 +277,22 @@ export type PlayBothOptions = { * whether `play()` resolved. `play()` can resolve while the element is still transitioning out of * `paused`, so checking `paused` on the very next microtask reports a block on a video that plays * a moment later — which is why the feed showed "Could not play both videos" on essentially every - * scroll while the recordings played fine. A rejected `play()` needs no special case: a rejection - * leaves the element paused, so it fails the same check. + * scroll while the recordings played fine. + * + * **But a rejected `play()` does need a special case, and assuming otherwise is GEO-2978.** This + * comment used to end "a rejection leaves the element paused, so it fails the same check". It does + * not, or not in time: `play()` sets `paused` to false *synchronously* and only then rejects, and + * the user agent pauses it again afterwards. So the first confirm poll can see both elements + * un-paused and report success for a play the browser was in the middle of refusing. + * + * Measured on an iPhone in Low Power Mode, which refuses every autoplay: the card reported + * `playing=true` with both elements `PAUSED` at `t=0.0` and `calls=[play REJECTED:NotAllowedError]`. + * From there nothing recovers — the autoplay effect will not retry a card it believes is playing, + * no refusal is recorded, and the paused glyph never renders. The viewer's first tap only pauses + * what the app imagined was running, which is why it took two taps to start a video. + * + * So a refusal now invalidates the reading: whatever `paused` said mid-flight, the browser has + * told us plainly that it did not start. * * The grace window is deliberately short. It only has to outlast the paused -> playing transition, * and every millisecond of it delays the muted retry on a genuine block. @@ -300,17 +314,60 @@ async function bothRunning( return !primary.paused && !secondary.paused; } +/** + * Whether a rejected `play()` was the browser declining, rather than us + * interrupting. + * + * Real engines name it; the message is checked too because a rejection that + * crosses a boundary can arrive as a plain `Error` carrying the same sentence. + */ +function isRefusal(reason: unknown): boolean { + if (!(reason instanceof Error)) return false; + + return reason.name === 'NotAllowedError' || /notallowed|does not allow|user agent/i.test(reason.message); +} + export async function playBothWithMutedFallback( primary: PlayableVideo, secondary: PlayableVideo, { wait = defaultWait, isCancelled }: PlayBothOptions = {} ): Promise { - const attempt = async () => { - await Promise.allSettled([primary.play(), secondary.play()]); - return bothRunning(primary, secondary, wait); + /** + * Whether the browser said no, as opposed to simply not having started yet. + * + * `play()` rejects for two quite different reasons and they need telling + * apart: `NotAllowedError` is a policy answer — no gesture, or iOS in Low + * Power Mode — while an `AbortError` is our own `pause()` interrupting the + * attempt. Only the first is a fact about the device. + */ + const attempt = async (): Promise<{ running: boolean; refused: boolean }> => { + const settled = await Promise.allSettled([primary.play(), secondary.play()]); + const refused = settled.some(result => result.status === 'rejected' && isRefusal(result.reason)); + + return { running: await bothRunning(primary, secondary, wait), refused }; }; - if (await attempt()) return 'playing'; + const first = await attempt(); + + if (first.running && !first.refused) return 'playing'; + + /* + * A refusal outranks the cancellation check below, and that ordering is the + * whole fix for GEO-2978. + * + * `resumeBoth` bumps its generation on entry, and its caller re-enters while + * `playing` is false — so by the time this returns, `isCancelled` is routinely + * true simply because the *next* attempt has started. Reporting 'cancelled' + * there threw away the browser's answer, the next attempt threw away its own, + * and the card never learned it had been refused: no play control, no error, + * just a still frame. Measured against the preview with `play()` forced to + * reject — zero play controls on six cards. + * + * Only when there is nothing left to try. With audio still on, the muted retry + * below is the thing that usually turns a refusal into playback, and skipping + * it to report early would lose real playback to protect a flag. + */ + if (first.refused && primary.muted && secondary.muted) return 'blocked'; // Someone paused these, or scrolled them off screen, while the confirm above was polling. The // retry would start them again — and the caller checking ownership after this returns cannot @@ -333,5 +390,8 @@ export async function playBothWithMutedFallback( // `DebateFeedPlayer` re-asserts it when `isResuming` falls. primary.muted = true; secondary.muted = true; - return (await attempt()) ? 'playing-muted' : 'blocked'; + + const retry = await attempt(); + + return retry.running && !retry.refused ? 'playing-muted' : 'blocked'; } diff --git a/apps/web/core/debates/use-debate-playback.test.tsx b/apps/web/core/debates/use-debate-playback.test.tsx index 36120506d6..dc78fe808e 100644 --- a/apps/web/core/debates/use-debate-playback.test.tsx +++ b/apps/web/core/debates/use-debate-playback.test.tsx @@ -227,6 +227,93 @@ describe('useDebatePlayback — an interrupted resume must not report failure (G expect(slot2.paused).toBe(true); }); + /** + * A refusal is a control, not an error (GEO-2978). + * + * Measured on a phone with the diagnostic readout: `play()` comes back + * `NotAllowedError` with both elements muted, inline and fully buffered — + * iOS in Low Power Mode, or with auto-play turned off for the site. Neither + * is a fault, and the videos are fine. + * + * The card used to answer that with "Could not play both videos. Try Play + * again." and no play button, because `showControls` reads `userPaused` and + * nobody had paused. The viewer's first tap only revealed the control and + * the second started it — the two-tap sequence this was reported as. + */ + it('offers the play control when the browser refuses to autoplay', async () => { + const { result, slot1, slot2 } = await mounted(); + + await act(async () => { + void result.current.resumeBoth(); + await Promise.resolve(); + // What iOS answers: refused outright, nothing superseded the attempt. + slot1.rejectPlay(); + slot2.rejectPlay(); + await new Promise(resolve => setTimeout(resolve, 400)); + }); + + expect(result.current.autoplayBlocked).toBe(true); + expect(result.current.playing).toBe(false); + // And no alarming copy about a failure that did not happen. + expect(result.current.error).toBeNull(); + }); + + /** + * The case that actually happens, and the reason the first fix did nothing. + * + * Every entry to `resumeBoth` bumps the generation, and the autoplay effect + * re-enters while `playing` is false — so an attempt is routinely superseded + * before it reaches the outcome. The refusal is a fact about the device rather + * than about the attempt, so it has to survive that; otherwise each attempt + * discards its own answer and the next one asks again. + */ + it('records a refusal even when a newer resume supersedes the attempt', async () => { + const { result, slot1, slot2 } = await mounted(); + + await act(async () => { + void result.current.resumeBoth(); + await Promise.resolve(); + slot1.rejectPlay(); + slot2.rejectPlay(); + // A second activation lands inside the first one's confirm window. + void result.current.resumeBoth(); + await Promise.resolve(); + slot1.rejectPlay(); + slot2.rejectPlay(); + await new Promise(resolve => setTimeout(resolve, 400)); + }); + + expect(result.current.autoplayBlocked).toBe(true); + }); + + /** + * The flag has to clear, or a card refused once stays refused for the session + * even after the viewer's tap — which is allowed, being a gesture. + */ + it('clears the refusal once playback actually starts', async () => { + const { result, slot1, slot2 } = await mounted(); + + await act(async () => { + void result.current.resumeBoth(); + await Promise.resolve(); + slot1.rejectPlay(); + slot2.rejectPlay(); + await new Promise(resolve => setTimeout(resolve, 400)); + }); + expect(result.current.autoplayBlocked).toBe(true); + + await act(async () => { + void result.current.resumeBoth(); + await Promise.resolve(); + slot1.settlePlay(); + slot2.settlePlay(); + await new Promise(resolve => setTimeout(resolve, 400)); + }); + + expect(result.current.autoplayBlocked).toBe(false); + expect(result.current.playing).toBe(true); + }); + /** The positive control: an uninterrupted resume still reports playback. */ it('reports playing when nothing interrupts it', async () => { const { result, slot1, slot2 } = await mounted(); @@ -337,21 +424,31 @@ describe('useDebatePlayback — an interrupted resume must not report failure (G expect(slot2.playbackRate).not.toBe(1); }); - /** And the guard must not swallow a real block — the autoplay-policy error still surfaces. */ - it('still surfaces a genuine failure to start', async () => { + /** + * And the guard must not swallow a real block — it still reaches the viewer, but as a + * control rather than as a sentence. + * + * This asserted an error message until GEO-2978, on the reasoning that a refusal is + * something the viewer needs to be told. The reasoning was right and the rendering was not: + * `showControls` reads `userPaused`, so a refused card offered no play button, and the + * message — "Could not play both videos. Try Play again." — named a Play control that was + * not on screen and a failure that had not happened. Measured on a phone, the videos are + * muted, inline and fully buffered; iOS simply wants to be asked by a person. + */ + it('turns a genuine block into a play control rather than an error', async () => { const { result, slot1, slot2 } = await mounted(); await act(async () => { void result.current.resumeBoth(); await Promise.resolve(); - // The browser refuses to start them and nothing superseded the attempt, so the viewer - // does need to be told. + // The browser refuses to start them and nothing superseded the attempt. slot1.rejectPlay(); slot2.rejectPlay(); await new Promise(resolve => setTimeout(resolve, 400)); }); - expect(result.current.error).not.toBeNull(); + expect(result.current.autoplayBlocked).toBe(true); + expect(result.current.error).toBeNull(); expect(result.current.playing).toBe(false); }); diff --git a/apps/web/core/debates/use-debate-playback.ts b/apps/web/core/debates/use-debate-playback.ts index 9ce9a88d9a..1b0574c77c 100644 --- a/apps/web/core/debates/use-debate-playback.ts +++ b/apps/web/core/debates/use-debate-playback.ts @@ -109,6 +109,22 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { const [error, setError] = React.useState(null); const [playing, setPlaying] = React.useState(false); const [userPaused, setUserPaused] = React.useState(false); + /** + * The browser refused to start this pair, so the viewer has to. + * + * Distinct from `userPaused`, which is a decision somebody made. This is a + * decision made *for* them — and measured on a phone rather than guessed at + * (GEO-2978): `play()` comes back `NotAllowedError` with the elements muted, + * inline and fully buffered, which is iOS in Low Power Mode, or with + * auto-play turned off for the site. Both are ordinary states a reader can be + * in, not faults. + * + * It drives the same two things `userPaused` does — show the play control, + * and stop the autoplay effect trying again — because a refusal that keeps + * being retried is a refusal every time, and the viewer's tap is the one + * thing that will be allowed. + */ + const [autoplayBlocked, setAutoplayBlocked] = React.useState(false); const [isScrubbing, setIsScrubbing] = React.useState(false); const isScrubbingRef = React.useRef(false); const wasPlayingBeforeScrubRef = React.useRef(false); @@ -606,13 +622,41 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { // thing noticed from inside, and is spelled out rather than left to the generation check so // that a future caller cannot accidentally read it as a successful start. if (outcome === 'cancelled') return; + + /* + * A refusal outlives the attempt that discovered it. + * + * It describes the device — this browser will not autoplay right now — so + * it is recorded above the ownership check, which exists to stop a + * superseded attempt writing *playback* state. And superseded is the norm + * rather than the exception: `resumeBoth` bumps the generation on entry + * and the autoplay effect re-enters while `playing` is false, so an + * attempt is routinely overtaken before it reports. Recorded below the + * check, the refusal was discarded every time and the card never learned + * of it. + * + * It also ends that loop, because the effect reads the flag. + */ + if (outcome === 'blocked') setAutoplayBlocked(true); + if (resumeGenerationRef.current !== generation) return; + if (outcome === 'blocked') { + /* + * The elements and the playback state belong to *this* attempt, so they + * stay under the ownership check — pausing elements a newer resume has + * started would undo it. + * + * No error copy. This used to say "Could not play both videos. Try Play + * again.", which named a control that was not on screen and a failure + * that had not happened: the videos are fine and the device simply wants + * to be asked by a person. `autoplayBlocked` puts that question on the + * card instead. + */ primaryVideo.pause(); secondaryVideo.pause(); setPlaying(false); setTurnState(null); - setError('Could not play both videos. Try Play again.'); return; } // The browser only allowed it muted (GEO-2783) — record that so the unmute control is honest @@ -621,6 +665,8 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { if (outcome === 'playing-muted') setMutedByUser(true); setPlaying(true); setUserPaused(false); + // Whatever refused last time has stopped refusing. + setAutoplayBlocked(false); }, [offsets, seekVideosTo, setMutedByUser, timelineSeconds] ); @@ -840,6 +886,7 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { error, playing, userPaused, + autoplayBlocked, isScrubbing, isResuming, playbackEnded, From 1cfbeee3570b4183f1df41ba289a59f1f7bc82e4 Mon Sep 17 00:00:00 2001 From: Preston Mantel Date: Sat, 19 Sep 2026 15:24:12 -0700 Subject: [PATCH 2/8] feat(debates): playback diagnostics, behind a flag (GEO-2978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Autoplay faults in the feed do not reproduce anywhere a debugger is attached. Headless Chromium plays every card, and headless WebKit — the engine iOS runs — reports the clock advancing and `play()` resolving, because neither carries iOS's media policy, Low Power Mode, or a toolbar that resizes the viewport as you scroll. Four fixes for GEO-2978 went in against remote probes and none of them touched the fault; one reading from the phone identified it exactly. So this asks the phone. With the flag on, a readout reports every debate video actually on screen — how much is shown, paused or playing, its clock, ready state, muted and inline — plus the sequence of `play()`/`pause()` calls against it and how each settled, and the player's own conclusion beside it. `copy` puts the text on the clipboard, because the call trace is the part a screenshot loses. That separates causes which are identical on screen: nothing ever called `play()`; the browser refused; or it started and something took it back. The fault this found was the fourth — a card reporting `playing=true` over a refused `play()`. Visibility is measured on **both axes**. A vertical-only test counts a card scrolled out sideways in a row as on screen, which is how an earlier round came to believe twelve videos were playing at once and spent a day on a decoder limit that was never reached. Co-Authored-By: Claude Opus 5 (1M context) --- apps/web/app/entry.tsx | 2 + .../web/core/debates/playback-diagnostics.tsx | 193 ++++++++++++++++++ apps/web/core/state/feature-flags.test.tsx | 3 + apps/web/core/state/feature-flags.ts | 11 + .../feature-flags-dialog.test.tsx | 1 + 5 files changed, 210 insertions(+) create mode 100644 apps/web/core/debates/playback-diagnostics.tsx diff --git a/apps/web/app/entry.tsx b/apps/web/app/entry.tsx index 7fe33fdd99..892702f7ae 100644 --- a/apps/web/app/entry.tsx +++ b/apps/web/app/entry.tsx @@ -9,6 +9,7 @@ import dynamic from 'next/dynamic'; import { DebateCoordinator } from '~/core/debates/debate-coordinator'; import { DebateMediaSessionProvider } from '~/core/debates/media-session'; +import { PlaybackDiagnostics } from '~/core/debates/playback-diagnostics'; import { DebateRecordingUploadCoordinator } from '~/core/debates/recording-upload-coordinator'; import { useGeoLogoutCleanup } from '~/core/hooks/use-geo-logout'; import { useKeyboardShortcuts } from '~/core/hooks/use-keyboard-shortcuts'; @@ -135,6 +136,7 @@ export function App({ children }: { children: React.ReactNode }) {
+ {/* Client-side rendered due to `window.localStorage` usage */} diff --git a/apps/web/core/debates/playback-diagnostics.tsx b/apps/web/core/debates/playback-diagnostics.tsx new file mode 100644 index 0000000000..a3dad4c168 --- /dev/null +++ b/apps/web/core/debates/playback-diagnostics.tsx @@ -0,0 +1,193 @@ +'use client'; + +import * as React from 'react'; + +import { usePlaybackDiagnosticsEnabled } from '~/core/state/feature-flags'; + +/** + * Why a debate video on screen is not playing, answered on the device it is not + * playing on (GEO-2978). + * + * Autoplay faults here do not reproduce anywhere a debugger is attached. + * Headless Chromium plays every card, and headless WebKit — the engine iOS runs + * — reports the clock advancing and `play()` resolving, because neither carries + * iOS's media policy, Low Power Mode, or a toolbar that resizes the viewport as + * you scroll. The phone is the only witness, so this asks it. + * + * It separates causes that look identical on screen: + * + * * `calls=[]` — nothing asked it to play, so the fault is in whatever decides + * a card is active. + * * `calls=[play REJECTED:…]` — the browser refused. + * * `calls=[play ok]` with `PAUSED`, or playing with a frozen `t` — something + * took it back, or it cannot decode. + * + * Read the app's own conclusion beside them: a card reporting `playing=true` + * over a refused `play()` is the pair that produced GEO-2978, where a + * synchronous un-pause inside `play()` was mistaken for a successful start. + */ +type Trace = string[]; + +const traces = new WeakMap(); +let patched = false; + +/** + * Records every `play()` and `pause()` against the element it happened to. + * + * Patched once on the prototype rather than per element: the cards mount and + * unmount as the feed scrolls, and an element that was never wrapped is exactly + * the one whose silence needs explaining. + */ +function patchMediaElement() { + if (patched || typeof HTMLMediaElement === 'undefined') return; + patched = true; + + const note = (element: HTMLMediaElement, entry: string) => { + const trace = traces.get(element) ?? []; + trace.push(entry); + traces.set(element, trace.slice(-6)); + }; + + const originalPlay = HTMLMediaElement.prototype.play; + const originalPause = HTMLMediaElement.prototype.pause; + + HTMLMediaElement.prototype.play = function patchedPlay(this: HTMLMediaElement) { + note(this, 'play'); + const result = originalPlay.call(this); + + if (result && typeof result.then === 'function') { + result.then( + () => note(this, 'ok'), + (error: unknown) => note(this, `REJECTED:${error instanceof Error ? error.name : 'unknown'}`) + ); + } + + return result; + }; + + HTMLMediaElement.prototype.pause = function patchedPause(this: HTMLMediaElement) { + note(this, 'pause'); + return originalPause.call(this); + }; +} + +/** + * How much of an element is on screen, as a fraction of itself. + * + * **Both axes.** A card scrolled out sideways in a horizontal row is not on + * screen, and a vertical-only test calls it visible — which is exactly how a + * gallery's off-screen cards came to be counted as playing and sent an + * investigation after a decoder limit that was never reached. + */ +function shownFraction(element: Element) { + const box = element.getBoundingClientRect(); + if (box.width === 0 || box.height === 0) return 0; + + const vertical = Math.max(0, Math.min(box.bottom, window.innerHeight) - Math.max(box.top, 0)); + const horizontal = Math.max(0, Math.min(box.right, window.innerWidth) - Math.max(box.left, 0)); + + return (vertical * horizontal) / (box.width * box.height); +} + +const isReallyVisible = (element: Element) => shownFraction(element) > 0; + +function describe(video: HTMLVideoElement, index: number) { + const shown = shownFraction(video); + const calls = traces.get(video) ?? []; + + return [ + `v${index}`, + `${Math.round(shown * 100)}% shown`, + video.paused ? 'PAUSED' : 'playing', + `t=${video.currentTime.toFixed(1)}`, + `ready=${video.readyState}`, + video.muted ? 'muted' : 'AUDIBLE', + video.playsInline ? 'inline' : 'NOT-INLINE', + video.error ? `err=${video.error.code}` : null, + `calls=[${calls.join(' ')}]`, + ] + .filter(Boolean) + .join(' · '); +} + +export function PlaybackDiagnostics() { + const enabled = usePlaybackDiagnosticsEnabled(); + const [lines, setLines] = React.useState([]); + const [copied, setCopied] = React.useState(false); + + React.useEffect(() => { + if (!enabled) return; + + patchMediaElement(); + + const read = () => { + const all = [...document.querySelectorAll('video')]; + const visible = all.filter(isReallyVisible); + + /* + * The player's own conclusion, read off the attributes it publishes. + * + * The trace above says what the browser did; this says what the app made + * of it, and the gap between the two is where these faults live. + */ + const players = [...document.querySelectorAll('[data-debate-ready]')] + .filter(isReallyVisible) + .slice(0, 2) + .map( + (player, index) => + `card${index} · ready=${player.getAttribute('data-debate-ready')}` + + ` active=${player.getAttribute('data-debate-active')}` + + ` playing=${player.getAttribute('data-debate-playing')}` + + ` blocked=${player.getAttribute('data-debate-autoplay-blocked')}` + + ` playBtn=${player.querySelectorAll('[aria-label="Resume debate"]').length}` + ); + + setLines([ + `${all.length} video(s) on the page, ${visible.length} on screen, ${all.filter(v => !v.paused).length} playing`, + ...(players.length > 0 ? players : ['no debate player on screen']), + ...visible.slice(0, 4).map((video, index) => describe(video, index)), + // A video running where nobody can see it is its own fault and worth + // naming separately. + ...all + .filter(video => !video.paused && !isReallyVisible(video)) + .slice(0, 2) + .map((video, index) => `off-screen but playing #${index} · t=${video.currentTime.toFixed(1)}`), + ]); + }; + + read(); + const interval = window.setInterval(read, 500); + + return () => window.clearInterval(interval); + }, [enabled]); + + if (!enabled) return null; + + const text = lines.join('\n'); + + return ( +
+
+ playback diagnostics + +
+ {lines.map(line => ( +
{line}
+ ))} +
+ ); +} diff --git a/apps/web/core/state/feature-flags.test.tsx b/apps/web/core/state/feature-flags.test.tsx index 88a1f68ae7..fc582aec1d 100644 --- a/apps/web/core/state/feature-flags.test.tsx +++ b/apps/web/core/state/feature-flags.test.tsx @@ -28,6 +28,7 @@ describe('feature flags', () => { expect(defaultFeatureFlags.exploreSidePanel).toBe(false); expect(defaultFeatureFlags.bountiesTab).toBe(true); expect(normalizeFeatureFlags(null)).toEqual({ + playbackDiagnostics: false, debugDebatesPage: false, debateDebugging: false, debateFormatSelector: false, @@ -41,6 +42,7 @@ describe('feature flags', () => { // reaching the dialog would render a checkbox for a flag nothing reads. it('drops the retired claims-and-debates flags that are still in storage', () => { expect(normalizeFeatureFlags({ questionsTab: true, debatesTab: true, debateDebugging: true })).toEqual({ + playbackDiagnostics: false, debugDebatesPage: false, debateDebugging: true, debateFormatSelector: false, @@ -65,6 +67,7 @@ describe('feature flags', () => { // serialize in is incidental — it follows the definition list, and pinning it here would fail // on a reordering that changes nothing a reader could notice. expect(JSON.parse(window.localStorage.getItem(featureFlagsStorageKey) ?? 'null')).toEqual({ + playbackDiagnostics: false, debugDebatesPage: true, debateDebugging: true, debateFormatSelector: true, diff --git a/apps/web/core/state/feature-flags.ts b/apps/web/core/state/feature-flags.ts index 0864d6f925..e80f7604ed 100644 --- a/apps/web/core/state/feature-flags.ts +++ b/apps/web/core/state/feature-flags.ts @@ -20,6 +20,13 @@ export const featureFlagDefinitions = [ description: 'Allow the first matched debater to choose a format before accepting.', enabledByDefault: false, }, + { + id: 'playbackDiagnostics', + label: 'Playback diagnostics', + description: + 'Pin a readout to the bottom of the screen showing, for every debate video on screen, whether play() was called and what happened to it. For autoplay faults that only happen on a real phone (GEO-2978).', + enabledByDefault: false, + }, { id: 'debugDebatesPage', label: 'Debates debug tab per space', @@ -111,6 +118,10 @@ export function useDebugDebatesPageEnabled() { return useFeatureFlag('debugDebatesPage'); } +export function usePlaybackDiagnosticsEnabled() { + return useFeatureFlag('playbackDiagnostics'); +} + /** * Read *and* write, for the flags dialog. Deliberately not hydration-gated like * {@link useFeatureFlag}: the dialog's contents are inside a Radix `Root` that is closed until a diff --git a/apps/web/partials/feature-flags/feature-flags-dialog.test.tsx b/apps/web/partials/feature-flags/feature-flags-dialog.test.tsx index 0e2ceba8ee..a27eccf70f 100644 --- a/apps/web/partials/feature-flags/feature-flags-dialog.test.tsx +++ b/apps/web/partials/feature-flags/feature-flags-dialog.test.tsx @@ -57,6 +57,7 @@ describe('FeatureFlagsDialog', () => { await waitFor(() => { // Values, not key order — see the note in `feature-flags.test.ts`. expect(JSON.parse(window.localStorage.getItem(featureFlagsStorageKey) ?? 'null')).toEqual({ + playbackDiagnostics: false, debugDebatesPage: true, debateDebugging: true, debateFormatSelector: true, From d8d79f939cf2f7e27726329e2ef50b7ca8f826af Mon Sep 17 00:00:00 2001 From: Preston Mantel Date: Sat, 19 Sep 2026 15:42:06 -0700 Subject: [PATCH 3/8] fix(debates): tell a refusal apart from a start that never confirmed (GEO-2978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults in the fix, both from folding the browser's "no" together with everything else that fails to start. A refusal is now its own outcome. `playBothWithMutedFallback` returns 'blocked' for any start it cannot confirm inside the ~300ms grace window — a stalled buffer, a recording that 404s, a decode failure — and the previous commit latched every one of those as an autoplay refusal. That stops the feed's autoplay effect retrying (it reads the flag) and, with the old error copy gone, leaves a stalled card behind a tap control that does nothing and says nothing. 'refused' latches and offers the tap; 'blocked' keeps its message and stays retryable. And a stale refusal can no longer contradict a newer attempt that won. The refusal is recorded above the ownership check on purpose — attempts overlap by construction, so requiring ownership discarded it every time — but that also lets an attempt answer after a later one has started the video. Guarded on the generation that last confirmed playback, so the only refusal dropped is one a success has already overtaken. Without the guard the card draws the tap control over a playing video and the tap stops it, which is the two-tap symptom this whole change removes. Both are covered by tests that fail without their fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../debates/browse/debate-feed-player.tsx | 18 ++--- .../web/core/debates/playback-diagnostics.tsx | 14 +++- apps/web/core/debates/playback-utils.test.ts | 37 ++++++--- apps/web/core/debates/playback-utils.ts | 18 ++++- .../core/debates/use-debate-playback.test.tsx | 76 +++++++++++++++++++ apps/web/core/debates/use-debate-playback.ts | 43 +++++++---- 6 files changed, 171 insertions(+), 35 deletions(-) diff --git a/apps/web/core/debates/browse/debate-feed-player.tsx b/apps/web/core/debates/browse/debate-feed-player.tsx index 7baefdf209..d10e1e649d 100644 --- a/apps/web/core/debates/browse/debate-feed-player.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.tsx @@ -84,6 +84,15 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb seekBothRaw(seconds); }; + /** + * Stopped, and only a tap will start it. + * + * The two ways in are different facts — the viewer paused, or the browser + * refused — and identical from here: the video is not running and the control + * is the only answer either accepts. + */ + const awaitingTap = userPaused || autoplayBlocked; + // Autoplay the debate that's in view; pause the rest. Respect an explicit // user pause so scrolling back doesn't fight the viewer, and don't resume // mid-scrub. @@ -98,15 +107,6 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb } }, [active, autoplayBlocked, isScrubbing, playbackEnded, playing, ready, resumeBoth, suspend, userPaused]); - /** - * Stopped, and only a tap will start it. - * - * The two ways in are different facts — the viewer paused, or the browser - * refused — and identical from here: the video is not running and the control - * is the only answer either accepts. - */ - const awaitingTap = userPaused || autoplayBlocked; - const showControls = ready && (awaitingTap || (playbackEnded && !hasVoted)); // End of an unvoted debate offers a replay; a stopped one shows the paused glyph. const showReplay = ready && playbackEnded && !hasVoted; diff --git a/apps/web/core/debates/playback-diagnostics.tsx b/apps/web/core/debates/playback-diagnostics.tsx index a3dad4c168..b8b0efdd3a 100644 --- a/apps/web/core/debates/playback-diagnostics.tsx +++ b/apps/web/core/debates/playback-diagnostics.tsx @@ -36,7 +36,9 @@ let patched = false; * * Patched once on the prototype rather than per element: the cards mount and * unmount as the feed scrolls, and an element that was never wrapped is exactly - * the one whose silence needs explaining. + * the one whose silence needs explaining. Never unpatched — turning the flag off + * leaves the wrapper in place until the next page load, which costs a push to a + * `WeakMap` per call and keeps the trace intact for a reading taken right after. */ function patchMediaElement() { if (patched || typeof HTMLMediaElement === 'undefined') return; @@ -115,6 +117,16 @@ export function PlaybackDiagnostics() { const [lines, setLines] = React.useState([]); const [copied, setCopied] = React.useState(false); + // The confirmation has to expire, or the button reads "copied" over a readout that has moved on + // since — and this one repaints every 500ms. + React.useEffect(() => { + if (!copied) return; + + const timeout = window.setTimeout(() => setCopied(false), 1_500); + + return () => window.clearTimeout(timeout); + }, [copied]); + React.useEffect(() => { if (!enabled) return; diff --git a/apps/web/core/debates/playback-utils.test.ts b/apps/web/core/debates/playback-utils.test.ts index 4d410ebed0..3bb5c13429 100644 --- a/apps/web/core/debates/playback-utils.test.ts +++ b/apps/web/core/debates/playback-utils.test.ts @@ -499,12 +499,6 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { expect(b.muted).toBe(true); }); - /** - * The retry is the only point where this function starts something it did not start, and it is - * reached after a confirm window it spent asleep — so a pause or a scroll-away routinely lands - * in between. A caller that checks ownership only once this returns is too late: `play()` has - * already been called, and no state check can take it back. - */ /** * The element iOS actually gives you when it refuses (GEO-2978). * @@ -536,7 +530,7 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { const a = refusingVideo(); const b = refusingVideo(); - expect(await playBothWithMutedFallback(a, b)).toBe('blocked'); + expect(await playBothWithMutedFallback(a, b)).toBe('refused'); }); /** @@ -558,7 +552,7 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { return video; }; - expect(await playBothWithMutedFallback(stuck(), stuck())).toBe('blocked'); + expect(await playBothWithMutedFallback(stuck(), stuck())).toBe('refused'); }); /** @@ -578,7 +572,26 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { }; b.play = a.play.bind(b); - expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('blocked'); + expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('refused'); + }); + + /** + * A start that never confirms is not a refusal, and the difference is the caller's whole + * behaviour: 'refused' latches a tap control and stops the autoplay effect retrying, so + * reporting it for a stall would leave a buffering card behind a dead button with nothing to + * say for itself. + */ + it('reports a stalled start as blocked, not refused', async () => { + // Resolves, never un-pauses: a stall, a missing recording, a decode failure. + const stalled = () => { + const video = fakeVideo({ muted: true, blockUnmuted: false }); + video.play = async () => { + video.plays += 1; + }; + return video; + }; + + expect(await playBothWithMutedFallback(stalled(), stalled())).toBe('blocked'); }); /** But an interruption of ours is still a cancellation, not a refusal. */ @@ -594,6 +607,12 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('cancelled'); }); + /** + * The retry is the only point where this function starts something it did not start, and it is + * reached after a confirm window it spent asleep — so a pause or a scroll-away routinely lands + * in between. A caller that checks ownership only once this returns is too late: `play()` has + * already been called, and no state check can take it back. + */ it('does not retry when the attempt was cancelled while confirming', async () => { const a = fakeVideo({ muted: false }); const b = fakeVideo({ muted: false }); diff --git a/apps/web/core/debates/playback-utils.ts b/apps/web/core/debates/playback-utils.ts index 125904fd2e..0c0253f2e9 100644 --- a/apps/web/core/debates/playback-utils.ts +++ b/apps/web/core/debates/playback-utils.ts @@ -240,7 +240,17 @@ export function speakerLabel(participant: Pick & { play: () => Promise }; -export type PlayBothOutcome = 'playing' | 'playing-muted' | 'blocked' | 'cancelled'; +/** + * `'refused'` and `'blocked'` are both "it did not start", and the difference between them is + * whether trying again could ever work. + * + * `'refused'` is the browser's answer — `NotAllowedError`, autoplay policy, iOS in Low Power Mode + * — and it will be the same answer to the same question, so the only way forward is a control the + * viewer taps. `'blocked'` is everything else that failed to confirm: a stalled buffer, a missing + * recording, a decode failure. Those are worth retrying and worth saying out loud, so folding them + * into `'refused'` would leave a card sitting silently behind a play button that does nothing. + */ +export type PlayBothOutcome = 'playing' | 'playing-muted' | 'refused' | 'blocked' | 'cancelled'; export type PlayBothOptions = { /** Injectable so tests do not wait on real timers. */ @@ -367,7 +377,7 @@ export async function playBothWithMutedFallback( * below is the thing that usually turns a refusal into playback, and skipping * it to report early would lose real playback to protect a flag. */ - if (first.refused && primary.muted && secondary.muted) return 'blocked'; + if (first.refused && primary.muted && secondary.muted) return 'refused'; // Someone paused these, or scrolled them off screen, while the confirm above was polling. The // retry would start them again — and the caller checking ownership after this returns cannot @@ -393,5 +403,7 @@ export async function playBothWithMutedFallback( const retry = await attempt(); - return retry.running && !retry.refused ? 'playing-muted' : 'blocked'; + if (retry.running && !retry.refused) return 'playing-muted'; + + return first.refused || retry.refused ? 'refused' : 'blocked'; } diff --git a/apps/web/core/debates/use-debate-playback.test.tsx b/apps/web/core/debates/use-debate-playback.test.tsx index dc78fe808e..146c41f378 100644 --- a/apps/web/core/debates/use-debate-playback.test.tsx +++ b/apps/web/core/debates/use-debate-playback.test.tsx @@ -171,6 +171,24 @@ function fakeVideo() { browserResume() { video.paused = false; }, + /** Resolve the play() without the element ever starting — a stall, or a recording that 404s. */ + stallPlay() { + video.pending?.resolve(); + video.pending = null; + }, + /** + * Take the in-flight play() aside and hand back the way to refuse it later. + * + * A real element only tracks its newest `play()`, and so does this one. Detaching is how a + * test can start a second attempt over the first and still let the first answer afterwards, + * which is the ordering that matters here: attempts overlap by design, and the later answer + * is not always the later attempt's. + */ + detachPlay() { + const detached = video.pending; + video.pending = null; + return () => detached?.reject(new Error('play() failed because the user agent does not allow it')); + }, }; return video as unknown as HTMLVideoElement & { plays: number; @@ -178,6 +196,8 @@ function fakeVideo() { rejectPlay: () => void; browserPause: () => void; browserResume: () => void; + stallPlay: () => void; + detachPlay: () => () => void; }; } @@ -314,6 +334,62 @@ describe('useDebatePlayback — an interrupted resume must not report failure (G expect(result.current.playing).toBe(true); }); + /** + * The other side of letting a refusal outlive its attempt (GEO-2978). + * + * Recording it above the ownership check is what makes it reach the card at all, but it also + * lets a stale attempt speak after a newer one has won. Here the refused attempt answers last, + * with the video already running — and if that answer latched, the card would draw the tap + * control over a playing video and the tap would stop it, which is the symptom this whole + * change exists to remove. + */ + it('does not let a stale refusal contradict a newer resume that started', async () => { + const { result, slot1, slot2 } = await mounted(); + + await act(async () => { + void result.current.resumeBoth(); + await Promise.resolve(); + // Hold the first attempt's play() aside so the second can run over it. + const refuseFirst1 = slot1.detachPlay(); + const refuseFirst2 = slot2.detachPlay(); + + void result.current.resumeBoth(); + await Promise.resolve(); + slot1.settlePlay(); + slot2.settlePlay(); + await new Promise(resolve => setTimeout(resolve, 400)); + + // Only now does the browser answer the attempt it was asked first. + refuseFirst1(); + refuseFirst2(); + await new Promise(resolve => setTimeout(resolve, 400)); + }); + + expect(result.current.playing).toBe(true); + expect(result.current.autoplayBlocked).toBe(false); + }); + + /** + * A start that never confirms is not the browser refusing, and the two cannot share an + * outcome: a refusal latches the tap control and stops the autoplay effect retrying, so a + * stalled card would sit behind a button that does nothing, with no reason given. + */ + it('keeps the error, and the retry, for a start that stalls rather than being refused', async () => { + const { result, slot1, slot2 } = await mounted(); + + await act(async () => { + void result.current.resumeBoth(); + await Promise.resolve(); + slot1.stallPlay(); + slot2.stallPlay(); + await new Promise(resolve => setTimeout(resolve, 400)); + }); + + expect(result.current.playing).toBe(false); + expect(result.current.autoplayBlocked).toBe(false); + expect(result.current.error).toBe('Could not play both videos. Try Play again.'); + }); + /** The positive control: an uninterrupted resume still reports playback. */ it('reports playing when nothing interrupts it', async () => { const { result, slot1, slot2 } = await mounted(); diff --git a/apps/web/core/debates/use-debate-playback.ts b/apps/web/core/debates/use-debate-playback.ts index 1b0574c77c..89440dc752 100644 --- a/apps/web/core/debates/use-debate-playback.ts +++ b/apps/web/core/debates/use-debate-playback.ts @@ -174,6 +174,15 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { * - `setUserPaused(false)` could erase a pause the viewer made during the await. */ const resumeGenerationRef = React.useRef(0); + /** + * The newest attempt that confirmed playback, so an older one cannot contradict it. + * + * `resumeGenerationRef` alone cannot settle this. It says which attempt *owns* the elements now, + * and the refusal above it is recorded precisely because that ownership has usually moved on by + * the time an attempt reports. This says which attempt last won, which is the only thing that + * makes a stale refusal safe to drop. + */ + const playingGenerationRef = React.useRef(0); /** * How many resumes are still confirming. * @@ -636,33 +645,41 @@ export function useDebatePlayback(debate: Debate, enabled: boolean) { * of it. * * It also ends that loop, because the effect reads the flag. + * + * The one thing that does overrule it is a later attempt that actually + * started. A refusal reported after that attempt won would put the tap + * control over a running video, and the tap would stop it — the two-tap + * symptom this was written to remove. */ - if (outcome === 'blocked') setAutoplayBlocked(true); + if (outcome === 'refused' && playingGenerationRef.current < generation) setAutoplayBlocked(true); if (resumeGenerationRef.current !== generation) return; - if (outcome === 'blocked') { - /* - * The elements and the playback state belong to *this* attempt, so they - * stay under the ownership check — pausing elements a newer resume has - * started would undo it. - * - * No error copy. This used to say "Could not play both videos. Try Play - * again.", which named a control that was not on screen and a failure - * that had not happened: the videos are fine and the device simply wants - * to be asked by a person. `autoplayBlocked` puts that question on the - * card instead. - */ + if (outcome === 'refused' || outcome === 'blocked') { + // The elements and the playback state belong to *this* attempt, so they stay under the + // ownership check — pausing elements a newer resume has started would undo it. primaryVideo.pause(); secondaryVideo.pause(); setPlaying(false); setTurnState(null); + /* + * Only the unexplained failure says so out loud. + * + * 'blocked' is a stall, a missing recording, a decode failure — the viewer is owed both a + * reason and a retry, and the autoplay effect keeps retrying because nothing latched. + * 'refused' is the browser declining, where this copy was actively wrong: it named a + * control that was not on screen and a failure that had not happened. The videos are fine; + * the device simply wants to be asked by a person, and `autoplayBlocked` puts that + * question on the card as a control instead of a sentence. + */ + if (outcome === 'blocked') setError('Could not play both videos. Try Play again.'); return; } // The browser only allowed it muted (GEO-2783) — record that so the unmute control is honest // and later autoplays stop being blocked the same way. The viewer's next tap is a gesture and // will be allowed. if (outcome === 'playing-muted') setMutedByUser(true); + playingGenerationRef.current = generation; setPlaying(true); setUserPaused(false); // Whatever refused last time has stopped refusing. From f08e796616822a680b7964cd94a96ae30f252e7e Mon Sep 17 00:00:00 2001 From: Preston Mantel Date: Sat, 19 Sep 2026 16:09:47 -0700 Subject: [PATCH 4/8] fix(debates): classify a refused play() by its name, never its message (GEO-2978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isRefusal` fell back to matching the rejection's message, and a phrase broad enough to catch WebKit's "not allowed by the user agent" also catches Chrome interrupting with the user agent in the sentence. Since an interruption here is almost always our own `pause()` — a scroll-away, a scrub — that read an ordinary interaction as a policy refusal, latched the tap control and stopped the card autoplaying for the rest of the session. The failure this change exists to remove, reintroduced by the fix for it. Thanks @copilot. The name is the whole classification. `play()` rejects with a `DOMException` and the spec names it: `NotAllowedError` for a refusal, `AbortError` for an interruption. The message is prose an engine writes for a human, worded differently per engine and overlapping across meanings. Checking the name also meant fixing how it is read, and that is the larger half. The guard was `instanceof Error`, which `social-video-share.ts` already documents as wrong for exactly this: `DOMException` is not reliably an instance of `Error`, and a guard written that way once cost us a share failure that reached us as a person reading the string aloud. Under it, every genuine `NotAllowedError` from `play()` was classified as *not* a refusal — the opposite error, in the same predicate. jsdom demonstrates it: reverting the predicate fails seven tests that now reject the way a browser does. So `errorName` moves out of `social-video-share.ts` — a 'use client' module with React and analytics in it, which `playback-utils` cannot import — into `core/utils/error-name.ts`, and the three places that read an error's name for classification use it: `isRefusal`, `isAbortError`, and the diagnostic's call trace, which had the same `instanceof Error` guard and would have reported the refusal that identified this bug as `unknown`. Test fixtures now reject with a real `DOMException` rather than a plain `Error` carrying invented prose. The old fixtures only classified because of the message fallback — a test shaping production code to match itself. Co-Authored-By: Claude Opus 5 (1M context) --- .../web/core/debates/playback-diagnostics.tsx | 5 +- apps/web/core/debates/playback-utils.test.ts | 16 +++-- apps/web/core/debates/playback-utils.ts | 20 ++++-- apps/web/core/debates/social-video-share.ts | 20 +----- .../core/debates/use-debate-playback.test.tsx | 69 ++++++++++++++++++- apps/web/core/utils/error-name.test.ts | 30 ++++++++ apps/web/core/utils/error-name.ts | 21 ++++++ 7 files changed, 147 insertions(+), 34 deletions(-) create mode 100644 apps/web/core/utils/error-name.test.ts create mode 100644 apps/web/core/utils/error-name.ts diff --git a/apps/web/core/debates/playback-diagnostics.tsx b/apps/web/core/debates/playback-diagnostics.tsx index b8b0efdd3a..04420fe6c0 100644 --- a/apps/web/core/debates/playback-diagnostics.tsx +++ b/apps/web/core/debates/playback-diagnostics.tsx @@ -3,6 +3,7 @@ import * as React from 'react'; import { usePlaybackDiagnosticsEnabled } from '~/core/state/feature-flags'; +import { errorName } from '~/core/utils/error-name'; /** * Why a debate video on screen is not playing, answered on the device it is not @@ -60,7 +61,9 @@ function patchMediaElement() { if (result && typeof result.then === 'function') { result.then( () => note(this, 'ok'), - (error: unknown) => note(this, `REJECTED:${error instanceof Error ? error.name : 'unknown'}`) + // Structurally, not behind `instanceof Error`: `play()` rejects with a `DOMException`, + // and reporting the refusal that matters as `unknown` would waste the whole reading. + (error: unknown) => note(this, `REJECTED:${errorName(error)}`) ); } diff --git a/apps/web/core/debates/playback-utils.test.ts b/apps/web/core/debates/playback-utils.test.ts index 3bb5c13429..d845725a6b 100644 --- a/apps/web/core/debates/playback-utils.test.ts +++ b/apps/web/core/debates/playback-utils.test.ts @@ -520,7 +520,7 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { await Promise.resolve(); // And the user agent takes it back. video.paused = true; - throw Object.assign(new Error('refused'), { name: 'NotAllowedError' }); + throw new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); }, }; return video; @@ -546,7 +546,7 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { async play() { video.plays += 1; video.paused = false; - throw Object.assign(new Error('refused'), { name: 'NotAllowedError' }); + throw new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); }, }; return video; @@ -568,7 +568,7 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { const b = fakeVideo({ muted: true, blockUnmuted: false }); a.play = async function refuse() { a.plays += 1; - throw Object.assign(new Error('refused'), { name: 'NotAllowedError' }); + throw new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); }; b.play = a.play.bind(b); @@ -594,13 +594,19 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { expect(await playBothWithMutedFallback(stalled(), stalled())).toBe('blocked'); }); - /** But an interruption of ours is still a cancellation, not a refusal. */ + /** + * But an interruption of ours is still a cancellation, not a refusal — and the wording is the + * point. Classification used to fall back to matching the message, where a phrase broad enough + * for WebKit's "not allowed by the user agent" also caught an interruption that named the user + * agent. That turns an ordinary pause or scroll-away into a latched refusal, which stops the + * card autoplaying for the rest of the session. Only the `name` separates the two. + */ it('still reports a cancellation when our own pause interrupted the attempt', async () => { const a = fakeVideo({ muted: true, blockUnmuted: false }); const b = fakeVideo({ muted: true, blockUnmuted: false }); a.play = async function abort() { a.plays += 1; - throw Object.assign(new Error('interrupted by a call to pause()'), { name: 'AbortError' }); + throw new DOMException('The play() request was interrupted by the user agent.', 'AbortError'); }; b.play = a.play.bind(b); diff --git a/apps/web/core/debates/playback-utils.ts b/apps/web/core/debates/playback-utils.ts index 0c0253f2e9..87c835c4d7 100644 --- a/apps/web/core/debates/playback-utils.ts +++ b/apps/web/core/debates/playback-utils.ts @@ -1,3 +1,5 @@ +import { errorName } from '~/core/utils/error-name'; + import type { Debate, DebateMediaResponse, DebateMediaTurnSegment, DebateParticipant, ParticipantSlot } from './api'; export type TurnState = { @@ -325,16 +327,20 @@ async function bothRunning( } /** - * Whether a rejected `play()` was the browser declining, rather than us - * interrupting. + * Whether a rejected `play()` was the browser declining, rather than us interrupting. + * + * On the name only. `play()` rejects with a `DOMException` and the spec names it: `NotAllowedError` + * for a policy refusal, `AbortError` for an interruption — which, here, is almost always our own + * `pause()`. Matching the *message* instead conflates them, because the engines do not agree on + * wording and their phrases overlap: WebKit refuses with "not allowed by the user agent", while + * Chrome interrupts with "the play() request was interrupted", and a substring broad enough to + * catch the first catches interruptions that mention the user agent too. Calling one of those a + * refusal latches the tap control and stops autoplay after an ordinary scroll. * - * Real engines name it; the message is checked too because a rejection that - * crosses a boundary can arrive as a plain `Error` carrying the same sentence. + * `errorName` rather than `instanceof Error`, for the reason documented there. */ function isRefusal(reason: unknown): boolean { - if (!(reason instanceof Error)) return false; - - return reason.name === 'NotAllowedError' || /notallowed|does not allow|user agent/i.test(reason.message); + return errorName(reason) === 'NotAllowedError'; } export async function playBothWithMutedFallback( diff --git a/apps/web/core/debates/social-video-share.ts b/apps/web/core/debates/social-video-share.ts index eb8d04fce9..1829bce426 100644 --- a/apps/web/core/debates/social-video-share.ts +++ b/apps/web/core/debates/social-video-share.ts @@ -3,6 +3,7 @@ import * as React from 'react'; import { capture } from '~/core/analytics'; +import { errorName } from '~/core/utils/error-name'; import { useDebateMediaArtifactUrl } from './hooks'; @@ -330,7 +331,7 @@ export async function downloadSocialVideo( } export function isAbortError(error: unknown): boolean { - return typeof error === 'object' && error !== null && 'name' in error && error.name === 'AbortError'; + return errorName(error) === 'AbortError'; } /** @@ -348,23 +349,6 @@ export function isUnretryableShareError(error: unknown): boolean { return UNRETRYABLE_SHARE_ERROR_NAMES.has(errorName(error)); } -/** - * The error's own name, for telemetry. - * - * Read structurally rather than behind `instanceof Error`, which is what every call site here used - * to do — and `DOMException` is **not** an instance of `Error`. Since the Web Share API and the - * fetch abort path both reject with `DOMException`, that guard reported every one of them as - * `UnknownError`: the share failure Preston hit arrived as a person telling us the string, because - * `NotAllowedError` never reached the event. `isAbortError` above already reads `.name` this way, - * which is why cancellation detection worked while the reporting beside it did not. - */ -export function errorName(error: unknown): string { - if (typeof error === 'object' && error !== null && 'name' in error && typeof error.name === 'string') { - return error.name; - } - return 'UnknownError'; -} - export function captureSocialVideoEvent(eventName: string, properties: Record) { try { capture(eventName, properties); diff --git a/apps/web/core/debates/use-debate-playback.test.tsx b/apps/web/core/debates/use-debate-playback.test.tsx index 146c41f378..6570a2e939 100644 --- a/apps/web/core/debates/use-debate-playback.test.tsx +++ b/apps/web/core/debates/use-debate-playback.test.tsx @@ -108,6 +108,23 @@ describe('useDebatePlayback — playback URLs survive re-activation (GEO-2895)', }); }); +/** + * The rejection WebKit gives for a refused autoplay, in the shape it gives it (GEO-2978). + * + * A `DOMException` rather than an `Error`, because that is what `play()` rejects with and the two + * are not interchangeable to a guard written as `instanceof Error`. Classification reads the + * `name`; the message is here only so that a fixture matching on prose would be seen to be wrong. + */ +const refusal = () => + new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); + +/** + * And the rejection our own `pause()` produces, which must never read as a refusal — note that + * Chrome's wording for it mentions the user agent too. + */ +const interruption = () => + new DOMException('The play() request was interrupted by the user agent.', 'AbortError'); + /** * A fake