flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
162.22k stars 26.65k forks source link

[MacOS] Exiting a Dialog w/ TextField from RawKeyboardListener.onKey breaks all TextFields in-app #143270

Closed Xazin closed 2 weeks ago

Xazin commented 3 months ago

Steps to reproduce

Using a RawKeyboardListener.onKey that wraps a TextField, to close a Dialog using Navigator.pop.

Notice: Could not be reproduced on Windows, but reproduced on multiple MacOS devices. Cannot be reproduced on stable, only on beta (both 3.18 and 3.19)

Expected results

TextFields can receive and act on input normally.

Actual results

All TextFields are broken and can no longer function properly, eg. backspace doesn't work.

Code sample

Code sample ```dart import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; void main() { runApp(const MyApp()); } class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return MaterialApp( home: const Home(), theme: ThemeData.light(useMaterial3: true), ); } } class Home extends StatefulWidget { const Home({super.key}); @override State createState() => _HomeState(); } class _HomeState extends State { String val = 'Hello World'; @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(title: const Text('TextField breaks')), body: Column( children: [ Text('Value: $val'), TextButton( onPressed: () => showDialog( context: context, builder: (context) => ADialog( value: val, ), ).then((v) { if (mounted && v != null && v != val) { setState(() => val = v); } }), child: const Text('Change'), ), const TextField(), ], ), ); } } class ADialog extends StatefulWidget { const ADialog({super.key, required this.value}); final String value; @override State createState() => ADialogState(); } class ADialogState extends State { final focusNode = FocusNode(); late final controller = TextEditingController(text: widget.value); @override void dispose() { focusNode.dispose(); controller.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return AlertDialog( title: const Text('Edit text'), content: RawKeyboardListener( focusNode: focusNode, onKey: (event) { if (event is! RawKeyDownEvent) return; if (event.logicalKey == LogicalKeyboardKey.escape) { Navigator.of(context).pop(); } }, child: Column( mainAxisSize: MainAxisSize.min, children: [ TextField( controller: controller, autofocus: true, ), TextButton( onPressed: () => Navigator.of(context).pop(controller.text), child: const Text('Save'), ) ], ), ), ); } } ```

Screenshots or Video

Screenshots / Video demonstration https://github.com/flutter/flutter/assets/42929161/a688898e-e684-4723-bf89-9dfe635d8808

Logs

Nothing at all in the logs.

Flutter Doctor output

Doctor output ```console [✓] Flutter (Channel beta, 3.19.0-0.4.pre, on macOS 14.2 23C64 darwin-arm64, locale en-DK) • Flutter version 3.19.0-0.4.pre on channel beta at /Users/mathias/Development/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision b7e7d46a04 (8 days ago), 2024-02-02 08:21:06 -0600 • Engine revision 98820f0a77 • Dart version 3.3.0 (build 3.3.0-279.3.beta) • DevTools version 2.31.0 [✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0) • Android SDK at /Users/mathias/Library/Android/sdk • Platform android-34, build-tools 34.0.0 • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 15.1) • Xcode at /Applications/Xcode.app/Contents/Developer • Build 15C65 • CocoaPods version 1.14.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2022.3) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231) [✓] VS Code (version 1.85.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.82.0 [✓] Connected device (4 available) • Super iPhone (mobile) • 00008110-00123CD634D9401E • ios • iOS 17.2.1 21C66 • iPhone 15 Pro (mobile) • 4DE2390A-F63A-4355-AB41-51C0E40E4913 • ios • com.apple.CoreSimulator.SimRuntime.iOS-17-2 (simulator) • macOS (desktop) • macos • darwin-arm64 • macOS 14.2 23C64 darwin-arm64 • Chrome (web) • chrome • web-javascript • Google Chrome 121.0.6167.160 [✓] Network resources • All expected network resources are available. • No issues found! ```
darshankawar commented 3 months ago

@Xazin Thanks for the report. The widget RawKeyboardListener you are using listens to the key events only when its focus node has focus. Also, I observed that this widget / api is now deprecated, per:

Screenshot 2024-02-12 at 3 31 02 PM

So, can you try KeyboardListener instead of it and check if you still get same behavior or not ? https://api.flutter.dev/flutter/widgets/KeyboardListener-class.html

Xazin commented 3 months ago

@Xazin Thanks for the report.

The widget RawKeyboardListener you are using listens to the key events only when its focus node has focus.

Also, I observed that this widget / api is now deprecated, per:

Screenshot 2024-02-12 at 3 31 02 PM

So, can you try KeyboardListener instead of it and check if you still get same behavior or not ?

https://api.flutter.dev/flutter/widgets/KeyboardListener-class.html

I already tried KeyboardListener, and it didn't change the result. This was my first idea as well, I just reported using RawKeyboardListener as that is what we primarily use in our applications (which I will make sure to replace), as we often need to check for modifiers such as if shift is also pressed.

There is no issue on stable, which suggests something broke somewhere.

