fix(player): keep the remote on the player surface after a window switch (#1797)

Returning to the desktop window with the chrome still up left arrow keys
navigating the OSD instead of seeking: the first press seeked and silently
moved focus onto Play/Pause, and every press after that walked the buttons.

A window blur drops Flutter's primary focus to the root scope, so the player
screen's reclaim parks it on its own node. The only handoff back down to the
controls was the chrome visible->hidden transition, so with the OSD up nothing
reclaimed it -- hence the reported workarounds of letting the controls hide, or
moving the pointer off the player and back. Pointer exit normally hides the
chrome and masks this, which is why it only shows when the pointer stays over
the player while another window takes focus.

Hand the surface back on window re-activation, next to the existing hide-path
claim, and rename the helper since it is no longer hidden-chrome specific. The
claim runs synchronously because a platform callback is not guaranteed to be
followed by a frame; the screen's reclaim re-tests hasFocus when it runs, so the
two no longer compete.

Also gate the screen's self-heal so a directional key no longer pulls focus into
the OSD when "Video Player Navigation" is off -- Tab and select keep their path
in, which the ungated return value would otherwise consume with nowhere to go.
This commit is contained in:
edde746
2026-08-05 15:24:57 +02:00
parent 23b8befe11
commit 9d51a040c3
5 changed files with 365 additions and 6 deletions
+7 -1
View File
@@ -2127,8 +2127,14 @@ class VideoPlayerScreenState extends State<VideoPlayerScreen> with WidgetsBindin
} }
// Self-heal: if this node itself has primary focus (no descendant // Self-heal: if this node itself has primary focus (no descendant
// focused, e.g. after controls auto-hide), redirect to first descendant. // focused, e.g. after controls auto-hide), redirect to first descendant.
// Arrows stay playback shortcuts on desktop unless the viewer opted into
// player navigation; only Tab/select may deliberately pull focus into
// the OSD (#1797). Consuming navigation keys either way keeps them from
// leaking to the route below.
if (node.hasPrimaryFocus) { if (node.hasPrimaryFocus) {
if (event.isActionable) { final claimsChrome =
!event.logicalKey.isDpadDirection || _videoPlayerNavigationEnabled || PlatformDetector.isTV();
if (event.isActionable && claimsChrome) {
_chromeController.show(focusTarget: PlayerChromeFocusTarget.playPause); _chromeController.show(focusTarget: PlayerChromeFocusTarget.playPause);
} }
return event.logicalKey.isNavigationKey ? KeyEventResult.handled : KeyEventResult.ignored; return event.logicalKey.isNavigationKey ? KeyEventResult.handled : KeyEventResult.ignored;
@@ -250,7 +250,7 @@ extension _PlexVideoControlsVisibilityMethods on _PlexVideoControlsState {
_controlsOpaque = false; _controlsOpaque = false;
if (_currentMarker != null) _skipButtonDismissed = true; if (_currentMarker != null) _skipButtonDismissed = true;
}); });
_claimHiddenChromeFocus(); _claimPlayerSurfaceFocus();
} else if (visibilityChanged) { } else if (visibilityChanged) {
// The timeline is about to take over held-key seeking; commit whatever // The timeline is about to take over held-key seeking; commit whatever
// the hidden-chrome burst accumulated so it can't rebase from a stale // the hidden-chrome burst accumulated so it can't rebase from a stale
@@ -280,11 +280,11 @@ extension _PlexVideoControlsVisibilityMethods on _PlexVideoControlsState {
} }
} }
/// Park focus on the player surface so the hidden-chrome key layer owns the /// Park focus on the player surface so this widget's key layer owns the
/// remote. Without this the screen node keeps primary focus and its /// remote. Without this the screen node keeps primary focus and its
/// self-heal raises the whole chrome on the first actionable key, which is /// self-heal raises the whole chrome on the first actionable key, which is
/// what the transient seek and transport indicators exist to avoid. /// what the transient seek and transport indicators exist to avoid.
void _claimHiddenChromeFocus() { void _claimPlayerSurfaceFocus() {
final sheetOpen = OverlaySheetController.maybeOf(context)?.isOpen ?? false; final sheetOpen = OverlaySheetController.maybeOf(context)?.isOpen ?? false;
if (sheetOpen) return; if (sheetOpen) return;
_focusNode.requestFocus(); _focusNode.requestFocus();
@@ -299,7 +299,7 @@ extension _PlexVideoControlsVisibilityMethods on _PlexVideoControlsState {
WidgetsBinding.instance.addPostFrameCallback((_) { WidgetsBinding.instance.addPostFrameCallback((_) {
if (!mounted || !widget.chromeController.controlsVisible) return; if (!mounted || !widget.chromeController.controlsVisible) return;
// Never steal focus from an open sheet (same rule as // Never steal focus from an open sheet (same rule as
// _claimHiddenChromeFocus). // _claimPlayerSurfaceFocus).
if (OverlaySheetController.maybeOf(context)?.isOpen ?? false) return; if (OverlaySheetController.maybeOf(context)?.isOpen ?? false) return;
switch (target) { switch (target) {
case PlayerChromeFocusTarget.playPause: case PlayerChromeFocusTarget.playPause:
+20 -1
View File
@@ -858,7 +858,7 @@ class _PlexVideoControlsState extends State<PlexVideoControls>
// A route that opened with no chrome never ran the hide transition that // A route that opened with no chrome never ran the hide transition that
// normally hands focus down here, and this Focus autofocuses too late to // normally hands focus down here, and this Focus autofocuses too late to
// win it: the screen node claimed it during the loading phase. // win it: the screen node claimed it during the loading phase.
if (!widget.chromeController.controlsVisible) _claimHiddenChromeFocus(); if (!widget.chromeController.controlsVisible) _claimPlayerSurfaceFocus();
if (PlatformDetector.isMobile(context) && !PlatformDetector.isTV()) { if (PlatformDetector.isMobile(context) && !PlatformDetector.isTV()) {
_refreshDeviceAdjustmentValues(); _refreshDeviceAdjustmentValues();
} }
@@ -1010,6 +1010,25 @@ class _PlexVideoControlsState extends State<PlexVideoControls>
} }
} }
/// Re-activating the window drops Flutter's primary focus to the root scope,
/// and the enclosing player screen reclaims it onto its own node. Nothing
/// hands it back down while the chrome stays visible — the hide transition is
/// the only other handoff — so the next arrow key reaches the screen's
/// self-heal and jumps focus into the OSD (#1797). Take the surface back
/// unless a control below already owns it.
@override
void onWindowFocus() {
// Claim now rather than post-frame: this arrives on a platform callback,
// which is not guaranteed to be followed by a frame. The screen's own
// reclaim re-tests `hasFocus` when it runs, so once the surface holds the
// remote the two no longer compete.
if (!mounted || _focusNode.hasFocus) return;
// A route pushed above the player still leaves these controls mounted;
// re-activating the window must not pull the remote off the top route.
if (ModalRoute.of(context)?.isCurrent != true) return;
_claimPlayerSurfaceFocus();
}
@override @override
// ignore: no-empty-block - required by WindowListener interface // ignore: no-empty-block - required by WindowListener interface
void onWindowResize() {} void onWindowResize() {}
@@ -0,0 +1,92 @@
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/providers/playback_state_provider.dart';
import 'package:plezy/screens/video_player_screen.dart';
import 'package:plezy/services/settings_service.dart';
import 'package:plezy/utils/platform_detector.dart';
import 'package:plezy/widgets/video_controls/player_chrome_controller.dart';
import 'package:provider/provider.dart';
import '../../test_helpers/media_items.dart';
import '../../test_helpers/mock_player_channels.dart';
import '../../test_helpers/prefs.dart';
/// Regression coverage for #1797: while the screen node holds primary focus,
/// its self-heal answers an actionable key by raising the chrome onto the
/// Play/Pause button. With "Video Player Navigation" off, arrows are playback
/// shortcuts and must not be turned into a focus jump — but Tab is the
/// deliberate way into the OSD and must keep working.
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
setUp(() async {
resetSharedPreferencesForTest();
SettingsService.resetForTesting();
await SettingsService.getInstance();
TvDetectionService.debugSetAppleTVOverride(false);
});
tearDown(() {
TvDetectionService.debugSetAppleTVOverride(null);
});
testWidgets('an arrow is left to the playback shortcuts when player navigation is off', (tester) async {
final target = await _selfHealTargetFor(tester, LogicalKeyboardKey.arrowLeft);
expect(target, isNull, reason: 'an arrow must seek, not pull focus onto Play/Pause');
});
testWidgets('Tab still walks into the player controls when player navigation is off', (tester) async {
final target = await _selfHealTargetFor(tester, LogicalKeyboardKey.tab);
expect(
target,
PlayerChromeFocusTarget.playPause,
reason: 'Tab is the deliberate way into the OSD and must keep reaching it',
);
});
}
/// Sends [key] to a freshly opened player route — whose screen node still owns
/// primary focus, exactly as after a window re-activation — and reports the
/// focus target its self-heal queued on the chrome, if any.
Future<PlayerChromeFocusTarget?> _selfHealTargetFor(WidgetTester tester, LogicalKeyboardKey key) async {
final screenKey = GlobalKey<VideoPlayerScreenState>();
PlayerChromeFocusTarget? target;
await withMockPlayerChannels(
methodChannelName: 'com.plezy/mpv_player',
eventChannelName: 'com.plezy/mpv_player/events',
testBody: () async {
await tester.pumpWidget(
ChangeNotifierProvider(
create: (_) => PlaybackStateProvider(),
child: MaterialApp(
home: VideoPlayerScreen(
key: screenKey,
metadata: testMediaItem(title: 'Self-heal keys'),
isOffline: true,
),
),
),
);
await tester.pump();
final chrome = screenKey.currentState!.chromeController;
// Drain anything the route queued while opening, so the assertion can
// only see what this key press produced.
chrome.takeFocusTarget();
await tester.sendKeyDownEvent(key);
await tester.pump();
await tester.sendKeyUpEvent(key);
await tester.pump();
target = chrome.takeFocusTarget();
await tester.pumpWidget(const SizedBox.shrink());
},
);
return target;
}
@@ -0,0 +1,242 @@
import 'package:drift/native.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:intl/date_symbol_data_local.dart';
import 'package:provider/provider.dart';
import 'package:window_manager/window_manager.dart';
import 'package:plezy/database/app_database.dart';
import 'package:plezy/i18n/strings.g.dart';
import 'package:plezy/mpv/mpv.dart';
import 'package:plezy/providers/playback_state_provider.dart';
import 'package:plezy/services/settings_service.dart';
import 'package:plezy/services/video_volume_controller.dart';
import 'package:plezy/utils/platform_detector.dart';
import 'package:plezy/watch_together/providers/watch_together_provider.dart';
import 'package:plezy/widgets/video_controls/desktop_video_controls.dart';
import 'package:plezy/widgets/video_controls/player_chrome_controller.dart';
import 'package:plezy/widgets/video_controls/video_controls.dart';
import 'package:plezy/widgets/video_controls/widgets/player_toast_indicator.dart';
import '../test_helpers/media_items.dart';
import '../test_helpers/prefs.dart';
import '../test_helpers/theme.dart';
/// Regression coverage for #1797: returning to the desktop window while the
/// chrome is up left primary focus parked on the enclosing screen node. The
/// chrome-hide transition is the only other handoff back to the player surface,
/// so with the OSD visible nothing reclaimed it, and the screen's self-heal
/// turned the next arrow key into a jump onto the Play/Pause button — from
/// there every further arrow navigated the OSD instead of seeking.
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
group('window refocus', () {
late _PlayingPlayer player;
late PlayerChromeController chrome;
late PlayerToastController toast;
late VideoVolumeController volume;
late PlaybackStateProvider playbackState;
late WatchTogetherProvider watchTogether;
late AppDatabase database;
late ValueNotifier<bool> hasFirstFrame;
late FocusNode screenFocusNode;
late List<LogicalKeyboardKey> keysReachingScreen;
setUp(() async {
LocaleSettings.setLocaleSync(AppLocale.en);
await initializeDateFormatting('en');
resetSharedPreferencesForTest();
SettingsService.resetForTesting();
final settings = await SettingsService.getInstance();
TvDetectionService.debugSetAppleTVOverride(false);
PlatformDetector.debugSetIsDesktopOSOverride(false);
database = AppDatabase.forTesting(NativeDatabase.memory());
player = _PlayingPlayer();
// The reported state: the viewer came back with the OSD still up.
chrome = PlayerChromeController(initiallyVisible: true);
toast = PlayerToastController();
volume = VideoVolumeController(player: player, settings: settings, initialVolume: 100);
playbackState = PlaybackStateProvider();
watchTogether = WatchTogetherProvider();
hasFirstFrame = ValueNotifier<bool>(true);
screenFocusNode = FocusNode(debugLabel: 'VideoPlayerScreen');
keysReachingScreen = <LogicalKeyboardKey>[];
});
tearDown(() async {
TvDetectionService.debugSetAppleTVOverride(null);
PlatformDetector.debugSetIsDesktopOSOverride(null);
hasFirstFrame.dispose();
screenFocusNode.dispose();
volume.dispose();
playbackState.dispose();
watchTogether.dispose();
chrome.dispose();
toast.dispose();
await database.close();
});
Widget shell(Widget child) {
return MultiProvider(
providers: [
Provider<AppDatabase>.value(value: database),
ChangeNotifierProvider<PlaybackStateProvider>.value(value: playbackState),
ChangeNotifierProvider<WatchTogetherProvider>.value(value: watchTogether),
],
child: MaterialApp(
theme: ThemeData(platform: TargetPlatform.windows, extensions: const [testMonoTokens]),
home: Scaffold(
body: SizedBox(
width: 1280,
height: 720,
child: Focus(
focusNode: screenFocusNode,
autofocus: true,
onKeyEvent: (node, event) {
if (event is KeyDownEvent) keysReachingScreen.add(event.logicalKey);
return KeyEventResult.ignored;
},
child: child,
),
),
),
),
);
}
/// Reproduces the post-blur layout: the screen node owns primary focus (the
/// player screen reclaims it whenever focus leaves the subtree) while the
/// controls are mounted with their chrome up.
Future<void> pumpControlsUnderScreenFocus(WidgetTester tester) async {
await tester.pumpWidget(shell(const SizedBox.expand()));
await tester.pump();
expect(screenFocusNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
shell(
PlexVideoControls(
player: player,
volumeController: volume,
metadata: testMediaItem(id: 'window-refocus'),
toastController: toast,
chromeController: chrome,
hasFirstFrame: hasFirstFrame,
canNavigateMediaItems: false,
),
),
);
await tester.pumpAndSettle();
// The controls' own `autofocus` cannot win the scope back, and a visible
// chrome never runs the hide transition that hands focus down — so this
// is exactly the state a window re-activation leaves behind.
expect(
screenFocusNode.hasPrimaryFocus,
isTrue,
reason: 'precondition: focus is stranded on the screen node with the OSD up',
);
}
testWidgets('re-activating the window hands the remote back to the player surface', (tester) async {
await pumpControlsUnderScreenFocus(tester);
(tester.state(find.byType(PlexVideoControls)) as WindowListener).onWindowFocus();
await tester.pumpAndSettle();
expect(
screenFocusNode.hasPrimaryFocus,
isFalse,
reason: 'the player surface owns the remote again, not the screen node',
);
// The reported symptom lands on the second press: the first is answered
// while the very same event also steals focus into the OSD.
for (var press = 0; press < 2; press++) {
await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowLeft);
await tester.pump();
await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowLeft);
await tester.pump();
}
expect(
keysReachingScreen,
isEmpty,
reason: 'no arrow may reach the screen self-heal, or it jumps focus onto Play/Pause',
);
chrome.cancelAutoHide();
await tester.pumpWidget(const SizedBox.shrink());
});
testWidgets('a control the viewer focused on purpose keeps the remote', (tester) async {
await pumpControlsUnderScreenFocus(tester);
// Deliberate focus inside the OSD, as traversal or a TV remote leaves it.
tester.state<DesktopVideoControlsState>(find.byType(DesktopVideoControls)).requestPlayPauseFocus();
await tester.pumpAndSettle();
final focusedControl = FocusManager.instance.primaryFocus;
expect(focusedControl?.debugLabel, 'PlayPause', reason: 'a control took the remote');
(tester.state(find.byType(PlexVideoControls)) as WindowListener).onWindowFocus();
await tester.pumpAndSettle();
expect(
FocusManager.instance.primaryFocus,
same(focusedControl),
reason: 'the surface claim must not yank focus off a control the viewer chose',
);
chrome.cancelAutoHide();
await tester.pumpWidget(const SizedBox.shrink());
});
});
}
/// Minimal [Player] reporting steady playback, the state the player settles
/// into once the media is open.
class _PlayingPlayer implements Player {
final List<Duration> seeks = [];
Duration _position = const Duration(minutes: 5);
@override
String get playerType => 'mpv';
@override
PlayerState get state =>
PlayerState(playing: true, position: _position, duration: const Duration(minutes: 45), seekable: true);
@override
Future<void> seek(Duration position) async {
seeks.add(position);
_position = position;
}
@override
PlayerStreams get streams => PlayerStreams(
playing: const Stream<bool>.empty(),
completed: const Stream<bool>.empty(),
buffering: const Stream<bool>.empty(),
position: const Stream<Duration>.empty(),
duration: const Stream<Duration>.empty(),
seekable: const Stream<bool>.empty(),
buffer: const Stream<Duration>.empty(),
volume: const Stream<double>.empty(),
rate: const Stream<double>.empty(),
tracks: const Stream<Tracks>.empty(),
track: const Stream<TrackSelection>.empty(),
log: const Stream<PlayerLog>.empty(),
error: const Stream<PlayerError>.empty(),
audioDevice: const Stream<AudioDevice>.empty(),
audioDevices: const Stream<List<AudioDevice>>.empty(),
bufferRanges: const Stream<List<BufferRange>>.empty(),
playbackRestart: const Stream<void>.empty(),
backendSwitched: const Stream<void>.empty(),
);
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}