diff --git a/apps/web/app/entry.tsx b/apps/web/app/entry.tsx index a33dcd72f6..5e962e0f19 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'; @@ -137,6 +138,7 @@ export function App({ children }: { children: React.ReactNode }) { + {/* Client-side rendered due to `window.localStorage` usage */} 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 af727929af..97b63c8aba 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, @@ -83,22 +84,33 @@ 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. 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. + 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; // Clicking the video briefly flashes the action it just took — feedback only, not a control. const [flash, setFlash] = React.useState<{ icon: 'play' | 'pause'; visible: boolean }>({ @@ -120,7 +132,24 @@ export function DebateFeedPlayer({ debate, active, preload = false, votes }: Deb }; return ( -
+
+ ({ + top: box.top, + left: box.left, + right: box.left + box.width, + bottom: box.top + box.height, + width: box.width, + height: box.height, + x: box.left, + y: box.top, + toJSON: () => ({}), + }) as DOMRect; +} + +export const ON_SCREEN = { top: 10, left: 0, width: 300, height: 200 }; +export const OFF_TO_THE_SIDE = { top: 10, left: 5_000, width: 300, height: 200 }; + +/** A debate card as `DebateFeedPlayer` publishes it: the state attributes, with its videos inside. */ +export function debateCard(state: { playing: boolean; blocked: boolean }) { + const card = document.createElement('div'); + card.setAttribute('data-debate-ready', 'true'); + card.setAttribute('data-debate-active', 'true'); + card.setAttribute('data-debate-playing', String(state.playing)); + card.setAttribute('data-debate-autoplay-blocked', String(state.blocked)); + placeAt(card, ON_SCREEN); + + const video = document.createElement('video'); + placeAt(video, ON_SCREEN); + card.append(video); + + return { card, video }; +} diff --git a/apps/web/core/debates/playback-diagnostics-install.test.tsx b/apps/web/core/debates/playback-diagnostics-install.test.tsx new file mode 100644 index 0000000000..227253824e --- /dev/null +++ b/apps/web/core/debates/playback-diagnostics-install.test.tsx @@ -0,0 +1,82 @@ +import '@testing-library/jest-dom/vitest'; +import { cleanup, render, screen, waitFor } from '@testing-library/react'; + +import * as React from 'react'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { featureFlagsStorageKey } from '~/core/state/feature-flags'; + +import { PlaybackDiagnostics } from './playback-diagnostics'; +import { debateCard } from './playback-diagnostics-fixtures'; + +/* + * Its own file, and that is the point rather than an accident of organisation. + * + * `patchMediaElement` installs once per module instance and never uninstalls, so any earlier test + * in the same file leaves the prototype already patched — and a test for *when* the patch arrives + * would then pass without it. Vitest gives each file a fresh module, which is the only thing that + * makes this assertion mean anything. + */ +vi.mock('~/core/state/feature-flags', async importOriginal => ({ + ...(await importOriginal()), + usePlaybackDiagnosticsEnabled: () => true, +})); + +const originalPlay = HTMLMediaElement.prototype.play; + +beforeEach(() => { + window.innerWidth = 400; + window.innerHeight = 800; + // The install reads the persisted flag directly rather than through the hydration-gated hook, + // because that hook is false for the first commit by design. + window.localStorage.setItem(featureFlagsStorageKey, JSON.stringify({ playbackDiagnostics: true })); + HTMLMediaElement.prototype.play = () => + Promise.reject(new DOMException('not allowed by the user agent', 'NotAllowedError')); +}); + +afterEach(() => { + cleanup(); + HTMLMediaElement.prototype.play = originalPlay; + window.localStorage.removeItem(featureFlagsStorageKey); + document.querySelectorAll('[data-debate-ready], video').forEach(node => node.remove()); +}); + +describe('PlaybackDiagnostics — the trace is installed before the effects it observes', () => { + /** + * The probe has to be in place before the playback it watches, or it reports a negative that + * cannot be told apart from a real one. + * + * `usePlaybackDiagnosticsEnabled` is hydration-gated and returns `false` for the first commit by + * design, so patching on it left the trace to a passive effect racing `DebateFeedPlayer`'s own. + * Lose that race and the readout says `calls=[]` — which does not mean "not recorded", it means + * "nothing ever asked this to play": a different fault, in a different part of the code, and + * exactly the wrong signpost this file exists to remove. + * + * A sibling's passive effect stands in for the card's, mounted *ahead* of the diagnostic so that + * a passive install loses. Layout effects run during commit, before every passive effect in the + * tree, which is what makes the ordering a guarantee rather than a race this usually wins. + */ + it('records a play() from a passive effect that mounts ahead of it', async () => { + const { card, video } = debateCard({ playing: false, blocked: false }); + document.body.append(card); + + function PlaysOnMount() { + React.useEffect(() => { + void video.play().catch(() => {}); + }, []); + return null; + } + + render( + <> + + + + ); + + await waitFor(() => expect(screen.getByText(/calls=\[play REJECTED:NotAllowedError/)).toBeInTheDocument(), { + timeout: 3_000, + }); + }); +}); diff --git a/apps/web/core/debates/playback-diagnostics.test.tsx b/apps/web/core/debates/playback-diagnostics.test.tsx new file mode 100644 index 0000000000..0d2fb2d25c --- /dev/null +++ b/apps/web/core/debates/playback-diagnostics.test.tsx @@ -0,0 +1,149 @@ +import '@testing-library/jest-dom/vitest'; +import { cleanup, render, screen, waitFor } from '@testing-library/react'; + +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { featureFlagsStorageKey } from '~/core/state/feature-flags'; + +import { PlaybackDiagnostics } from './playback-diagnostics'; +import { OFF_TO_THE_SIDE, ON_SCREEN, debateCard, placeAt } from './playback-diagnostics-fixtures'; + +/* + * `usePlaybackDiagnosticsEnabled` is mocked rather than `useFeatureFlag`: the component calls the + * former, and the former calls the latter through the module's own binding, which a mocked export + * does not intercept. + */ +vi.mock('~/core/state/feature-flags', async importOriginal => ({ + ...(await importOriginal()), + usePlaybackDiagnosticsEnabled: () => true, +})); + +/** + * jsdom has no media stack, so `play()` is unimplemented there. This stands in with the rejection + * a refusing browser gives — and it has to be installed before the first render, because the + * diagnostic patches the prototype once and captures whatever `play` it finds at that moment. + */ +const originalPlay = HTMLMediaElement.prototype.play; + +beforeAll(() => { + HTMLMediaElement.prototype.play = () => + Promise.reject(new DOMException('not allowed by the user agent', 'NotAllowedError')); +}); + +afterAll(() => { + HTMLMediaElement.prototype.play = originalPlay; +}); + +beforeEach(() => { + window.innerWidth = 400; + window.innerHeight = 800; + // The patch reads the persisted flag directly rather than through the hydration-gated hook, so + // the stored value is what decides whether it installs. + window.localStorage.setItem(featureFlagsStorageKey, JSON.stringify({ playbackDiagnostics: true })); +}); + +afterEach(() => { + cleanup(); + window.localStorage.removeItem(featureFlagsStorageKey); + // The cards are appended to the body rather than rendered, so `cleanup` does not reach them. + document.querySelectorAll('[data-debate-ready], video').forEach(node => node.remove()); +}); + +describe('PlaybackDiagnostics', () => { + /** + * The readout's whole purpose is pairing a card's conclusion with what its videos did, and it + * used to build those as two independent lists lined up by position. Explore opens with a hero + * carousel above the feed, so the video described beside `card0` was regularly one the card had + * never touched — a wrong measurement in the instrument built to stop wrong measurements. + */ + it('describes only the videos inside a debate card, never unrelated media', () => { + const stray = document.createElement('video'); + placeAt(stray, ON_SCREEN); + // First in document order, which is exactly where the hero carousel sits. + document.body.append(stray); + + const { card } = debateCard({ playing: false, blocked: true }); + document.body.append(card); + + render(); + + // The card's own video is described, and attributed to the card by name rather than by index. + expect(screen.getByText(/card0\.v0/)).toBeInTheDocument(); + // The stray is counted, so it cannot be silently dropped, but never described as a card's. + expect(screen.getByText(/1 other video\(s\) on screen, outside any debate card/)).toBeInTheDocument(); + expect(screen.queryByText(/^\s*v0 ·/)).not.toBeInTheDocument(); + }); + + /** Two cards on screen keep their own videos rather than sharing one numbering. */ + it('keeps each card with its own video', () => { + const first = debateCard({ playing: true, blocked: false }); + const second = debateCard({ playing: false, blocked: true }); + document.body.append(first.card, second.card); + + render(); + + expect(screen.getByText(/card0 · ready=true.*playing=true.*blocked=false/)).toBeInTheDocument(); + expect(screen.getByText(/card1 · ready=true.*playing=false.*blocked=true/)).toBeInTheDocument(); + expect(screen.getByText(/card0\.v0/)).toBeInTheDocument(); + expect(screen.getByText(/card1\.v0/)).toBeInTheDocument(); + }); + + /** + * Every visible card, not the first two. + * + * Both the PR and the feature-flag description promise a reading of every debate video on + * screen, and the panel truncated after two. A device reading that quietly omits the third card + * can be wrong about which card is misbehaving — the same wrong-measurement failure this file + * was written to end, and one that reads as confidently as a correct one. + */ + it('reports every visible card, not the first two', () => { + const cards = [ + debateCard({ playing: true, blocked: false }), + debateCard({ playing: false, blocked: false }), + debateCard({ playing: false, blocked: true }), + ]; + document.body.append(...cards.map(entry => entry.card)); + + render(); + + expect(screen.getByText(/card0 ·/)).toBeInTheDocument(); + expect(screen.getByText(/card1 ·/)).toBeInTheDocument(); + // The one that used to fall off the end — and the only one reporting a refusal. + expect(screen.getByText(/card2 · .*blocked=true/)).toBeInTheDocument(); + expect(screen.getByText(/card2\.v0/)).toBeInTheDocument(); + }); + + /** + * Visibility is tested on both axes. A vertical-only test counts a card scrolled out sideways + * in a horizontal row as on screen, which is how an earlier round of this investigation came to + * believe twelve videos were playing at once and spent a day on a decoder limit never reached. + */ + it('does not count a card scrolled out sideways as on screen', () => { + const { card, video } = debateCard({ playing: false, blocked: false }); + placeAt(card, OFF_TO_THE_SIDE); + placeAt(video, OFF_TO_THE_SIDE); + document.body.append(card); + + render(); + + expect(screen.getByText('no debate player on screen')).toBeInTheDocument(); + }); + + /** + * The line that identified GEO-2978: a card claiming to be playing over a `play()` the browser + * refused. Reporting how the call settled, and not only what `paused` says, is the whole reason + * the readout could tell a refusal apart from a card nothing ever asked to play. + */ + it('records how each play() settled against the element it happened to', async () => { + const { card, video } = debateCard({ playing: true, blocked: false }); + document.body.append(card); + + render(); + + await video.play().catch(() => {}); + + await waitFor(() => expect(screen.getByText(/calls=\[.*REJECTED:NotAllowedError/)).toBeInTheDocument(), { + timeout: 3_000, + }); + }); +}); diff --git a/apps/web/core/debates/playback-diagnostics.tsx b/apps/web/core/debates/playback-diagnostics.tsx new file mode 100644 index 0000000000..ae25ffb802 --- /dev/null +++ b/apps/web/core/debates/playback-diagnostics.tsx @@ -0,0 +1,247 @@ +'use client'; + +import * as React from 'react'; + +import { useIsomorphicLayoutEffect } from '~/core/hooks/use-isomorphic-layout-effect'; +import { readStoredFeatureFlag, 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 + * 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. 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; + 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'), + // 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)}`) + ); + } + + 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; + +const DEBATE_PLAYER_SELECTOR = '[data-debate-ready]'; + +function describe(video: HTMLVideoElement, label: string) { + const shown = shownFraction(video); + const calls = traces.get(video) ?? []; + + return [ + label, + `${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); + + // 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]); + + /* + * The trace is installed before any playback effect can run, and deliberately not on `enabled`. + * + * `enabled` comes from a hydration-gated read, so it is `false` for the first commit by design. + * Waiting for it would leave the patch to a passive effect that runs alongside `DebateFeedPlayer`'s + * own — and if the card gets there first, its `play()` is never recorded. The readout would then + * say `calls=[]`, which does not mean "not recorded": it means "nothing ever asked this to play", + * a different fault in a different part of the code. Sending this investigation somewhere it did + * not need to go is the one thing this file exists to prevent. + * + * So the flag is read straight from storage, in a layout effect. Layout effects run during commit, + * ahead of every passive effect in the tree, which is what makes the ordering a guarantee rather + * than a race this usually wins. Only the patch runs early; the panel stays hydration-gated. + */ + useIsomorphicLayoutEffect(() => { + if (readStoredFeatureFlag('playbackDiagnostics')) patchMediaElement(); + }, []); + + React.useEffect(() => { + if (!enabled) return; + + // Idempotent, and this is the path that matters when the flag is switched on mid-session. + patchMediaElement(); + + const read = () => { + const all = [...document.querySelectorAll('video')]; + const visible = all.filter(isReallyVisible); + // Every visible card, not the first two. The panel scrolls inside its own height limit, and + // a reading that quietly omits the third card on screen is a reading that can be wrong about + // which card is misbehaving — the failure this whole file exists to prevent. + const players = [...document.querySelectorAll(DEBATE_PLAYER_SELECTOR)].filter(isReallyVisible); + + /* + * Each card reports its own videos, underneath it. + * + * The conclusion and the videos it is about used to be two lists built independently and + * lined up by position — `card0` beside `v0`, which was whatever came first in the + * document. Explore opens with a hero carousel above the feed, and any entity page can + * carry video of its own, so `v0` was regularly some unrelated element while `card0` + * described a debate below it. That reads as a card playing a video it has never touched, + * and this readout exists precisely because wrong measurements sent this investigation + * after causes that were never there. + * + * So the pairing is structural now: a card's videos are the ones inside it. + */ + const cards = players.flatMap((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}`, + ...[...player.querySelectorAll('video')].map((video, slot) => ` ${describe(video, `card${index}.v${slot}`)}`), + ]); + + // Media the debate feed does not own still matters — it competes for the same decoders — + // but it is counted, not described, so it can never be mistaken for a card's own video. + const strays = visible.filter(video => video.closest(DEBATE_PLAYER_SELECTOR) === null); + const offScreenButPlaying = all.filter(video => !video.paused && !isReallyVisible(video)); + + setLines([ + `${all.length} video(s) on the page, ${visible.length} on screen, ${all.filter(v => !v.paused).length} playing`, + ...(cards.length > 0 ? cards : ['no debate player on screen']), + ...(strays.length > 0 + ? [ + `${strays.length} other video(s) on screen, outside any debate card (${strays.filter(v => !v.paused).length} playing)`, + ] + : []), + // A video running where nobody can see it is its own fault and worth naming separately. + ...offScreenButPlaying.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, index) => ( + // Keyed by position: this is a readout regenerated whole every 500ms, and two cards in + // the same state produce byte-identical rows. +
{line}
+ ))} +
+ ); +} diff --git a/apps/web/core/debates/playback-utils.test.ts b/apps/web/core/debates/playback-utils.test.ts index 9fd34db830..88c94ac87d 100644 --- a/apps/web/core/debates/playback-utils.test.ts +++ b/apps/web/core/debates/playback-utils.test.ts @@ -499,6 +499,183 @@ describe('playBothWithMutedFallback (GEO-2783)', () => { expect(b.muted).toBe(true); }); + /** + * 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 new DOMException('The request is not allowed by the user agent or the platform.', '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('refused'); + }); + + /** + * 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 new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); + }, + }; + return video; + }; + + expect(await playBothWithMutedFallback(stuck(), stuck())).toBe('refused'); + }); + + /** + * 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 new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); + }; + b.play = a.play.bind(b); + + expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('refused'); + }); + + /** + * The muted retry's verdict is the one that counts, and only it. + * + * A refused *unmuted* request is the ordinary case — it is why the muted retry exists — so + * carrying that refusal into the final answer reported 'refused' for whatever the retry then + * did. Here the retry stalls instead of being refused: a recording that will not decode, on a + * device perfectly willing to autoplay it muted. Reporting a refusal latches the tap control, + * drops the retry, and withholds the message, and the tap achieves nothing. + */ + it('reports a muted retry that stalls as blocked, even though the unmuted request was refused', async () => { + const refusedThenStalled = () => { + const video = fakeVideo({ muted: false }); + video.play = async () => { + video.plays += 1; + // Muted now, which is the retry: the browser is willing, the media is not. + if (video.muted) return; + throw new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); + }; + return video; + }; + + expect(await playBothWithMutedFallback(refusedThenStalled(), refusedThenStalled())).toBe('blocked'); + }); + + /** And a retry the browser refuses in its own right is still a refusal. */ + it('reports a refusal when the muted retry is refused too', async () => { + const alwaysRefuses = () => { + const video = fakeVideo({ muted: false }); + video.play = async () => { + video.plays += 1; + throw new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); + }; + return video; + }; + + expect(await playBothWithMutedFallback(alwaysRefuses(), alwaysRefuses())).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'); + }); + + /** + * An unmuted refusal waits for the muted retry, even when that costs the answer (GEO-2783). + * + * Pinned because the reasoning lives in a caller. A refusal on an unmuted pair means only "not + * unmuted" — the ordinary case the muted fallback exists for — so reporting 'refused' would + * latch a tap control on a card that plays perfectly well muted. Cancelled, there is no retry + * left to ask, so the answer is genuinely lost for this attempt; it survives because the last + * attempt in an overlap chain is not cancelled and starts from a stopped card, where both + * elements are muted and the branch above fires. + */ + it('reports a cancellation, not a refusal, when the refused pair still had audio to give up', async () => { + const a = fakeVideo({ muted: false }); + const b = fakeVideo({ muted: false }); + a.play = async function refuse(this: { plays: number }) { + this.plays += 1; + throw new DOMException('The request is not allowed by the user agent or the platform.', 'NotAllowedError'); + }; + b.play = a.play.bind(b); + + expect(await playBothWithMutedFallback(a, b, { isCancelled: () => true })).toBe('cancelled'); + // And crucially it did not start anything while cancelled: one call each, no muted retry. + expect(a.plays).toBe(1); + expect(b.plays).toBe(1); + }); + + /** + * 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 new DOMException('The play() request was interrupted by the user agent.', 'AbortError'); + }; + b.play = a.play.bind(b); + + 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 diff --git a/apps/web/core/debates/playback-utils.ts b/apps/web/core/debates/playback-utils.ts index 6bbba3ad3a..fa12ba9148 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 = { @@ -240,7 +242,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. */ @@ -277,8 +289,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 +326,75 @@ async function bothRunning( return !primary.paused && !secondary.paused; } +/** + * 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. + * + * `errorName` rather than `instanceof Error`, for the reason documented there. + */ +function isRefusal(reason: unknown): boolean { + return errorName(reason) === 'NotAllowedError'; +} + 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 — on the autoplay path, which is the one + * GEO-2978 is about. + * + * `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. + * + * **The mute condition is what scopes this to autoplay, and it is deliberate.** A card the feed + * is starting is stopped, and `DebateFeedPlayer` derives `muted` as `!audible || mutedByUser` + * with `audible` requiring `playing` — so both elements are muted and this check is the one that + * fires. A resume of a pair that was already *playing* — `endScrub`, or the return from a + * backgrounded tab — has the speaking element unmuted, and there a refusal means only "not + * unmuted", which is the ordinary case GEO-2783 exists for. 'refused' is a statement about the + * device, and latching it on a card that would play perfectly well muted would take the retry + * away from a card that deserves one, so it is not reported until the muted retry has answered. + * + * The consequence, named because it is a real cost rather than an oversight: an unmuted refusal + * that is also cancelled returns 'cancelled' and the browser's answer is lost for that attempt. + * It survives because the last attempt in an overlap chain is by definition not cancelled, and + * that attempt starts from a stopped card with both elements muted — so it takes this branch. + * One attempt's delay, not a lost answer. Pinned by a test, since it rests on reasoning about a + * caller rather than on anything visible here. + */ + 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 @@ -333,5 +417,24 @@ 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(); + + if (retry.running && !retry.refused) return 'playing-muted'; + + /* + * The muted retry has the only verdict that describes it. + * + * A refused *unmuted* request is the ordinary case — it is why the muted retry exists at all — + * and carrying `first.refused` down here reported a refusal for whatever the retry did next. A + * muted attempt that stalls, on a recording that 404s or will not decode, is not the browser + * declining: the caller would latch the tap control and drop both the retry and the message a + * stall is owed, and the tap would achieve nothing. + * + * Refusal still outranks cancellation, for the reason given above the first check. + */ + if (retry.refused) return 'refused'; + if (isCancelled?.()) return 'cancelled'; + + return 'blocked'; } 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 5000d0269a..e14b6add37 100644 --- a/apps/web/core/debates/use-debate-playback.test.tsx +++ b/apps/web/core/debates/use-debate-playback.test.tsx @@ -143,6 +143,22 @@ 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