From 5890a2fc736fa8ba0ed9c31860709ed09c883ee7 Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Mon, 20 May 2024 16:45:08 -0700 Subject: [PATCH] SelectionArea's selection should not be cleared on loss of window focus (#148067) This change fixes an issue where SelectionArea would clear its selection when the application window lost focus by first checking if the application is running. This is needed because `FocusManager` is aware of the application lifecycle as of https://github.com/flutter/flutter/pull/142930 , and triggers a focus lost if the application is not active. Also fixes an issue where the `FocusManager` was not being reset on tests at the right time, causing it always to build with `TargetPlatform.android` as its context. --- .../lib/src/widgets/focus_manager.dart | 28 +++++++++++ .../lib/src/widgets/selectable_region.dart | 11 ++++- .../test/widgets/focus_manager_test.dart | 2 +- .../test/widgets/selectable_region_test.dart | 48 +++++++++++++++++++ packages/flutter_test/lib/src/binding.dart | 8 ++++ .../flutter_test/lib/src/widget_tester.dart | 2 +- 6 files changed, 96 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart index 002553a11a..f27293118f 100644 --- a/packages/flutter/lib/src/widgets/focus_manager.dart +++ b/packages/flutter/lib/src/widgets/focus_manager.dart @@ -1873,6 +1873,34 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { }()); } + /// Enables this [FocusManager] to listen to changes of the application + /// lifecycle if it does not already have an application lifecycle listener + /// active, and the current platform is detected as [kIsWeb] or non-Android. + /// + /// Typically, the application lifecycle listener for this [FocusManager] is + /// setup at construction, but sometimes it is necessary to manually initialize + /// it when the [FocusManager] does not have the relevant platform context in + /// [defaultTargetPlatform] at the time of construction. This can happen in + /// a test environment where the [BuildOwner] which initializes its own + /// [FocusManager], may not have the accurate platform context during its + /// initialization. In this case it is necessary for the test framework to call + /// this method after it has set up the test variant for a given test, so the + /// [FocusManager] can accurately listen to application lifecycle changes, if + /// supported. + @visibleForTesting + void listenToApplicationLifecycleChangesIfSupported() { + if (_appLifecycleListener == null && (kIsWeb || defaultTargetPlatform != TargetPlatform.android)) { + // It appears that some Android keyboard implementations can cause + // app lifecycle state changes: adding this listener would cause the + // text field to unfocus as the user is trying to type. + // + // Until this is resolved, we won't be adding the listener to Android apps. + // https://github.com/flutter/flutter/pull/142930#issuecomment-1981750069 + _appLifecycleListener = _AppLifecycleListener(_appLifecycleChange); + WidgetsBinding.instance.addObserver(_appLifecycleListener!); + } + } + @override List debugDescribeChildren() { return [ diff --git a/packages/flutter/lib/src/widgets/selectable_region.dart b/packages/flutter/lib/src/widgets/selectable_region.dart index 62f30cb309..3a24a288e0 100644 --- a/packages/flutter/lib/src/widgets/selectable_region.dart +++ b/packages/flutter/lib/src/widgets/selectable_region.dart @@ -454,7 +454,16 @@ class SelectableRegionState extends State with TextSelectionDe if (kIsWeb) { PlatformSelectableRegionContextMenu.detach(_selectionDelegate); } - _clearSelection(); + if (SchedulerBinding.instance.lifecycleState == AppLifecycleState.resumed) { + // We should only clear the selection when this SelectableRegion loses + // focus while the application is currently running. It is possible + // that the application is not currently running, for example on desktop + // platforms, clicking on a different window switches the focus to + // the new window causing the Flutter application to go inactive. In this + // case we want to retain the selection so it remains when we return to + // the Flutter application. + _clearSelection(); + } } if (kIsWeb) { PlatformSelectableRegionContextMenu.attach(_selectionDelegate); diff --git a/packages/flutter/test/widgets/focus_manager_test.dart b/packages/flutter/test/widgets/focus_manager_test.dart index 69b2e2a765..fcd8c4a83d 100644 --- a/packages/flutter/test/widgets/focus_manager_test.dart +++ b/packages/flutter/test/widgets/focus_manager_test.dart @@ -416,7 +416,7 @@ void main() { await setAppLifecycleState(AppLifecycleState.resumed); expect(focusNode.hasPrimaryFocus, isTrue); - }); + }, variant: TargetPlatformVariant.desktop()); testWidgets('Node is removed completely even if app is paused.', (WidgetTester tester) async { Future setAppLifecycleState(AppLifecycleState state) async { diff --git a/packages/flutter/test/widgets/selectable_region_test.dart b/packages/flutter/test/widgets/selectable_region_test.dart index 859026432d..6877d6f9b7 100644 --- a/packages/flutter/test/widgets/selectable_region_test.dart +++ b/packages/flutter/test/widgets/selectable_region_test.dart @@ -548,6 +548,54 @@ void main() { }, variant: TargetPlatformVariant.all()); group('SelectionArea integration', () { + testWidgets('selection is not cleared when app loses focus on desktop', (WidgetTester tester) async { + Future setAppLifecycleState(AppLifecycleState state) async { + final ByteData? message = const StringCodec().encodeMessage(state.toString()); + await TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .handlePlatformMessage('flutter/lifecycle', message, (_) {}); + } + final FocusNode focusNode = FocusNode(); + final GlobalKey selectableKey = GlobalKey(); + addTearDown(focusNode.dispose); + await tester.pumpWidget( + MaterialApp( + home: SelectableRegion( + key: selectableKey, + focusNode: focusNode, + selectionControls: materialTextSelectionControls, + child: const Center( + child: Text('How are you'), + ), + ), + ), + ); + await setAppLifecycleState(AppLifecycleState.resumed); + await tester.pumpAndSettle(); + + final RenderParagraph paragraph = tester.renderObject(find.descendant(of: find.text('How are you'), matching: find.byType(RichText))); + final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph, 2), kind: PointerDeviceKind.mouse); + addTearDown(gesture.removePointer); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + await gesture.down(textOffsetToPosition(paragraph, 2)); + await tester.pumpAndSettle(); + expect(paragraph.selections[0], const TextSelection(baseOffset: 0, extentOffset: 3)); + + await gesture.up(); + await tester.pumpAndSettle(); + expect(paragraph.selections[0], const TextSelection(baseOffset: 0, extentOffset: 3)); + expect(focusNode.hasFocus, isTrue); + + // Setting the app lifecycle state to AppLifecycleState.inactive to simulate + // a lose of window focus. + await setAppLifecycleState(AppLifecycleState.inactive); + await tester.pumpAndSettle(); + expect(focusNode.hasFocus, isFalse); + expect(paragraph.selections[0], const TextSelection(baseOffset: 0, extentOffset: 3)); + }, variant: TargetPlatformVariant.desktop()); + testWidgets('mouse can select single text on desktop platforms', (WidgetTester tester) async { final FocusNode focusNode = FocusNode(); addTearDown(focusNode.dispose); diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index 87fbce0398..84523808e7 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -254,6 +254,14 @@ abstract class TestWidgetsFlutterBinding extends BindingBase _testTextInput.register(); } CustomSemanticsAction.resetForTests(); // ignore: invalid_use_of_visible_for_testing_member + _enableFocusManagerLifecycleAwarenessIfSupported(); + } + + void _enableFocusManagerLifecycleAwarenessIfSupported() { + if (buildOwner == null) { + return; + } + buildOwner!.focusManager.listenToApplicationLifecycleChangesIfSupported(); // ignore: invalid_use_of_visible_for_testing_member } @override diff --git a/packages/flutter_test/lib/src/widget_tester.dart b/packages/flutter_test/lib/src/widget_tester.dart index 74e3b196e5..dec236910e 100644 --- a/packages/flutter_test/lib/src/widget_tester.dart +++ b/packages/flutter_test/lib/src/widget_tester.dart @@ -174,11 +174,11 @@ void testWidgets( test_package.addTearDown(binding.postTest); return binding.runTest( () async { - binding.reset(); // TODO(ianh): the binding should just do this itself in _runTest debugResetSemanticsIdCounter(); Object? memento; try { memento = await variant.setUp(value); + binding.reset(); // TODO(ianh): the binding should just do this itself in _runTest maybeSetupLeakTrackingForTest(experimentalLeakTesting, combinedDescription); await callback(tester); } finally {