diff --git a/lib/screens/discover_screen.dart b/lib/screens/discover_screen.dart index c06da5804..0cec8e3fd 100644 --- a/lib/screens/discover_screen.dart +++ b/lib/screens/discover_screen.dart @@ -266,7 +266,7 @@ class _DiscoverScreenState extends State WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted) return; - if (!_isTabVisible || !(ModalRoute.of(context)?.isCurrent ?? false)) { + if (!_isTabVisible || !(ModalRoute.of(context)?.isCurrent ?? false) || _focusHasLeftScreen) { _pendingTvBrowseRailFocus = false; return; } @@ -278,8 +278,26 @@ class _DiscoverScreenState extends State }); } + /// A rail-focus request stays armed while the rail has no hubs to focus + /// (empty first load), so hubs landing later still receive it. It must not + /// outlive the user's own navigation: once focus sits on a control off this + /// screen (a sidebar item), hubs arriving minutes later would yank the + /// remote back. A bare scope — MainScreen's content scope before any child + /// has focus — is "nowhere yet", not a destination, and keeps the request. + bool get _focusHasLeftScreen { + final node = FocusManager.instance.primaryFocus; + final focusContext = node?.context; + if (node == null || node is FocusScopeNode || focusContext == null) return false; + return !identical(focusContext.findAncestorStateOfType<_DiscoverScreenState>(), this); + } + void _applyPendingTvBrowseRailFocus() { - if (_pendingTvBrowseRailFocus) _focusTvBrowseRailWhenReady(); + if (!_pendingTvBrowseRailFocus) return; + if (_focusHasLeftScreen) { + _pendingTvBrowseRailFocus = false; + return; + } + _focusTvBrowseRailWhenReady(); } /// Handle vertical navigation between hubs diff --git a/lib/screens/main_screen.dart b/lib/screens/main_screen.dart index bdd1b1fbb..0044e0ed9 100644 --- a/lib/screens/main_screen.dart +++ b/lib/screens/main_screen.dart @@ -1493,9 +1493,15 @@ class _MainScreenState extends State // The tvOS engine normally passes root Menu presses through to UIKit. If a // stale event still reaches Flutter, avoid showing an exit prompt that - // cannot be honored app-side. + // cannot be honored app-side. Log it: a Menu swallowed here with the + // picker up or focus off this route is how #2239 read as a dead remote. if (PlatformDetector.isAppleTV()) { _lastBackPressAt = null; + appLogger.d( + 'tvOS Menu reached MainScreen at the home root: ' + 'passthrough=$_shouldPassTvosMenuToSystem picker=$_isShowingProfileSelection ' + 'focus=${FocusManager.instance.primaryFocus?.debugLabel}', + ); return KeyEventResult.handled; } diff --git a/lib/services/system_shelf_service.dart b/lib/services/system_shelf_service.dart index 120f782b2..fff84bb84 100644 --- a/lib/services/system_shelf_service.dart +++ b/lib/services/system_shelf_service.dart @@ -62,9 +62,14 @@ class SystemShelfService { @visibleForTesting int get debugGeneration => _generation; + /// Forgets the owner and drops the mutation queue. Deliberately does not + /// await the old tail: a widget test's FakeAsync zone ends without + /// flushing the microtasks that settle a chained future, so the previous + /// test's final clear would leave a tail that never completes and hang the + /// next test's setUp. Tests own their channel fakes, so nothing native is + /// lost by abandoning the chain. @visibleForTesting - Future debugReset() async { - await _mutationTail; + void debugReset() { _activeOwner = null; _generation = 0; _mutationTail = Future.value(); diff --git a/test/navigation/profile_session_focus_boundary_test.dart b/test/navigation/profile_session_focus_boundary_test.dart deleted file mode 100644 index 15c88d1af..000000000 --- a/test/navigation/profile_session_focus_boundary_test.dart +++ /dev/null @@ -1,157 +0,0 @@ -import 'package:drift/native.dart'; -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:plezy/connection/connection_registry.dart'; -import 'package:plezy/database/app_database.dart'; -import 'package:plezy/models/plex/plex_home_user.dart'; -import 'package:plezy/navigation/profile_session_screen.dart'; -import 'package:plezy/profiles/active_profile_provider.dart'; -import 'package:plezy/profiles/plex_home_service.dart'; -import 'package:plezy/profiles/profile.dart'; -import 'package:plezy/profiles/profile_connection_registry.dart'; -import 'package:plezy/profiles/profile_registry.dart'; -import 'package:plezy/providers/multi_server_provider.dart'; -import 'package:plezy/services/multi_server_manager.dart'; -import 'package:plezy/services/offline_watch_sync_service.dart'; -import 'package:plezy/services/storage_service.dart'; -import 'package:plezy/services/system_shelf_service.dart'; -import 'package:provider/provider.dart'; - -import '../test_helpers/io_fakes.dart'; -import '../test_helpers/multi_server_fixtures.dart'; -import '../test_helpers/prefs.dart'; - -// Own file: SystemShelfService.debugReset awaits the mutation tail queued by -// the previous test's profile switch, which never settles outside that test's -// zone, so a second testWidgets in profile_session_screen_test.dart hangs. -void main() { - TestWidgetsFlutterBinding.ensureInitialized(); - - setUp(() async { - resetSharedPreferencesForTest(); - await SystemShelfService().debugReset(); - }); - - testWidgets( - 'a root-navigator route over the session blocks focus steals below it and hands focus back on pop (#2239)', - (tester) async { - final db = AppDatabase.forTesting(NativeDatabase.memory()); - final profileRegistry = ProfileRegistry(db); - final connectionRegistry = ConnectionRegistry(db); - final profileConnectionRegistry = ProfileConnectionRegistry(db); - final storage = await StorageService.getInstance(); - final plexHome = _FakePlexHomeService( - connections: connectionRegistry, - profileConnections: profileConnectionRegistry, - storage: storage, - ); - final activeProfile = ActiveProfileProvider( - registry: profileRegistry, - plexHome: plexHome, - connections: connectionRegistry, - profileConnections: profileConnectionRegistry, - storage: storage, - ); - final serverManager = MultiServerManager(); - final multiServer = testMultiServerProvider(serverManager); - final offlineWatch = OfflineWatchSyncService(database: db, serverManager: serverManager); - final rootNavigator = GlobalKey(); - final content = FocusNode(debugLabel: 'SessionContent'); - final sidebar = FocusNode(debugLabel: 'SessionSidebar'); - final picker = FocusNode(debugLabel: 'RootPicker'); - - addTearDown(() async { - await tester.pumpWidget(const SizedBox.shrink()); - await tester.pumpAndSettle(); - content.dispose(); - sidebar.dispose(); - picker.dispose(); - await activeProfile.resetForTesting(); - activeProfile.dispose(); - multiServer.dispose(); - serverManager.dispose(); - await plexHome.dispose(); - offlineWatch.dispose(); - await db.close(); - }); - - final owner = Profile.local(id: 'local-owner', displayName: 'Owner', createdAt: DateTime(2026, 1, 1)); - await profileRegistry.upsert(owner); - await storage.setActiveProfileId(owner.id); - await activeProfile.initialize(); - - await tester.pumpWidget( - MultiProvider( - providers: [ - Provider.value(value: storage), - Provider.value(value: db), - Provider.value(value: connectionRegistry), - Provider.value(value: profileConnectionRegistry), - Provider.value(value: plexHome), - ChangeNotifierProvider.value(value: activeProfile), - ChangeNotifierProvider.value(value: multiServer), - ChangeNotifierProvider.value(value: offlineWatch), - ], - child: MaterialApp( - navigatorKey: rootNavigator, - home: ProfileSessionScreen.forTesting( - initialPromptHandled: true, - httpClientFactory: () => FakeHttpClient(200, const []), - profileShellBuilder: (context) => Column( - children: [ - Focus(focusNode: sidebar, child: const SizedBox(height: 10, width: 10)), - Focus(focusNode: content, autofocus: true, child: const SizedBox(height: 10, width: 10)), - ], - ), - ), - ), - ), - ); - await tester.pump(); - await tester.pump(const Duration(seconds: 1)); - expect(content.hasPrimaryFocus, isTrue); - - // The picker/PIN shape: pushed on the root navigator, above the whole - // nested profile-session navigator. - rootNavigator.currentState!.push( - MaterialPageRoute( - builder: (_) => Focus(focusNode: picker, autofocus: true, child: const SizedBox.expand()), - ), - ); - await tester.pump(); - await tester.pump(const Duration(seconds: 1)); - expect(picker.hasPrimaryFocus, isTrue); - - // A resume-time self-heal under MainScreen (sidebar reveal, grid load). - sidebar.requestFocus(); - await tester.pump(); - expect(sidebar.hasPrimaryFocus, isFalse, reason: 'covered session must not take focus'); - expect(picker.hasPrimaryFocus, isTrue); - - rootNavigator.currentState!.pop(); - await tester.pump(); - await tester.pump(const Duration(seconds: 1)); - expect(content.hasPrimaryFocus, isTrue, reason: 'focus returns to the session leaf that had it'); - }, - ); -} - -class _FakePlexHomeService extends PlexHomeService { - _FakePlexHomeService({required super.connections, required super.profileConnections, required StorageService storage}) - : super(storage: storage, plexHomeUserFetcher: (_) async => const []); - - @override - Map> get current => const {}; - - @override - Stream>> get stream => Stream.value(const {}); - - @override - Future start() async {} - - @override - Future reloadFromStorage() async {} - - @override - Future dispose() async {} -} diff --git a/test/navigation/profile_session_screen_test.dart b/test/navigation/profile_session_screen_test.dart index 673a17ff4..5a4fed979 100644 --- a/test/navigation/profile_session_screen_test.dart +++ b/test/navigation/profile_session_screen_test.dart @@ -31,7 +31,7 @@ void main() { setUp(() async { resetSharedPreferencesForTest(); - await SystemShelfService().debugReset(); + SystemShelfService().debugReset(); }); testWidgets('profile switch disposes the profile navigator, routes, and providers', (tester) async { @@ -186,6 +186,109 @@ void main() { expect(trackerHttpClients.toSet(), hasLength(trackerAuthClientsPerProfile * 3)); _expectCloseCount(trackerHttpClients, 1); }); + + testWidgets( + 'a root-navigator route over the session blocks focus steals below it and hands focus back on pop (#2239)', + (tester) async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + final profileRegistry = ProfileRegistry(db); + final connectionRegistry = ConnectionRegistry(db); + final profileConnectionRegistry = ProfileConnectionRegistry(db); + final storage = await StorageService.getInstance(); + final plexHome = _FakePlexHomeService( + connections: connectionRegistry, + profileConnections: profileConnectionRegistry, + storage: storage, + ); + final activeProfile = ActiveProfileProvider( + registry: profileRegistry, + plexHome: plexHome, + connections: connectionRegistry, + profileConnections: profileConnectionRegistry, + storage: storage, + ); + final serverManager = MultiServerManager(); + final multiServer = testMultiServerProvider(serverManager); + final offlineWatch = OfflineWatchSyncService(database: db, serverManager: serverManager); + final rootNavigator = GlobalKey(); + final content = FocusNode(debugLabel: 'SessionContent'); + final sidebar = FocusNode(debugLabel: 'SessionSidebar'); + final picker = FocusNode(debugLabel: 'RootPicker'); + + addTearDown(() async { + await tester.pumpWidget(const SizedBox.shrink()); + await tester.pumpAndSettle(); + content.dispose(); + sidebar.dispose(); + picker.dispose(); + await activeProfile.resetForTesting(); + activeProfile.dispose(); + multiServer.dispose(); + serverManager.dispose(); + await plexHome.dispose(); + offlineWatch.dispose(); + await db.close(); + }); + + final owner = Profile.local(id: 'local-owner', displayName: 'Owner', createdAt: DateTime(2026, 1, 1)); + await profileRegistry.upsert(owner); + await storage.setActiveProfileId(owner.id); + await activeProfile.initialize(); + + await tester.pumpWidget( + MultiProvider( + providers: [ + Provider.value(value: storage), + Provider.value(value: db), + Provider.value(value: connectionRegistry), + Provider.value(value: profileConnectionRegistry), + Provider.value(value: plexHome), + ChangeNotifierProvider.value(value: activeProfile), + ChangeNotifierProvider.value(value: multiServer), + ChangeNotifierProvider.value(value: offlineWatch), + ], + child: MaterialApp( + navigatorKey: rootNavigator, + home: ProfileSessionScreen.forTesting( + initialPromptHandled: true, + httpClientFactory: () => FakeHttpClient(200, const []), + profileShellBuilder: (context) => Column( + children: [ + Focus(focusNode: sidebar, child: const SizedBox(height: 10, width: 10)), + Focus(focusNode: content, autofocus: true, child: const SizedBox(height: 10, width: 10)), + ], + ), + ), + ), + ), + ); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + expect(content.hasPrimaryFocus, isTrue); + + // The picker/PIN shape: pushed on the root navigator, above the whole + // nested profile-session navigator. + rootNavigator.currentState!.push( + MaterialPageRoute( + builder: (_) => Focus(focusNode: picker, autofocus: true, child: const SizedBox.expand()), + ), + ); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + expect(picker.hasPrimaryFocus, isTrue); + + // A resume-time self-heal under MainScreen (sidebar reveal, grid load). + sidebar.requestFocus(); + await tester.pump(); + expect(sidebar.hasPrimaryFocus, isFalse, reason: 'covered session must not take focus'); + expect(picker.hasPrimaryFocus, isTrue); + + rootNavigator.currentState!.pop(); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + expect(content.hasPrimaryFocus, isTrue, reason: 'focus returns to the session leaf that had it'); + }, + ); } void _expectCloseCount(Iterable clients, int expected) { diff --git a/test/screens/discover_screen_test.dart b/test/screens/discover_screen_test.dart index 737a19435..0150ae92b 100644 --- a/test/screens/discover_screen_test.dart +++ b/test/screens/discover_screen_test.dart @@ -750,6 +750,164 @@ void main() { expect(find.byType(SystemClock), findsOneWidget, reason: 'a fullscreen leanback app hides the system clock'); await tester.pumpWidget(const SizedBox()); }); + + testWidgets('a pending TV rail focus is dropped once the user has moved to the sidebar', (tester) async { + // The first load arms "focus the rail when it has hubs"; an empty-items + // hub keeps it armed because the rail has nothing to render yet. Items + // landing on a later pass must not pull focus off a sidebar item the + // user has since navigated to. + final gated = await _pumpGatedDiscover(tester); + gated.sidebarItem.requestFocus(); + await tester.pump(); + + await gated.landItems(tester); + + expect(find.byType(TvBrowseRail), findsOneWidget); + expect( + FocusManager.instance.primaryFocus, + same(gated.sidebarItem), + reason: 'items landing must not steal the sidebar', + ); + }); + + testWidgets('a pending TV rail focus still claims the rail while focus sits on a bare scope', (tester) async { + // Startup shape: MainScreen has focused its content scope but nothing + // below it yet. That is "nowhere", not a destination the user chose, so + // the rail keeps its initial-focus claim when hubs finally arrive. + final gated = await _pumpGatedDiscover(tester); + expect(FocusManager.instance.primaryFocus, isA()); + + await gated.landItems(tester); + + expect(FocusManager.instance.primaryFocus?.debugLabel, 'tv_browse_rail'); + }); +} + +class _GatedDiscover { + _GatedDiscover({required this.client, required this.sidebarItem}); + + final _GatedHubsFakeClient client; + final FocusNode sidebarItem; + + /// Replace the empty-items hub with a populated one and run a full pass. + Future landItems(WidgetTester tester) async { + client.hubs + ..clear() + ..add( + MediaHub( + id: 'hub_1', + title: 'Recommended', + type: 'movie', + items: [testMediaItem(id: 'movie_1', backend: MediaBackend.plex, kind: MediaKind.movie, title: 'Movie 1')], + size: 1, + ), + ); + (tester.state(find.byType(DiscoverScreen)) as FullRefreshable).fullRefresh(); + await tester.pump(); + client.release(); + await tester.pump(); + await tester.pump(); + } +} + +/// Mounts a TV [DiscoverScreen] beside a sidebar stand-in and completes its +/// first load with a hub that has no items, so the rail has nothing to render +/// and the screen's "focus the rail when ready" request stays armed. +Future<_GatedDiscover> _pumpGatedDiscover(WidgetTester tester) async { + await SettingsService.getInstance(); + tester.view.devicePixelRatio = 1.0; + tester.view.physicalSize = const Size(1280, 720); + addTearDown(() { + tester.view.resetDevicePixelRatio(); + tester.view.resetPhysicalSize(); + }); + + final client = _GatedHubsFakeClient( + hubs: [MediaHub(id: 'hub_1', title: 'Recommended', type: 'movie', items: const [], size: 0)], + ); + final multiServerProvider = testMultiServer(clients: [client]).provider; + final hiddenLibrariesProvider = HiddenLibrariesProvider(); + final librariesProvider = LibrariesProvider(); + final watchTogetherProvider = WatchTogetherProvider(); + final companionRemoteProvider = CompanionRemoteProvider(); + final db = AppDatabase.forTesting(NativeDatabase.memory()); + final connectionRegistry = _FakeConnectionRegistry(db); + final profileConnectionRegistry = _FakeProfileConnectionRegistry(db); + final storage = await StorageService.getInstance(); + final plexHome = PlexHomeService( + connections: connectionRegistry, + profileConnections: profileConnectionRegistry, + storage: storage, + plexHomeUserFetcher: (_) async => const [], + ); + final activeProfileProvider = ActiveProfileProvider( + registry: _FakeProfileRegistry(db), + plexHome: plexHome, + connections: connectionRegistry, + profileConnections: profileConnectionRegistry, + storage: storage, + ); + final discoverProvider = DiscoverProvider( + multiServerProvider, + hiddenLibrariesProvider, + librariesProvider, + profileId: null, + isProfileBinding: () => false, + ); + final sidebarItem = FocusNode(debugLabel: 'sidebar_item'); + addTearDown(() async { + sidebarItem.dispose(); + discoverProvider.dispose(); + activeProfileProvider.dispose(); + companionRemoteProvider.dispose(); + watchTogetherProvider.dispose(); + librariesProvider.dispose(); + hiddenLibrariesProvider.dispose(); + // multiServerProvider + its manager are torn down by testMultiServer. + await plexHome.dispose(); + await db.close(); + }); + + await tester.pumpWidget( + TranslationProvider( + child: MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: multiServerProvider), + ChangeNotifierProvider.value(value: hiddenLibrariesProvider), + ChangeNotifierProvider.value(value: librariesProvider), + ChangeNotifierProvider.value(value: discoverProvider), + ChangeNotifierProvider.value(value: watchTogetherProvider), + ChangeNotifierProvider.value(value: companionRemoteProvider), + ChangeNotifierProvider.value(value: activeProfileProvider), + ], + child: InputModeTracker( + child: MaterialApp( + theme: monoTheme(dark: true), + home: Row( + children: [ + Focus(focusNode: sidebarItem, child: const SizedBox(width: 80, height: 720)), + MainScreenFocusScope( + focusSidebar: () {}, + sideNavigationWidth: SideNavigationRailState.expandedWidth, + reservedSideNavigationWidth: SideNavigationRailState.tvCollapsedWidth, + foregroundLeft: 80, + foregroundWidth: 1200, + viewportWidth: 1280, + child: const SizedBox(width: 1200, height: 720, child: DiscoverScreen()), + ), + ], + ), + ), + ), + ), + ), + ); + await tester.pump(); + client.release(); + await tester.pump(); + await tester.pump(); + expect(find.byType(TvBrowseRail), findsNothing, reason: 'an empty-items hub gives the rail nothing to show'); + return _GatedDiscover(client: client, sidebarItem: sidebarItem); } /// Mounts [DiscoverScreen] in the smallest graph both layout branches need, so