Skip to content

Conversation

@nturgut
Copy link
Contributor

@nturgut nturgut commented May 15, 2020

Reverts #57139

This causes Flutter for Web to fail for all pages which have an Editable Text.

We caught it in our integration tests: https://ci.chromium.org/p/flutter/builders/try/Linux%20Web%20Engine/4099?

These tests only run on the engine side so we were able to catch it only now.

Overall getData means paste, reading the clipboard on web, which requires permission and should only be done with user permission. Calling getData will bring a browser popup window and will ask user for permission. It is not the best practice to ask user for their clipboard's access without an explicit event that might trigger it. Let's discuss a solution that will also work for web before remerging this PR.

@nturgut nturgut requested review from goderbauer and justinmc May 15, 2020 06:13
@fluttergithubbot fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels May 15, 2020
@nturgut nturgut added platform-web Web applications specifically ⚠ TODAY labels May 15, 2020
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

That makes sense on web. This PR is used to hide/show the paste button in the text selection menu, which isn't necessary on the web because the web uses the native text selection menu.

I think if I just always set the clipboard status to pasteable on web without checking the clipboard, then this should work on web and shouldn't ever ask the user for permission.

@justinmc
Copy link
Contributor

@nturgut I've got a fix for this in #57324.

@nturgut
Copy link
Contributor Author

nturgut commented May 15, 2020

@nturgut I've got a fix for this in #57324.

Thanks a lot for the quick fix!

@nturgut
Copy link
Contributor Author

nturgut commented May 15, 2020

Merging the PR since it's related to broken tests, on the engine side.

@nturgut nturgut merged commit 32547dc into master May 15, 2020
@nturgut nturgut deleted the revert-57139-pasteable-revert-revert-3 branch May 15, 2020 17:42
justinmc added a commit to justinmc/flutter that referenced this pull request May 15, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. platform-web Web applications specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants