From 3a982c2a7a68339e8859e4464659a96328908bf8 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sat, 11 Jul 2026 12:57:40 +0200 Subject: [PATCH] fix(player): prevent duplicate back actions --- lib/screens/video_player/parts/build.dart | 4 -- lib/screens/video_player_screen.dart | 8 +-- lib/widgets/overlay_sheet.dart | 12 ++-- .../video_controls/parts/key_events.dart | 65 ++++++------------- .../video_controls/video_controls.dart | 9 --- .../companion_remote_receiver_test.dart | 6 +- test/widgets/overlay_sheet_test.dart | 47 ++++++++++++++ 7 files changed, 81 insertions(+), 70 deletions(-) diff --git a/lib/screens/video_player/parts/build.dart b/lib/screens/video_player/parts/build.dart index d16fa304..b62390c1 100644 --- a/lib/screens/video_player/parts/build.dart +++ b/lib/screens/video_player/parts/build.dart @@ -287,10 +287,6 @@ extension _VideoPlayerBuildMethods on VideoPlayerScreenState { onPlayPauseRequested: () => _playOrPauseWithPlaybackIntent(player!), onSeekCompleted: _notifyWatchTogetherSeek, onBack: _handleBackButton, - onHome: _handleHomeButton, - onDismissPrompt: (_showPlayNextDialog || _showStillWatchingPrompt) - ? _dismissPlaybackPromptForBack - : null, onReachedEnd: ({skipAutoPlayCountdown = false}) => _onVideoCompleted(true, skipAutoPlayCountdown: skipAutoPlayCountdown), canControl: canControl, diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index cb1e5b83..b0e27c24 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -1474,9 +1474,8 @@ class VideoPlayerScreenState extends State with WidgetsBindin final isCurrentRoute = ModalRoute.of(context)?.isCurrent ?? true; // Screen-level Focus wraps ALL phases (loading + initialized). // - autofocus: grabs focus when no deeper child claims it. - // - onKeyEvent: self-heals when this node has primary focus (no descendant - // focused). Nav keys are only consumed in that case; otherwise they pass - // through so DirectionalFocusAction can drive dpad nav in overlay sheets. + // - onKeyEvent: owns player-level navigation after descendants have had the + // opportunity to handle local layers such as sheets and content strips. return Focus( focusNode: _screenFocusNode, autofocus: isCurrentRoute, @@ -1485,9 +1484,6 @@ class VideoPlayerScreenState extends State with WidgetsBindin if (!isCurrentRoute) return KeyEventResult.ignored; final navigationKey = classifyPlayerNavigationKey(event, isAppleTV: PlatformDetector.isAppleTV()); if (navigationKey != PlayerNavigationKey.none) { - // Descendants (controls and sheets) own staged Back while focused. - // This fallback covers loading/error phases and focus drift. - if (!node.hasPrimaryFocus) return KeyEventResult.ignored; if (navigationKey != PlayerNavigationKey.home && PlatformDetector.isTV() && event is KeyDownEvent) { BackKeyCoordinator.markHandled(); } diff --git a/lib/widgets/overlay_sheet.dart b/lib/widgets/overlay_sheet.dart index aa669509..61e16c35 100644 --- a/lib/widgets/overlay_sheet.dart +++ b/lib/widgets/overlay_sheet.dart @@ -421,15 +421,16 @@ class _OverlaySheetHostState extends State with SingleTickerPr } void _autoFocus() { - if (!InputModeTracker.isKeyboardMode(context)) return; + final focusDescendant = InputModeTracker.isKeyboardMode(context); // First post-frame: the FocusScope is now built and the node is attached. - // Grab scope focus immediately so key events (especially back) are trapped. - // Second post-frame: ListView.builder items are laid out and their - // FocusNodes are registered — focus the first descendant for dpad nav. + // Always grab scope focus so key events (especially back) are trapped, even + // when a pointer opened the sheet. In keyboard mode, a second post-frame + // callback focuses the first descendant for dpad navigation. WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !_isOpen) return; _sheetFocusScopeNode.requestFocus(); + if (!focusDescendant) return; WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !_isOpen) return; // If the current top entry has an initialFocusNode that is attached, @@ -456,11 +457,12 @@ class _OverlaySheetHostState extends State with SingleTickerPr } void _refocus() { - if (!InputModeTracker.isKeyboardMode(context)) return; + final focusDescendant = InputModeTracker.isKeyboardMode(context); WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !_isOpen) return; _sheetFocusScopeNode.requestFocus(); + if (!focusDescendant) return; WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !_isOpen) return; final topEntry = _pageStack.isNotEmpty ? _pageStack.last : null; diff --git a/lib/widgets/video_controls/parts/key_events.dart b/lib/widgets/video_controls/parts/key_events.dart index 94a91fc1..b8a27a50 100644 --- a/lib/widgets/video_controls/parts/key_events.dart +++ b/lib/widgets/video_controls/parts/key_events.dart @@ -70,54 +70,27 @@ extension _PlexVideoControlsKeyEventMethods on _PlexVideoControlsState { _showControlsWithFocus(); } - KeyEventResult _handlePlayerNavigationKeyEvent(KeyEvent event) { - final navigationKey = classifyPlayerNavigationKey(event, isAppleTV: PlatformDetector.isAppleTV()); - if (navigationKey == PlayerNavigationKey.none) return KeyEventResult.ignored; - - if (navigationKey != PlayerNavigationKey.home && PlatformDetector.isTV() && event is KeyDownEvent) { - BackKeyCoordinator.markHandled(); + KeyEventResult _handleLocalPlayerNavigationKeyEvent(KeyEvent event, PlayerNavigationKey navigationKey) { + if (navigationKey == PlayerNavigationKey.none || navigationKey == PlayerNavigationKey.home) { + return KeyEventResult.ignored; } - return handlePlayerNavigationKeyAction(event, navigationKey, () { - if (navigationKey == PlayerNavigationKey.home) { - (widget.onHome ?? widget.onBack ?? () => Navigator.of(context).pop(true))(); - return; - } - if (widget.onDismissPrompt != null) { - widget.onDismissPrompt!(); - return; - } - _handleStagedPlayerBack(navigationKey); - }); - } + final sheetController = OverlaySheetController.maybeOf(context); + if (sheetController?.isOpen ?? false) { + return handlePlayerNavigationKeyAction(event, navigationKey, sheetController!.pop); + } - void _handleStagedPlayerBack(PlayerNavigationKey navigationKey) { - final disposition = resolvePlayerBackDisposition( - navigationKey: navigationKey, - contentStripVisible: widget.chromeController.contentStripVisible, - controlsVisible: _showControls, - ); - switch (disposition) { - case PlayerBackDisposition.closeContentStrip: + if (widget.chromeController.contentStripVisible) { + return handlePlayerNavigationKeyAction(event, navigationKey, () { _desktopControlsKey.currentState?.dismissContentStrip(); widget.chromeController.setContentStripVisible(false); _restartHideTimerForCurrentPlaybackState(); - return; - case PlayerBackDisposition.exitFullscreenIfActive: - unawaited(_handlePhysicalEscape()); - return; - case PlayerBackDisposition.hideControls: - _hideControls(); - return; - case PlayerBackDisposition.exitPlayer: - (widget.onBack ?? () => Navigator.of(context).pop(true))(); - return; + }); } - } - Future _handlePhysicalEscape() async { - if (await FullscreenStateManager().exitFullscreenIfActive()) return; - if (mounted) _handleStagedPlayerBack(PlayerNavigationKey.back); + // The enclosing player screen is the sole owner of fullscreen, chrome, + // prompt, and route-exit stages. + return KeyEventResult.ignored; } /// Global key event handler for focus-independent shortcuts (desktop only) @@ -136,10 +109,10 @@ extension _PlexVideoControlsKeyEventMethods on _PlexVideoControlsState { return false; } - // Focus.onKeyEvent will not fire if focus drifted outside the controls. - if (!_focusNode.hasFocus) { - final navigationResult = _handlePlayerNavigationKeyEvent(event); - if (navigationResult != KeyEventResult.ignored) return true; + // Native key events also continue through the focus tree after global + // handlers run. Player navigation must only mutate state there. + if (classifyPlayerNavigationKey(event, isAppleTV: PlatformDetector.isAppleTV()) != PlayerNavigationKey.none) { + return false; } // Only handle when video player navigation is disabled (desktop mode without D-pad nav) @@ -190,10 +163,12 @@ extension _PlexVideoControlsKeyEventMethods on _PlexVideoControlsState { } KeyEventResult _handleControlsKeyEvent(KeyEvent event, bool isMobile) { - final navigationResult = _handlePlayerNavigationKeyEvent(event); + final navigationKey = classifyPlayerNavigationKey(event, isAppleTV: PlatformDetector.isAppleTV()); + final navigationResult = _handleLocalPlayerNavigationKeyEvent(event, navigationKey); if (navigationResult != KeyEventResult.ignored) { return navigationResult; } + if (navigationKey != PlayerNavigationKey.none) return KeyEventResult.ignored; // Only handle KeyDown and KeyRepeat events. // Consume KeyUp events for navigation keys to prevent leaking to previous routes. diff --git a/lib/widgets/video_controls/video_controls.dart b/lib/widgets/video_controls/video_controls.dart index e4b1ef89..ea7c70e7 100644 --- a/lib/widgets/video_controls/video_controls.dart +++ b/lib/widgets/video_controls/video_controls.dart @@ -301,13 +301,6 @@ class PlexVideoControls extends StatefulWidget { /// Called when back button is pressed (for Watch Together session leave confirmation) final VoidCallback? onBack; - /// Called for a direct Home command after player-specific input handling. - final VoidCallback? onHome; - - /// Called when Back should dismiss a visible playback prompt before normal - /// player back handling. - final VoidCallback? onDismissPrompt; - /// Called when the video has effectively reached the end (e.g. credits extend /// to EOF and can't be seeked past). Parent should route this into its normal /// completion flow so the auto-play-next setting is honored. @@ -408,8 +401,6 @@ class PlexVideoControls extends StatefulWidget { this.onPlayPauseRequested, this.onSeekCompleted, this.onBack, - this.onHome, - this.onDismissPrompt, this.onReachedEnd, this.canControl = true, this.hasFirstFrame, diff --git a/test/services/companion_remote_receiver_test.dart b/test/services/companion_remote_receiver_test.dart index 2e64d4ac..dba46986 100644 --- a/test/services/companion_remote_receiver_test.dart +++ b/test/services/companion_remote_receiver_test.dart @@ -5,12 +5,14 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:plezy/models/companion_remote/remote_command.dart'; import 'package:plezy/services/companion_remote/companion_remote_receiver.dart'; +import 'package:plezy/widgets/video_controls/video_controls.dart'; void main() { testWidgets('Back command dispatches semantic gamepad B events', (tester) async { final focusNode = FocusNode(); addTearDown(focusNode.dispose); final events = []; + var actions = 0; await tester.pumpWidget( MaterialApp( @@ -18,7 +20,8 @@ void main() { focusNode: focusNode, onKeyEvent: (_, event) { events.add(event); - return KeyEventResult.handled; + final navigationKey = classifyPlayerNavigationKey(event, isAppleTV: false); + return handlePlayerNavigationKeyAction(event, navigationKey, () => actions++); }, child: const SizedBox.expand(), ), @@ -35,5 +38,6 @@ void main() { expect(events.last, isA()); expect(events.map((event) => event.logicalKey), everyElement(LogicalKeyboardKey.gameButtonB)); expect(events.map((event) => event.deviceType), everyElement(ui.KeyEventDeviceType.directionalPad)); + expect(actions, 1); }); } diff --git a/test/widgets/overlay_sheet_test.dart b/test/widgets/overlay_sheet_test.dart index 12005f89..cd6650ed 100644 --- a/test/widgets/overlay_sheet_test.dart +++ b/test/widgets/overlay_sheet_test.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:plezy/widgets/overlay_sheet.dart'; @@ -92,6 +93,52 @@ void main() { expect(sheetSize.width, 700); }); + testWidgets('pointer-opened sheet claims focus and handles Back before the screen', (tester) async { + final screenFocusNode = FocusNode(debugLabel: 'Screen'); + addTearDown(screenFocusNode.dispose); + var screenBacks = 0; + + await tester.pumpWidget( + MaterialApp( + home: Focus( + focusNode: screenFocusNode, + autofocus: true, + onKeyEvent: (_, event) { + if (event.logicalKey == LogicalKeyboardKey.gameButtonB) { + if (event is KeyUpEvent) screenBacks++; + return KeyEventResult.handled; + } + return KeyEventResult.ignored; + }, + child: OverlaySheetHost( + child: Scaffold( + body: Center( + child: Builder( + builder: (context) => ElevatedButton( + onPressed: () => OverlaySheetController.of(context).show( + builder: (_) => const SizedBox(height: 120, child: Center(child: Text('SHEET'))), + ), + child: const Text('Open'), + ), + ), + ), + ), + ), + ), + ), + ); + + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + expect(FocusManager.instance.primaryFocus?.debugLabel, 'OverlaySheetScope'); + await tester.sendKeyEvent(LogicalKeyboardKey.gameButtonB); + await tester.pumpAndSettle(); + + expect(find.text('SHEET'), findsNothing); + expect(screenBacks, 0); + }); + group('opt-in canPop / onSystemBack', () { // Pushes an OverlaySheetHost route on top of a home route so we can observe // whether a simulated system back pops the route. The host's child has an