From f5ceaf9810847885c86b0a4e8bfa22c30c592093 Mon Sep 17 00:00:00 2001 From: Justin McCandless Date: Wed, 9 Aug 2023 13:36:45 -0700 Subject: [PATCH] Handle hasStrings on web (#132093) By default, Flutter web uses the browser's built-in context menu. As of [recently](https://github.com/flutter/engine/pull/38682), it's possible to use a Flutter-rendered context menu like the other platforms. ```dart void main() { runApp(const MyApp()); BrowserContextMenu.disableContextMenu(); } ``` But there is a bug (https://github.com/flutter/flutter/issues/129692) that the Paste button is missing and never shows up. Screenshot 2023-08-07 at 2 39 03 PM The reason why it's missing is that Flutter first checks if there is any pasteable text on the clipboard before deciding to show the Paste button using the `hasStrings` platform channel method, but that was never implemented for web ([original hasStrings PR](https://github.com/flutter/flutter/pull/87678)). So let's just implement hasStrings for web? No, because Chrome shows a permissions prompt when the clipboard is accessed, and there is [no browser clipboard API](https://developer.mozilla.org/en-US/docs/Web/API/Clipboard_API) to avoid it. The prompt will show immediately when the EditableText is built, not just when the Paste button is pressed. ### This PR's solution Instead, before implementing hasStrings for web, this PR disables the hasStrings check for web. The result is that users will always see a paste button, even in the (unlikely) case that they have nothing pasteable on the clipboard. However, they will not see a permissions dialog until they actually click the Paste button. Subsequent pastes don't show the permission dialog.
Video of final behavior with this PR https://github.com/flutter/flutter/assets/389558/ed16c925-8111-44a7-99e8-35a09d682748
I think this will be the desired behavior for the vast majority of app developers. Those that want different behavior can use hasStrings themselves, which will be implemented in https://github.com/flutter/engine/pull/43360. ### References Fixes https://github.com/flutter/flutter/issues/129692 Engine PR to be merged after this: https://github.com/flutter/engine/pull/43360 --- .../lib/src/widgets/editable_text.dart | 23 ++++++- .../test/material/text_field_test.dart | 4 +- .../test/widgets/editable_text_test.dart | 63 +++++++++++++++++-- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index f371290e75..9f857021fd 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -2102,7 +2102,13 @@ class EditableTextState extends State with AutomaticKeepAliveClien final GlobalKey _editableKey = GlobalKey(); /// Detects whether the clipboard can paste. - final ClipboardStatusNotifier clipboardStatus = ClipboardStatusNotifier(); + final ClipboardStatusNotifier clipboardStatus = kIsWeb + // Web browsers will show a permission dialog when Clipboard.hasStrings is + // called. In an EditableText, this will happen before the paste button is + // clicked, often before the context menu is even shown. To avoid this + // poor user experience, always show the paste button on web. + ? _WebClipboardStatusNotifier() + : ClipboardStatusNotifier(); /// Detects whether the Live Text input is enabled. /// @@ -5561,3 +5567,18 @@ class _GlyphHeights { /// The glyph height of the last line. final double end; } + +/// A [ClipboardStatusNotifier] whose [value] is hardcoded to +/// [ClipboardStatus.pasteable]. +/// +/// Useful to avoid showing a permission dialog on web, which happens when +/// [Clipboard.hasStrings] is called. +class _WebClipboardStatusNotifier extends ClipboardStatusNotifier { + @override + ClipboardStatus value = ClipboardStatus.pasteable; + + @override + Future update() { + return Future.value(); + } +} diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 966ff68b0d..3697dbc37a 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -14154,7 +14154,9 @@ void main() { expect(calledGetData, false); // hasStrings is checked in order to decide if the content can be pasted. expect(calledHasStrings, true); - }); + }, + skip: kIsWeb, // [intended] web doesn't call hasStrings. + ); testWidgets('TextField changes mouse cursor when hovered', (WidgetTester tester) async { await tester.pumpWidget( diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 6d008132e8..461085d7f2 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -63,11 +63,10 @@ TextEditingValue collapsedAtEnd(String text) { } void main() { - final MockClipboard mockClipboard = MockClipboard(); - TestWidgetsFlutterBinding.ensureInitialized() - .defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, mockClipboard.handleMethodCall); - setUp(() async { + final MockClipboard mockClipboard = MockClipboard(); + TestWidgetsFlutterBinding.ensureInitialized() + .defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, mockClipboard.handleMethodCall); debugResetSemanticsIdCounter(); controller = TextEditingController(); // Fill the clipboard so that the Paste option is available in the text @@ -76,6 +75,8 @@ void main() { }); tearDown(() { + TestWidgetsFlutterBinding.ensureInitialized() + .defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, null); controller.dispose(); }); @@ -2145,7 +2146,7 @@ void main() { ); final EditableTextState state = - tester.state(find.byType(EditableText)); + tester.state(find.byType(EditableText)); // Show the toolbar. state.renderEditable.selectWordsInRange( @@ -2156,9 +2157,11 @@ void main() { final TextSelection copySelectionRange = localController.selection; + expect(find.byType(TextSelectionToolbar), findsNothing); state.showToolbar(); await tester.pumpAndSettle(); + expect(find.byType(TextSelectionToolbar), findsOneWidget); expect(find.text('Copy'), findsOneWidget); await tester.tap(find.text('Copy')); @@ -16603,6 +16606,56 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async }, skip: kIsWeb, // [intended] ); + + group('hasStrings', () { + late int calls; + setUp(() { + calls = 0; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(SystemChannels.platform, (MethodCall methodCall) { + if (methodCall.method == 'Clipboard.hasStrings') { + calls += 1; + } + return Future.value(); + }); + }); + tearDown(() { + TestWidgetsFlutterBinding.ensureInitialized() + .defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, null); + }); + + testWidgets('web avoids the paste permissions prompt by not calling hasStrings', (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + home: EditableText( + backgroundCursorColor: Colors.grey, + controller: TextEditingController(), + focusNode: focusNode, + obscureText: true, + toolbarOptions: const ToolbarOptions( + copy: true, + cut: true, + paste: true, + selectAll: true, + ), + style: textStyle, + cursorColor: cursorColor, + selectionControls: materialTextSelectionControls, + ), + ), + ); + + expect(calls, equals(kIsWeb ? 0 : 1)); + + // Long-press to bring up the context menu. + final Finder textFinder = find.byType(EditableText); + await tester.longPress(textFinder); + tester.state(textFinder).showToolbar(); + await tester.pumpAndSettle(); + + expect(calls, equals(kIsWeb ? 0 : 2)); + }); + }); } class UnsettableController extends TextEditingController {