Skip to content

Conversation

@justinmc
Copy link
Contributor

Description

#57139 checked the clipboard to see if it could show the paste button, but this broke web because it has to ask for the user's permission to use the clipboard (see #57139 (comment)). Web doesn't draw its own text selection menu though, so it doesn't need to check the clipboard, and this PR removes that functionality for web.

Related Issues

#57286

Tests

I added the following tests:

  • Check that web doesn't ask for clipboard contents when selecting text, but other environments do.

@justinmc justinmc requested review from goderbauer and nturgut May 15, 2020 16:24
@justinmc justinmc self-assigned this May 15, 2020
@fluttergithubbot fluttergithubbot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels May 15, 2020
@justinmc
Copy link
Contributor Author

@nturgut Is it possible to run those failing web tests locally to make sure that this fixes it?

@nturgut
Copy link
Contributor

nturgut commented May 15, 2020

We also have this control for the toolbar:

Is it related?

For a text field on the web, we don't render a copy/paste menu, we use the one that browser shows.

@justinmc
Copy link
Contributor Author

Yeah, these changes in EditableText are still needed in addition to that check because EditableText uses the clipboard status to decide to allow pasting via semantics/accessibility.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@justinmc justinmc force-pushed the pasteable-revert-revert-4 branch from 7ff5606 to 6a4c68a Compare May 15, 2020 21:07
@justinmc
Copy link
Contributor Author

I just rebased with master and reverted #57286 as part of this PR as well. I tried running a web app before and after 6a4c68a and confirmed that I saw the permissions notification only before that commit.

@nturgut
Copy link
Contributor

nturgut commented May 15, 2020

One option (for local development) is using felt tool.

You can run:

felt test --integration-tests-only --use-system-flutter

@nturgut
Copy link
Contributor

nturgut commented May 19, 2020

Flutter Web integration tests are passing, I run them on my local against this PR.

@justinmc
Copy link
Contributor Author

Confirmed that those tests pass locally now ✅

@fluttergithubbot fluttergithubbot merged commit 53d7f24 into flutter:master May 20, 2020
@justinmc justinmc deleted the pasteable-revert-revert-4 branch May 20, 2020 15:02
@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

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants