Unverified Commit f5ceaf98 authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

Handle hasStrings on web (#132093)

By default, Flutter web uses the browser's built-in context menu.

<img width="200" src="https://github.com/flutter/flutter/assets/389558/990f99cb-bc38-40f1-9e88-8839bc342da5" />

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.

<img width="284" alt="Screenshot 2023-08-07 at 2 39 03 PM" src="https://github.com/flutter/flutter/assets/389558/f632be25-28b1-4e2e-98f7-3bb443f077df">

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.

<img width="200" src="https://github.com/flutter/flutter/assets/389558/5abdb160-1b13-4f1a-87e1-4653ca19d73e" />

### 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.

<details>

<summary>Video of final behavior with this PR</summary>

https://github.com/flutter/flutter/assets/389558/ed16c925-8111-44a7-99e8-35a09d682748

</details>

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
parent 6ac161f9
......@@ -2102,7 +2102,13 @@ class EditableTextState extends State<EditableText> 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<void> update() {
return Future<void>.value();
}
}
......@@ -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(
......
......@@ -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<EditableTextState>(find.byType(EditableText));
tester.state<EditableTextState>(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<void>.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<EditableTextState>(textFinder).showToolbar();
await tester.pumpAndSettle();
expect(calls, equals(kIsWeb ? 0 : 2));
});
});
}
class UnsettableController extends TextEditingController {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment