hasStrings support for eliminating clipboard notifications #87678
Conversation
flutter-dashboard
bot
added
the
framework
justinmc
added
waiting for PR to land (fixed)
|
Gold has detected about 3 new digest(s) on patchset 1. |
|
Gold has detected about 7 new digest(s) on patchset 2. |
…earing down method channel mock
flutter-dashboard
bot
added
f: cupertino
|
The PR that this was waiting on, flutter/engine#27749, has been merged, but it still needs to be rolled into the framework and then this PR needs to get a merge commit. Otherwise this is ready to go. I tested it on an Android 12 emulator and it worked, though I wasn't seeing clipboard notifications at all for some reason. |
|
@wytesk133 This is enabling the permanent fix for your workaround in #86791. |
| } | ||
|
|
||
| ClipboardData? data; | ||
| late final bool hasStrings; |
There was a problem hiding this comment.
nit: is it possible to remove the late keyboard? Maybe the analyzer is smart enough to figure that out?
There was a problem hiding this comment.
It seems to be ok without late. I had no idea that was valid Dart, thanks for the suggestion!
| triedToReadClipboard = true; | ||
| calledGetData = true; | ||
| } | ||
| if (methodCall.method == 'Clipboard.hasStrings') { |
There was a problem hiding this comment.
nit: use else if or change the entire body toswitch-case
| // The clipboard is not checked because it requires user permissions and | ||
| // web doesn't show a custom text selection menu. | ||
| expect(triedToReadClipboard, false); | ||
| // hasStrings is not checked because web doesn't show a custom text |
There was a problem hiding this comment.
Seems like the comment on line 6237 needs to be updated as well.
There was a problem hiding this comment.
Good catch, thanks.
|
|
||
| setUp(() async { | ||
| TestDefaultBinaryMessengerBinding.instance!.defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, mockClipboard.handleMethodCall); |
There was a problem hiding this comment.
nit: use dartfmt to make the lines fit in 80 chars
|
Hello. Since this PR is not in Stable yet, is there any workaround to avoid having this notification? Idk, maybe somehow disable Handoff programatically for a huge project that is already in production? Or something else? |
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](flutter/engine#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 (#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](#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 flutter/engine#43360. ### References Fixes #129692 Engine PR to be merged after this: flutter/engine#43360
Previously, the framework was looking at the actual contents of the clipboard in order to decide if it could paste or not. This caused apps to show a notification ("app pasted from app") on certain platforms (currently iOS 13+ and Android 12+).
After a bunch of engine work linked below, we now have engine methods that can check the clipboard status without accessing the contents of the clipboard on supported platforms (Android and iOS).
This PR wires these engine methods up in the framework. On platforms that don't have support for methods like these, hasStrings still accesses the clipboard contents in the engine.
Related issues and PRs
Currently waiting for flutter/engine#27749.
Closes #60145
Partial fix for #74139 (Android 12+)
Depends on iOS engine support: flutter/engine#19859
Depends on Android engine support: flutter/engine#20393
Depends on Android 12 engine support: flutter/engine#27215
Depends on Mac engine support: flutter/engine#20531
Depends on Linux engine support: flutter/engine#21388
Depends on Windows engine support: flutter/engine#27749
Reopened from #61774
Makes work arounds like #86791 obsolete.
Fuchsia
Flutter on Fuchsia currently has no clipboard support. When it gets clipboard support we should also include support for hasStrings, even if it just uses getData.