Our application (AppFlowy) is heavily reliant on text editing, our own implementations of editable text does not break, but all common usage of TextFields does break throughout the app.

darshankawar commented 3 months ago

Thanks for the update. I was able to replicate the reported behavior on desktop only (tried on macos), while on web and mobile, it works as expected. The reported behavior occurs on latest beta and master, while on stable, it works as expected. Below are the exact steps to replicate the behavior.

  1. Be on latest master or beta and run flutter run -d macos:
  2. Input any text in the empty textfield (ex: this is test).
  3. Tap Change button.
  4. Update the existing text to any and hit save, which will close the dialog and show updated text.
  5. Tap Change button again and append any text and hit escape (without saving), which will close the dialog.
  6. Now, in the textfield, try to delete the text entered in step 2 above, and notice that it doesn't perform any action.
  7. Tap Change button again and try to delete the existing text or hit escape button which again doesn't perform any action.

Adding regression label, since it works on latest stable, but not on latest master and beta.

stable, master flutter doctor -v ``` [!] Flutter (Channel stable, 3.16.9, on macOS 12.2.1 21D62 darwin-x64, locale en-GB) • Flutter version 3.16.9 on channel stable at /Users/dhs/documents/fluttersdk/flutter ! Warning: `flutter` on your path resolves to /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter. Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path. ! Warning: `dart` on your path resolves to /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter. Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision 41456452f2 (12 days ago), 2024-01-25 10:06:23 -0800 • Engine revision f40e976bed • Dart version 3.2.6 • DevTools version 2.28.5 • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades. [!] Xcode - develop for iOS and macOS (Xcode 12.3) • Xcode at /Applications/Xcode.app/Contents/Developer ! Flutter recommends a minimum Xcode version of 13. Download the latest version or update via the Mac App Store. • CocoaPods version 1.11.2 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] VS Code (version 1.62.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.21.0 [✓] Connected device (5 available) • SM G975F (mobile) • RZ8M802WY0X • android-arm64 • Android 11 (API 30) • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios • iOS 14.4.1 18D61 • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729 • ios • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator) • macOS (desktop) • macos • darwin-x64 • Mac OS X 10.15.4 19E2269 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 98.0.4758.80 [✓] HTTP Host Availability • All required HTTP hosts are available ! Doctor found issues in 1 category. [!] Flutter (Channel master, 3.20.0-6.0.pre.49, on macOS 12.2.1 21D62 darwin-x64, locale en-GB) • Flutter version 3.20.0-6.0.pre.49 on channel master at /Users/dhs/documents/fluttersdk/flutter ! Warning: `flutter` on your path resolves to /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter. Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path. ! Warning: `dart` on your path resolves to /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter. Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision e93a10d1fb (62 minutes ago), 2024-02-13 09:27:15 +0530 • Engine revision 1c3ecee773 • Dart version 3.4.0 (build 3.4.0-132.0.dev) • DevTools version 2.33.0-dev.6 • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades. [!] Android toolchain - develop for Android devices (Android SDK version 30.0.3) • Android SDK at /Users/dhs/Library/Android/sdk ✗ cmdline-tools component is missing Run `path/to/sdkmanager --install "cmdline-tools;latest"` See https://developer.android.com/studio/command-line for more details. ✗ Android license status unknown. Run `flutter doctor --android-licenses` to accept the SDK licenses. See https://flutter.dev/docs/get-started/install/macos#android-setup for more details. [✓] Xcode - develop for iOS and macOS (Xcode 13.2.1) • Xcode at /Applications/Xcode.app/Contents/Developer • Build 13C100 • CocoaPods version 1.11.2 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] IntelliJ IDEA Ultimate Edition (version 2021.3.2) • IntelliJ at /Applications/IntelliJ IDEA.app • Flutter plugin version 65.1.4 • Dart plugin version 213.7228 [✓] VS Code (version 1.62.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.29.0 [✓] Connected device (3 available) • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios • iOS 15.3.1 19D52 • macOS (desktop) • macos • darwin-x64 • macOS 12.2.1 21D62 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 109.0.5414.119 [✓] Network resources • All expected network resources are available. ! Doctor found issues in 1 category. [!] Xcode - develop for iOS and macOS (Xcode 12.3) • Xcode at /Applications/Xcode.app/Contents/Developer ! Flutter recommends a minimum Xcode version of 13. Download the latest version or update via the Mac App Store. • CocoaPods version 1.11.2 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] VS Code (version 1.62.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.21.0 [✓] Connected device (5 available) • SM G975F (mobile) • RZ8M802WY0X • android-arm64 • Android 11 (API 30) • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios • iOS 14.4.1 18D61 • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729 • ios • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator) • macOS (desktop) • macos • darwin-x64 • Mac OS X 10.15.4 19E2269 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 98.0.4758.80 [✓] HTTP Host Availability • All required HTTP hosts are available ! Doctor found issues in 1 category. ```
beta flutter doctor ``` [!] Flutter (Channel beta, 3.19.0-0.4.pre, on macOS 12.2.1 21D62 darwin-x64, locale en-GB) • Flutter version 3.19.0-0.4.pre on channel beta at /Users/dhs/documents/fluttersdk/flutter ! Warning: `flutter` on your path resolves to /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter. Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path. ! Warning: `dart` on your path resolves to /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter. Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision b7e7d46a04 (6 days ago), 2024-02-02 08:21:06 -0600 • Engine revision 98820f0a77 • Dart version 3.3.0 (build 3.3.0-279.3.beta) • DevTools version 2.31.0 • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades. [✓] Xcode - develop for iOS and macOS (Xcode 13.2.1) • Xcode at /Applications/Xcode.app/Contents/Developer • Build 13C100 • CocoaPods version 1.11.2 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] IntelliJ IDEA Ultimate Edition (version 2021.3.2) • IntelliJ at /Applications/IntelliJ IDEA.app • Flutter plugin version 65.1.4 • Dart plugin version 213.7228 [✓] VS Code (version 1.62.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.29.0 [✓] Connected device (3 available) • iPhone 13 (mobile) • 3BD8F2FF-FDB5-4074-8C91-54F99C79156D • ios • com.apple.CoreSimulator.SimRuntime.iOS-15-2 (simulator) • macOS (desktop) • macos • darwin-x64 • macOS 12.2.1 21D62 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 105.0.5195.125 [✓] HTTP Host Availability • All required HTTP hosts are available • No issues found! ```
darshankawar commented 3 months ago

