From 90c37663ad91858d2abf42716945a64bee670e33 Mon Sep 17 00:00:00 2001 From: ghostcoder42 Date: Sun, 11 Oct 2026 16:08:13 +0800 Subject: [PATCH] fix(post): single-flight open guard, focus-gated autoplay, autoplay race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - single-flight guard for opening the post screen: rapid repeat taps on a card no longer stack multiple post screens (each with its own native player layering audio). The guard covers exactly the in-flight window — push until the pushed screen's transition ends (blur cleanup is the fallback unlock) — with no timer; once focused, the same video opens again immediately. - autoplay is gated on screen focus: a screen stacked under another — or whose detail query resolves long after the user backed out — never starts playing when its source lands late. The resume-on-refocus behavior is kept. - the play intent rides player construction + a readyToPlay re-assert: useVideoPlayer recreates the whole native player when the source lands, and a play() fired right after that recreation can race the native prepare and be silently dropped — the video then sat paused forever. Manual pause still wins over the retry. - retry: 0 on the video detail query: one 12s timeout then the error state with a retry button, instead of ~48s of spinner on a dead network that reads as a freeze. --- src/api/video-queries.ts | 4 ++ src/app/post/[id].source.test.tsx | 3 ++ src/app/post/[id].test.tsx | 3 ++ src/app/post/[id].tsx | 87 +++++++++++++++++++++++++++++- src/components/video-tile.test.tsx | 39 ++++++++++++++ src/components/video-tile.tsx | 5 ++ src/lib/navigation/open-guard.ts | 29 ++++++++++ 7 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 src/lib/navigation/open-guard.ts diff --git a/src/api/video-queries.ts b/src/api/video-queries.ts index a8dc16f..1e8a59a 100644 --- a/src/api/video-queries.ts +++ b/src/api/video-queries.ts @@ -62,6 +62,10 @@ export const useVideoDetail = createQuery ({ Stack: { Screen: () => null }, useLocalSearchParams: jest.fn().mockReturnValue({ id: 'v1', slug: 'my-video' }), useFocusEffect: (_cb: () => (() => void) | undefined) => undefined, + useNavigation: () => ({ + addListener: jest.fn(() => jest.fn()), + }), })); const mockPlayer = { diff --git a/src/app/post/[id].test.tsx b/src/app/post/[id].test.tsx index 3ac59fa..4470fbe 100644 --- a/src/app/post/[id].test.tsx +++ b/src/app/post/[id].test.tsx @@ -11,6 +11,9 @@ jest.mock('expo-router', () => ({ useFocusEffect: (cb: () => (() => void) | undefined) => { mockEffectCallback = cb; }, + useNavigation: () => ({ + addListener: jest.fn(() => jest.fn()), + }), })); const mockPlayer = { diff --git a/src/app/post/[id].tsx b/src/app/post/[id].tsx index ed6084c..496fa02 100644 --- a/src/app/post/[id].tsx +++ b/src/app/post/[id].tsx @@ -1,5 +1,5 @@ import { useEvent } from 'expo'; -import { Link, Stack, useFocusEffect, useLocalSearchParams } from 'expo-router'; +import { Link, Stack, useFocusEffect, useLocalSearchParams, useNavigation } from 'expo-router'; import { VideoView, useVideoPlayer } from 'expo-video'; import * as React from 'react'; import { ScrollView, TouchableOpacity, View } from 'react-native'; @@ -10,6 +10,7 @@ import { ActivityIndicator, Button, FocusAwareStatusBar, Text } from '@/componen import { useVideoDownload } from '@/lib/hooks'; import { useVideoAutoplay } from '@/lib/hooks/use-video-autoplay'; import { useTranslate } from '@/lib/i18n'; +import { endVideoOpen } from '@/lib/navigation/open-guard'; import { artistChipLabel } from '@/lib/r34/artists'; import { toOfflineDetail } from '@/lib/r34/offline-detail'; import type { VideoDetail, VideoFormat } from '@/lib/r34/types'; @@ -72,24 +73,77 @@ function VideoSection({ data }: VideoSectionProps): React.ReactElement { const { autoplayEnabled } = useVideoAutoplay(); + // Focus guard: a screen stacked under another one (rapid duplicate pushes, + // post→tag/author→post chains) — or one whose detail query resolves long + // after the user backed out — must never start playing. Its blur-time + // pause() ran before the stream source landed and was a no-op, so a late + // autoplay would layer audio under the focused screen. + const isFocusedRef = React.useRef(true); + const player = useVideoPlayer(videoSource, (player) => { player.loop = true; + // Autoplay intent belongs to the player's CONSTRUCTION, not to an effect + // afterwards: useVideoPlayer recreates the whole native player when the + // source lands (empty → url), and a play() fired right after that + // recreation can race the native prepare and be dropped — the video then + // sits paused forever (reproduced on hanime1's Android 8.1 device: paused + // overlay, no buffering, manual play starts instantly). Playing here, in + // the same tick as creation, gives the intent its earliest chance; the + // statusChange retry below covers the rest. + if (autoplayEnabled && videoSource && isFocusedRef.current) { + try { + player.play(); + } catch { + // Source not prepared yet — statusChange retry re-asserts autoplay. + } + } }); const { isPlaying } = useEvent(player, 'playingChange', { isPlaying: player.playing, }); + const { status } = useEvent(player, 'statusChange', { + status: player.status, + }); + + // Manual pause (while this screen is focused) must win over autoplay + // retries. A pause that arrives with focus already lost is the blur + // cleanup, not the user. `hasPlayed` keeps the initial paused state from + // counting as a user pause. + const userPausedRef = React.useRef(false); + const hasPlayedRef = React.useRef(false); + React.useEffect(() => { + if (isPlaying) { + userPausedRef.current = false; + hasPlayedRef.current = true; + } else if (hasPlayedRef.current && isFocusedRef.current) { + userPausedRef.current = true; + } + }, [isPlaying]); // Start playback when the page opens (and when the source changes, e.g. a // quality switch or the download taking over) — unless the user turned // autoplay off. Manual pausing is never overridden: the effect only runs // when the source or the setting itself changes. React.useEffect(() => { - if (autoplayEnabled && videoSource) { + if (autoplayEnabled && videoSource && isFocusedRef.current) { player.play(); } }, [autoplayEnabled, player, videoSource]); + // Compensation retry: even with construction-time intent, re-assert autoplay + // once the native player reports readiness — belt and suspenders for + // prepare-race paths that drop the initial intent (see setup comment). + React.useEffect(() => { + if (status !== 'readyToPlay' || !autoplayEnabled || userPausedRef.current) return; + if (!isFocusedRef.current) return; + try { + player.play(); + } catch { + // Player being released — nothing to do. + } + }, [status, autoplayEnabled, player]); + // The screen stays mounted in the stack after navigating away (e.g. to a // tag or another post), so the player keeps playing — and two posts pushed // on top of each other would play simultaneously. Pause whenever this @@ -99,10 +153,12 @@ function VideoSection({ data }: VideoSectionProps): React.ReactElement { const wasPlayingRef = React.useRef(false); useFocusEffect( React.useCallback(() => { + isFocusedRef.current = true; if (wasPlayingRef.current) { player.play(); } return () => { + isFocusedRef.current = false; try { wasPlayingRef.current = player.playing; player.pause(); @@ -238,6 +294,33 @@ export default function Post(): React.ReactElement | null { const data = networkData ?? (downloaded ? toOfflineDetail(downloaded, local.id) : undefined); const addHistory = useHistoryStore((s) => s.addHistory); + // Release the single-flight navigation guard when the OPEN truly completes. + // Focus fires on the first frame after the push — while the stack + // transition animation (~300ms) is still running, the source screen stays + // visible AND touchable, so taps landing inside that window could push the + // same video again (reproduced on hanime1: furious tapping stacked 2+ + // screens). The transition's end is the moment the card is finally covered. + const navigation = useNavigation(); + React.useEffect(() => { + // expo-router's navigation prop doesn't type native-stack's transition + // events, but the stack navigator emits them at runtime. + const transitionNav = navigation as unknown as { + addListener: (event: 'transitionEnd', cb: () => void) => () => void; + }; + const unsubscribe = transitionNav.addListener('transitionEnd', () => endVideoOpen()); + return unsubscribe; + }, [navigation]); + + // Fallback unlock: the user backed out mid-transition (open aborted), the + // screen blurred, or no transition event will fire for this navigator. + useFocusEffect( + React.useCallback(() => { + return () => { + endVideoOpen(); + }; + }, []) + ); + // Why the page info couldn't be refreshed, so the toast can tell the two // apart. A removed video shows up two ways: an HTTP 404/410, or a 200 // "shell" page (title only, no formats) that parses successfully. Anything diff --git a/src/components/video-tile.test.tsx b/src/components/video-tile.test.tsx index 711c4e2..0582781 100644 --- a/src/components/video-tile.test.tsx +++ b/src/components/video-tile.test.tsx @@ -1,3 +1,4 @@ +import { endVideoOpen, resetOpenGuard } from '@/lib/navigation/open-guard'; import { useDownloadedStore } from '@/lib/stores/downloaded-store'; import { useFavoritesStore } from '@/lib/stores/favorites-store'; import { cleanup, fireEvent, render } from '@/lib/test-utils'; @@ -60,6 +61,7 @@ beforeEach(() => { entries: [], loaded: false, }); + resetOpenGuard(); }); describe('VideoTile — rendering', () => { @@ -136,6 +138,43 @@ describe('VideoTile — gestures', () => { // the native menu — those are verified on device, not in jsdom. expect(mockPush).not.toHaveBeenCalled(); }); + + it('pushes only once for rapid repeat taps on the same card', () => { + const { getByTestId } = render(); + + for (let i = 0; i < 6; i++) { + fireEvent(getByTestId('video-tile'), 'press'); + } + + expect(mockPush).toHaveBeenCalledTimes(1); + }); + + it('opens the same video again once the pushed screen has focused', () => { + const { getByTestId } = render(); + + fireEvent(getByTestId('video-tile'), 'press'); + // The post screen reports focus — the open completed. + endVideoOpen(); + fireEvent(getByTestId('video-tile'), 'press'); + + expect(mockPush).toHaveBeenCalledTimes(2); + }); + + it('still navigates for a different card while another open is in flight', () => { + const other = { ...item, id: '2' }; + const { getAllByTestId } = render( + <> + + + + ); + + const tiles = getAllByTestId('video-tile'); + fireEvent(tiles[0], 'press'); + fireEvent(tiles[1], 'press'); + + expect(mockPush).toHaveBeenCalledTimes(2); + }); }); describe('VideoTile — context menu actions', () => { diff --git a/src/components/video-tile.tsx b/src/components/video-tile.tsx index 19691aa..14fb50b 100644 --- a/src/components/video-tile.tsx +++ b/src/components/video-tile.tsx @@ -5,6 +5,7 @@ import { Platform } from 'react-native'; import { Image, Pressable, Text, View } from '@/components/ui'; import { useVideoActions } from '@/lib/hooks/use-video-actions'; +import { tryBeginVideoOpen } from '@/lib/navigation/open-guard'; import { useActiveDownload } from '@/lib/stores/active-downloads-store'; /** @@ -87,6 +88,10 @@ function VideoTileImpl({ item, children }: Props): React.ReactElement { ]; const openDetail = () => { + // The push is synchronous, but while the open is still in flight (screen + // not yet focused) repeat taps on this card are swallowed — each extra + // push would stack another post screen with its own native player. + if (!tryBeginVideoOpen(item.id)) return; router.push({ pathname: '/post/[id]', params: { id: item.id, slug: item.slug } }); }; diff --git a/src/lib/navigation/open-guard.ts b/src/lib/navigation/open-guard.ts new file mode 100644 index 0000000..f311c53 --- /dev/null +++ b/src/lib/navigation/open-guard.ts @@ -0,0 +1,29 @@ +/** + * Single-flight guard for opening the post screen. + * + * `router.push` is a synchronous dispatch — the tap IS the open. The only + * window where a repeat tap can duplicate the push is while the open is + * still in flight (the pushed screen has not gained focus yet — e.g. taps + * queued during a JS-thread stall, or jabs landing during the transition). + * While that window is open, further opens of the SAME video are swallowed; + * the pushed screen unlocks the guard when it gains focus. No timers: the + * lock's lifetime is exactly the open operation. + */ +let openingVideoId: string | null = null; + +/** Returns true when this video's open may proceed; false = already opening. */ +export function tryBeginVideoOpen(videoId: string): boolean { + if (openingVideoId === videoId) return false; + openingVideoId = videoId; + return true; +} + +/** Called by the post screen when it gains focus — the open has completed. */ +export function endVideoOpen(): void { + openingVideoId = null; +} + +/** Test hook — clears any in-flight open. */ +export function resetOpenGuard(): void { + openingVideoId = null; +}