diff --git a/lib/focus/key_event_utils.dart b/lib/focus/key_event_utils.dart index 5262b43c..e9e0bd47 100644 --- a/lib/focus/key_event_utils.dart +++ b/lib/focus/key_event_utils.dart @@ -123,6 +123,23 @@ KeyEventResult handleOneShotSelect(KeyEvent event, VoidCallback onActivate) { return KeyEventResult.handled; } +/// Whether the primary focus currently belongs to an active text editor. +/// +/// Ancestor key handlers use this to stay off keys a focused field owns. +/// Flutter dispatches a key event from the focused node upwards, and the +/// editing shortcuts that turn Backspace into a deletion live in +/// [DefaultTextEditingShortcuts] at the very top of the app — *above* any +/// screen. An ancestor that claims Backspace as "back" therefore both steals +/// the navigation and stops the character from ever being deleted (#1741). +/// +/// [EditableText] builds its [Focus] internally, so the focused node's context +/// resolves to the owning [EditableTextState]. +bool isTextEditingFocused() { + final context = FocusManager.instance.primaryFocus?.context; + if (context == null) return false; + return context.findAncestorStateOfType() != null; +} + /// Expands a UTF-16 [range] to whole extended grapheme clusters in [text]. /// /// Flutter selections use UTF-16 code-unit offsets. Custom editors must pass a diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index a6d796fe..b2701a91 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -466,6 +466,14 @@ class VideoPlayerScreenState extends State with WidgetsBindin // key events never escape the video player route. late final FocusNode _screenFocusNode; + /// Key for a context below this screen's own [OverlaySheetHost]. The State's + /// context sits ABOVE the host, so resolving the controller with `context` + /// always misses it and the screen would walk its back pipeline (hide chrome, + /// then exit the player) while a sheet is still open (#1741). + final GlobalKey _overlayChildKey = GlobalKey(); + + BuildContext get _sheetContext => _overlayChildKey.currentContext ?? context; + // VLC-style in-player toast controller (rate changes, backend switch, etc.). final PlayerToastController _toastController = PlayerToastController(); bool _reclaimingFocus = false; @@ -1454,7 +1462,7 @@ class VideoPlayerScreenState extends State with WidgetsBindin void _handleScreenPlayerNavigation(PlayerNavigationKey navigationKey) { if (navigationKey != PlayerNavigationKey.home) { - final sheetController = OverlaySheetController.maybeOf(context); + final sheetController = OverlaySheetController.maybeOf(_sheetContext); if (sheetController?.isOpen ?? false) { sheetController!.pop(); return; @@ -1998,6 +2006,7 @@ class VideoPlayerScreenState extends State with WidgetsBindin _handleScreenPlayerNavigation(PlayerNavigationKey.back); }, child: Builder( + key: _overlayChildKey, builder: (sheetContext) => _isPlayerInitialized && player != null ? _buildVideoPlayer(sheetContext) : (_playerInitializationError != null diff --git a/lib/widgets/video_controls/video_controls.dart b/lib/widgets/video_controls/video_controls.dart index c39de772..4bff1baf 100644 --- a/lib/widgets/video_controls/video_controls.dart +++ b/lib/widgets/video_controls/video_controls.dart @@ -325,7 +325,22 @@ PlayerBackDisposition resolvePlayerBackDisposition({ return controlsVisible ? PlayerBackDisposition.hideControls : PlayerBackDisposition.exitPlayer; } -PlayerNavigationKey classifyPlayerNavigationKey(KeyEvent event, {required bool isAppleTV, bool? hasModifiers}) { +/// Maps a key event to the player-level navigation stage it should drive. +/// +/// [textEditingActive] defaults to [isTextEditingFocused]; inject it in tests. +/// Bare Backspace and Home double as player navigation *and* as caret editing +/// keys, so a focused text editor takes them back — otherwise typing in a +/// player sheet (subtitle search) walks the back pipeline out of the player +/// instead of correcting a character (#1741). Only physical-keyboard presses +/// are surrendered: a synthesized dpad/gamepad/companion press has no caret, +/// and [LogicalKeyboardKey.browserHome] is a dedicated navigation key with no +/// editing role at all. +PlayerNavigationKey classifyPlayerNavigationKey( + KeyEvent event, { + required bool isAppleTV, + bool? hasModifiers, + bool? textEditingActive, +}) { final key = event.logicalKey; if (key == LogicalKeyboardKey.escape) { return event.isPhysicalKeyboardEvent && !isAppleTV ? PlayerNavigationKey.physicalEscape : PlayerNavigationKey.back; @@ -338,10 +353,14 @@ PlayerNavigationKey classifyPlayerNavigationKey(KeyEvent event, {required bool i HardwareKeyboard.instance.isControlPressed || HardwareKeyboard.instance.isAltPressed || HardwareKeyboard.instance.isMetaPressed); + // Resolved lazily: only the two editing keys below pay for the focus lookup. + bool textEditorOwnsKey() => event.isPhysicalKeyboardEvent && (textEditingActive ?? isTextEditingFocused()); + if (key == LogicalKeyboardKey.backspace && event.isPhysicalKeyboardEvent && !modifiersPressed) { - return PlayerNavigationKey.back; + return textEditorOwnsKey() ? PlayerNavigationKey.none : PlayerNavigationKey.back; } if ((key == LogicalKeyboardKey.home || key == LogicalKeyboardKey.browserHome) && !modifiersPressed) { + if (key == LogicalKeyboardKey.home && textEditorOwnsKey()) return PlayerNavigationKey.none; return PlayerNavigationKey.home; } return PlayerNavigationKey.none; diff --git a/test/screens/video_player/player_text_input_navigation_test.dart b/test/screens/video_player/player_text_input_navigation_test.dart new file mode 100644 index 00000000..baff34c9 --- /dev/null +++ b/test/screens/video_player/player_text_input_navigation_test.dart @@ -0,0 +1,323 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/focus/dpad_navigator.dart'; +import 'package:plezy/focus/focusable_text_field.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/widgets/overlay_sheet.dart'; +import 'package:plezy/widgets/video_controls/video_controls.dart'; +import 'package:provider/provider.dart'; + +import '../../test_helpers/media_items.dart'; +import '../../test_helpers/mock_player_channels.dart'; +import '../../test_helpers/prefs.dart'; + +/// Mirrors `VideoPlayerScreen.build`: a screen-level [Focus] that owns player +/// navigation, wrapping the screen's own [OverlaySheetHost]. Sheets therefore +/// sit *below* the navigation handler in the focus chain, which is what let a +/// Delete press inside the subtitle-search field walk the back pipeline out of +/// the player instead of correcting a character (#1741). +/// +/// Booting the real screen needs a live player, so this shell reproduces the +/// layering instead. That makes the Backspace/Home cases genuine regression +/// guards — they exercise the production [classifyPlayerNavigationKey] — while +/// the sheet-dismissal cases pin the wiring contract the screen must keep: +/// resolve [OverlaySheetController] from a context *below* the host +/// (`_overlayChildKey`/`_sheetContext`), never from the State's own context. +class _PlayerShell extends StatefulWidget { + final TextEditingController controller; + final FocusNode fieldFocusNode; + final FocusNode sheetButtonFocusNode; + final void Function(PlayerNavigationKey) onPlayerNavigation; + + const _PlayerShell({ + required this.controller, + required this.fieldFocusNode, + required this.sheetButtonFocusNode, + required this.onPlayerNavigation, + }); + + @override + State<_PlayerShell> createState() => _PlayerShellState(); +} + +class _PlayerShellState extends State<_PlayerShell> { + final FocusNode _screenFocusNode = FocusNode(debugLabel: 'VideoPlayerScreen'); + final GlobalKey _overlayChildKey = GlobalKey(); + + BuildContext get _sheetContext => _overlayChildKey.currentContext ?? context; + + @override + void dispose() { + _screenFocusNode.dispose(); + super.dispose(); + } + + void _handleScreenPlayerNavigation(PlayerNavigationKey navigationKey) { + if (navigationKey != PlayerNavigationKey.home) { + final sheetController = OverlaySheetController.maybeOf(_sheetContext); + if (sheetController?.isOpen ?? false) { + sheetController!.pop(); + return; + } + } + widget.onPlayerNavigation(navigationKey); + } + + @override + Widget build(BuildContext context) { + return Focus( + focusNode: _screenFocusNode, + autofocus: true, + onKeyEvent: (node, event) { + final navigationKey = classifyPlayerNavigationKey(event, isAppleTV: false); + if (navigationKey != PlayerNavigationKey.none) { + return handlePlayerNavigationKeyAction( + event, + navigationKey, + () => _handleScreenPlayerNavigation(navigationKey), + ); + } + if (node.hasPrimaryFocus) { + return event.logicalKey.isNavigationKey ? KeyEventResult.handled : KeyEventResult.ignored; + } + return KeyEventResult.ignored; + }, + child: OverlaySheetHost( + canPop: false, + onSystemBack: () => _handleScreenPlayerNavigation(PlayerNavigationKey.back), + child: Builder( + key: _overlayChildKey, + builder: (sheetContext) => Scaffold( + body: Center( + child: ElevatedButton( + onPressed: () => OverlaySheetController.of(sheetContext).show( + builder: (_) => Column( + mainAxisSize: MainAxisSize.min, + children: [ + FocusableTextField( + controller: widget.controller, + focusNode: widget.fieldFocusNode, + tvTextInputPresentation: TvTextInputPresentation.platform, + ), + TextButton( + focusNode: widget.sheetButtonFocusNode, + onPressed: () {}, + child: const Text('Download'), + ), + ], + ), + ), + child: const Text('Search subtitles'), + ), + ), + ), + ), + ), + ); + } +} + +class _Harness { + final TextEditingController controller; + final FocusNode fieldFocusNode; + final FocusNode sheetButtonFocusNode; + final List navigations; + + _Harness({ + required this.controller, + required this.fieldFocusNode, + required this.sheetButtonFocusNode, + required this.navigations, + }); + + bool get sheetIsOpen => find.byType(FocusableTextField).evaluate().isNotEmpty; +} + +Future<_Harness> _pumpPlayerWithOpenSearchSheet(WidgetTester tester) async { + final controller = TextEditingController(); + addTearDown(controller.dispose); + final fieldFocusNode = FocusNode(debugLabel: 'subtitleSearchField'); + addTearDown(fieldFocusNode.dispose); + final sheetButtonFocusNode = FocusNode(debugLabel: 'sheetButton'); + addTearDown(sheetButtonFocusNode.dispose); + final navigations = []; + + await tester.pumpWidget( + MaterialApp( + home: _PlayerShell( + controller: controller, + fieldFocusNode: fieldFocusNode, + sheetButtonFocusNode: sheetButtonFocusNode, + onPlayerNavigation: navigations.add, + ), + ), + ); + + await tester.tap(find.text('Search subtitles')); + await tester.pumpAndSettle(); + + return _Harness( + controller: controller, + fieldFocusNode: fieldFocusNode, + sheetButtonFocusNode: sheetButtonFocusNode, + navigations: navigations, + ); +} + +Future _focusFieldWithText(WidgetTester tester, _Harness harness, String text, {required int caret}) async { + await tester.tap(find.byType(TextField)); + await tester.pumpAndSettle(); + await tester.enterText(find.byType(TextField), text); + await tester.pumpAndSettle(); + harness.controller.selection = TextSelection.collapsed(offset: caret); + await tester.pump(); + expect(harness.fieldFocusNode.hasPrimaryFocus, isTrue); +} + +Future _pressKey(WidgetTester tester, LogicalKeyboardKey key) async { + await tester.sendKeyDownEvent(key); + await tester.sendKeyUpEvent(key); + await tester.pumpAndSettle(); +} + +void main() { + testWidgets('Backspace in the subtitle search field edits text instead of leaving the player', (tester) async { + final harness = await _pumpPlayerWithOpenSearchSheet(tester); + await _focusFieldWithText(tester, harness, 'star wars', caret: 5); + + await _pressKey(tester, LogicalKeyboardKey.backspace); + + expect(harness.controller.text, 'starwars'); + expect(harness.navigations, isEmpty); + expect(harness.sheetIsOpen, isTrue); + }); + + testWidgets('Home in the subtitle search field moves the caret instead of leaving the player', (tester) async { + final harness = await _pumpPlayerWithOpenSearchSheet(tester); + await _focusFieldWithText(tester, harness, 'star wars', caret: 5); + + await _pressKey(tester, LogicalKeyboardKey.home); + + // Proves the unhandled event reached DefaultTextEditingShortcuts rather + // than being consumed by the screen's navigation handler. + expect(harness.controller.selection, const TextSelection.collapsed(offset: 0)); + expect(harness.controller.text, 'star wars'); + expect(harness.navigations, isEmpty); + expect(harness.sheetIsOpen, isTrue); + }); + + testWidgets('Backspace on a non-text sheet control closes the sheet without leaving the player', (tester) async { + final harness = await _pumpPlayerWithOpenSearchSheet(tester); + harness.sheetButtonFocusNode.requestFocus(); + await tester.pumpAndSettle(); + + await _pressKey(tester, LogicalKeyboardKey.backspace); + + expect(harness.sheetIsOpen, isFalse); + expect(harness.navigations, isEmpty); + }); + + testWidgets('Backspace with no sheet open still drives player Back', (tester) async { + final harness = await _pumpPlayerWithOpenSearchSheet(tester); + harness.sheetButtonFocusNode.requestFocus(); + await tester.pumpAndSettle(); + await _pressKey(tester, LogicalKeyboardKey.backspace); + expect(harness.sheetIsOpen, isFalse); + + await _pressKey(tester, LogicalKeyboardKey.backspace); + + expect(harness.navigations, [PlayerNavigationKey.back]); + }); + + testWidgets('Escape still closes an open sheet', (tester) async { + final harness = await _pumpPlayerWithOpenSearchSheet(tester); + await _focusFieldWithText(tester, harness, 'star wars', caret: 5); + + await _pressKey(tester, LogicalKeyboardKey.escape); + + expect(harness.sheetIsOpen, isFalse); + expect(harness.navigations, isEmpty); + }); + + // Binds the fix to the real screen: the shell tests above cannot catch a + // regression of `_overlayChildKey`/`_sheetContext`, because they model the + // corrected wiring themselves. Resolving the host from the State's own + // context (which sits ABOVE it) silently returns null, so Back skips the + // sheet stage and runs the exit pipeline instead. + testWidgets('the player screen resolves its own sheet host before exiting', (tester) async { + resetSharedPreferencesForTest(); + SettingsService.resetForTesting(); + await SettingsService.getInstance(); + + final nativeInitialize = Completer(); + final navigatorKey = GlobalKey(); + final sheetButtonFocusNode = FocusNode(debugLabel: 'playerSheetButton'); + addTearDown(sheetButtonFocusNode.dispose); + + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + // Never completes: the screen stays on its initialization surface, so no + // real player is needed and the chrome is not presented — the state in + // which a leaked Back goes straight to exitPlayer. + methodHandler: (call) => call.method == 'initialize' ? nativeInitialize.future : Future.value(), + eventHandler: (_) async => null, + testBody: () async { + await tester.pumpWidget( + ChangeNotifierProvider( + create: (_) => PlaybackStateProvider(), + child: MaterialApp( + navigatorKey: navigatorKey, + home: const Scaffold(body: Center(child: Text('behind the player'))), + ), + ), + ); + + unawaited( + navigatorKey.currentState!.push( + MaterialPageRoute( + builder: (_) => VideoPlayerScreen(metadata: testMediaItem(title: 'Sheet host test'), isOffline: true), + ), + ), + ); + // The loading spinner animates forever, so pumpAndSettle would hang. + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + expect(find.byType(VideoPlayerScreen), findsOneWidget); + + // A context below the screen's own OverlaySheetHost. + final sheetContext = tester.element(find.byType(CircularProgressIndicator)); + unawaited( + OverlaySheetController.of(sheetContext).show( + builder: (_) => + TextButton(focusNode: sheetButtonFocusNode, onPressed: () {}, child: const Text('Sheet action')), + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + expect(find.text('Sheet action'), findsOneWidget); + + sheetButtonFocusNode.requestFocus(); + await tester.pump(); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.backspace); + await tester.sendKeyUpEvent(LogicalKeyboardKey.backspace); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + await tester.pump(const Duration(seconds: 1)); + + expect(find.text('Sheet action'), findsNothing, reason: 'Back must close the sheet'); + expect(find.byType(VideoPlayerScreen), findsOneWidget, reason: 'Back must not also leave the player'); + + await tester.pumpWidget(const SizedBox.shrink()); + nativeInitialize.complete(true); + await tester.pump(); + }, + ); + }); +} diff --git a/test/widgets/video_controls_test.dart b/test/widgets/video_controls_test.dart index bc9817d7..231b8a25 100644 --- a/test/widgets/video_controls_test.dart +++ b/test/widgets/video_controls_test.dart @@ -362,6 +362,56 @@ void main() { ); } }); + + test('surrenders bare Backspace to a focused text editor', () { + final event = _navigationKeyDown(LogicalKeyboardKey.backspace, ui.KeyEventDeviceType.keyboard); + + expect( + classifyPlayerNavigationKey(event, isAppleTV: false, hasModifiers: false, textEditingActive: true), + PlayerNavigationKey.none, + ); + expect( + classifyPlayerNavigationKey(event, isAppleTV: false, hasModifiers: false, textEditingActive: false), + PlayerNavigationKey.back, + ); + }); + + test('surrenders bare Home to a focused text editor but never browser Home', () { + expect( + classifyPlayerNavigationKey( + _navigationKeyDown(LogicalKeyboardKey.home, ui.KeyEventDeviceType.keyboard), + isAppleTV: false, + hasModifiers: false, + textEditingActive: true, + ), + PlayerNavigationKey.none, + ); + // browserHome has no caret role, so an editor never takes it. + expect( + classifyPlayerNavigationKey( + _navigationKeyDown(LogicalKeyboardKey.browserHome, ui.KeyEventDeviceType.keyboard), + isAppleTV: false, + hasModifiers: false, + textEditingActive: true, + ), + PlayerNavigationKey.home, + ); + }); + + test('keeps simulated remote Home navigating while a text editor has focus', () { + for (final deviceType in [ui.KeyEventDeviceType.directionalPad, ui.KeyEventDeviceType.gamepad]) { + expect( + classifyPlayerNavigationKey( + _navigationKeyDown(LogicalKeyboardKey.home, deviceType), + isAppleTV: false, + hasModifiers: false, + textEditingActive: true, + ), + PlayerNavigationKey.home, + reason: 'a synthesized remote press has no caret to move', + ); + } + }); }); group('handlePlayerNavigationKeyAction', () {