/cc @justinmc @gspencergoog

gspencergoog commented 3 months ago

I can also reproduce this, I'll try and bisect it to the offending change.

In the meantime, there is a workaround: use Shortcuts instead, which doesn't seem to have this problem.

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: const Home(),
      theme: ThemeData.light(useMaterial3: true),
    );
  }
}

class Home extends StatefulWidget {
  const Home({super.key});

  @override
  State<Home> createState() => _HomeState();
}

class _HomeState extends State<Home> {
  String val = 'Hello World';

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('TextField breaks')),
      body: Column(
        children: <Widget>[
          Text('Value: $val'),
          TextButton(
            onPressed: () => showDialog<String>(
              context: context,
              builder: (BuildContext context) => ADialog(
                value: val,
              ),
            ).then((String? v) {
              if (mounted && v != null && v != val) {
                setState(() => val = v);
              }
            }),
            child: const Text('Change'),
          ),
          const TextField(),
        ],
      ),
    );
  }
}

class ADialog extends StatefulWidget {
  const ADialog({super.key, required this.value});

  final String value;

  @override
  State<ADialog> createState() => ADialogState();
}

class ADialogState extends State<ADialog> {
  final FocusNode focusNode = FocusNode();
  late final TextEditingController controller = TextEditingController(text: widget.value);

  @override
  void dispose() {
    focusNode.dispose();
    controller.dispose();
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return AlertDialog(
      title: const Text('Edit text'),
      content: Shortcuts(
        shortcuts: <ShortcutActivator, Intent>{
          const SingleActivator(LogicalKeyboardKey.escape): VoidCallbackIntent(() {
            Navigator.of(context).pop();
          }),
        },
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            TextField(
              controller: controller,
              autofocus: true,
            ),
            TextButton(
              onPressed: () => Navigator.of(context).pop(controller.text),
              child: const Text('Save'),
            )
          ],
        ),
      ),
    );
  }
}
bleroux commented 3 months ago

@gspencergoog I bisected to https://github.com/flutter/flutter/pull/134554. Not sure why I can't repro it on Linux. I did the bisect on macOS.

gspencergoog commented 3 months ago

Ahh, @bleroux I wish I'd seen your comment earlier. :-). I bisected to the same change, though.

@chunhtai Seems like your PR is the cause of this regression, at least on macOS. Can you take a look?

Xazin commented 3 months ago

I've found the issue, thanks a lot for bisecting, luckily I saw both messages before starting.

Issue specifically stems from changes in lib/src/widgets/routes.dart More specifically calling changedInternalState() in

  @override
  void didPopNext(Route<dynamic> nextRoute) {
    super.didPopNext(nextRoute);
    changedInternalState();
  }

Seems to stem from the call to setState in changedInternalState as a result of didPopNext

chunhtai commented 2 weeks ago

For some reason if the textfield in the dialog is focused and user presses esc, the FlutterTextInputPlugin receives cancelOperation selector and attempt to send to the framework but got stuck. which blocks the rest selector call to. I guess it probably has something to do with the esc was handled in the framework side or some weirdness when disposing textfield. Needs more investigation

chunhtai commented 2 weeks ago

Looks like the change somhow made the TextField in the dialog give up connection a bit faster, which should be fine and it will make FlutterTextInputPlugin to give up firstResponder. However, the FlutterTextInputPlugin some how still receive cancel operation after it gave up firstResponder. I think this is more of a corner case in FlutterTextInputPlugin that gets surfaced by my previous pr. I will send a patch to macos engine

github-actions[bot] commented 21 hours ago

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.