Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Try to use getPrimaryClipDescription on Android 12 #22271

Closed
Amejia481 opened this issue Nov 2, 2021 · 11 comments
Closed

Try to use getPrimaryClipDescription on Android 12 #22271

Amejia481 opened this issue Nov 2, 2021 · 11 comments
Assignees
Labels
Android:12 Issues specific to Android 12 - API 31 eng:qa:verified QA Verified Feature:Toolbar Address bar, see also Feature:Search Hacktoberfest Highlights issues for Hacktoberfest contributors to help us! help wanted Help wanted from a contributor. More complex than good first issue.
Milestone

Comments

@Amejia481
Copy link
Contributor

Amejia481 commented Nov 2, 2021

On Android 12 there is a new behavior change when accessing the clipboard, the OS shows a toast indicating that the app is is accessing it, more details can be found here.

When users try to search or open an URL in the toolbar Fenix tries to check on the clipboard to see if there is an URL on the clipboard that the user wants to navigates to, we use these extension functions to try to figure out, but they are triggering the Clipboard access notifications on Android 12, to avoid that we want to use getPrimaryClipDescription()

Steps to show the reproduce the Clipboard access notifications on Android 12:

  1. Copy a URL to clipboard in some other app (e.g. https://example.org )
  2. Start Firefox Nightly

┆Issue is synchronized with this Jira Task

@Amejia481 Amejia481 added help wanted Help wanted from a contributor. More complex than good first issue. Hacktoberfest Highlights issues for Hacktoberfest contributors to help us! Android:12 Issues specific to Android 12 - API 31 Feature:Toolbar Address bar, see also Feature:Search labels Nov 2, 2021
@github-actions github-actions bot added the needs:triage Issue needs triage label Nov 2, 2021
@dholbert
Copy link
Contributor

dholbert commented Nov 2, 2021

One particularly-important use case to consider here, from a user-confusion/suspicion perspective: under some configurations[1], Firefox apparently never shows the "Fill link from clipboard" UI, but nonetheless we seem to check the clipboard as if we were going to show that UI. This ends up meaning that the user still sees a "Firefox Nightly pasted from your clipboard" system notification when there's no clear user-visible indication about why/where Firefox pasted the clipboard contents (or what Firefox found there). This ends up feeling unsettling/concerning from a user perspective.

As part of fixing this bug, perhaps we should bypass the clipboard access entirely for this configuration (assuming that we're hiding the UI intentionally).

[1] In particular: on my phone, "Settings | Search | Show clipboard suggestions: [disabled]" setting causes the "fill link from clipboard" UI to never appear. (but Firefox still queries my clipboard for apparently no reason). (The "Show search engines: [enabled]" setting is another way to hide the fill-link-from-clipboard UI, but I think that's a bug; I filed #22272 on that.)

@nathan-castlehow
Copy link

nathan-castlehow commented Nov 7, 2021

Hi,
I'm happy to give this a stab if that will help at all. New to this project so forgive me If I've missed any steps here.

I had an initial look into this.
The extension functions listed are already using getPrimaryDescription before then reading the clipboard.

private fun ClipboardManager.isPrimaryClipPlainText() = primaryClipDescription?.hasMimeType(MIME_TYPE_TEXT_PLAIN) ?: false

private fun ClipboardManager.isPrimaryClipHtmlText() = primaryClipDescription?.hasMimeType(MIME_TYPE_TEXT_HTML) ?: false

Api 31 has introduced getConfidenceScore on the clipboard description class which we can use to check if an item is a URL or not.

We could use this before deciding to then read the clipboard
Something similar (just threw this together, needs api checks etc) to
private fun ClipboardManager.isPrimaryClipURI() = 0.5f < (primaryClipDescription?.getConfidenceScore(TextClassifier.TYPE_URL) ?: 0f)

If the confidence score is unavailable (it may not be calculated at the time this runs) what behaviour do we fall back to? (I presume the existing behaviour?
Ultimately though when we do eventually decide to read the clipboard it looks like the notification toast will always appear.

I've noticed on Android12 that the default keyboard already has the option to paste the url into the search bar (see image below). I wonder if there is a separate conversation about deprecating the feature for 12 and up as the clipboard read toast is pretty scary.

image

@Amejia481
Copy link
Contributor Author

Thanks @dholbert, I'll take a look to confirm this #22271 (comment).

@Amejia481
Copy link
Contributor Author

I've noticed on Android12 that the default keyboard already has the option to paste the url into the search bar (see image below). I wonder if there is a separate conversation about deprecating the feature for 12 and up as the clipboard read toast is pretty scary.

@nathan-castlehow thanks for bring this out, yeah I noticed Chrome resigned the functionality to only copy from the clipboard upon user's request. I think it will be better for me to clarify with the UI/UX team, before doing the changes I'm not sure how we should proceed. Let's not do any changes until confirming, thanks for interesting on this bug.

image

@Amejia481
Copy link
Contributor Author

One particularly-important use case to consider here, from a user-confusion/suspicion perspective: under some configurations[1], Firefox apparently never shows the "Fill link from clipboard" UI, but nonetheless we seem to check the clipboard as if we were going to show that UI. This ends up meaning that the user still sees a "Firefox Nightly pasted from your clipboard" system notification when there's no clear user-visible indication about why/where Firefox pasted the clipboard contents (or what Firefox found there). This ends up feeling unsettling/concerning from a user perspective.

As part of fixing this bug, perhaps we should bypass the clipboard access entirely for this configuration (assuming that we're hiding the UI intentionally).

[1] In particular: on my phone, "Settings | Search | Show clipboard suggestions: [disabled]" setting causes the "fill link from clipboard" UI to never appear. (but Firefox still queries my clipboard for apparently no reason). (The "Show search engines: [enabled]" setting is another way to hide the fill-link-from-clipboard UI, but I think that's a bug; I filed #22272 on that.)

It looks like we are still accessing the API even the setting is off, we are only not showing the result, for this reason the notification is still showing.

@amedyne amedyne removed the needs:triage Issue needs triage label Nov 8, 2021
@Amejia481 Amejia481 self-assigned this Nov 8, 2021
@Amejia481 Amejia481 added this to Ready for Engineering (min-5 ; max-22) in Android Engineering Team Kanban board via automation Nov 8, 2021
@Amejia481 Amejia481 moved this from Ready for Engineering (min-5 ; max-22) to In Development (WIP limit - 15) in Android Engineering Team Kanban board Nov 8, 2021
Amejia481 added a commit to Amejia481/fenix that referenced this issue Nov 9, 2021
Amejia481 added a commit to Amejia481/fenix that referenced this issue Nov 10, 2021
Amejia481 added a commit to Amejia481/fenix that referenced this issue Nov 10, 2021
@Amejia481
Copy link
Contributor Author

@nathan-castlehow as this became a pressing issue I had to address it myself sorry for that, hope you could understand.
I put a PR where you can see the approach that we took. Please take a look your input it's really helpful and really appreciate ❤️.

@Amejia481
Copy link
Contributor Author

Amejia481 commented Nov 15, 2021

This landed on main, the fix should be available in the next update nightly.

QA team please help use to verify that issue is fixed.

@Amejia481 Amejia481 moved this from In Development (WIP limit - 15) to In Testing/QA in Android Engineering Team Kanban board Nov 15, 2021
@Amejia481 Amejia481 added this to the 96 milestone Nov 15, 2021
@Amejia481 Amejia481 added the eng:qa:needed QA Needed label Nov 15, 2021
@Amejia481
Copy link
Contributor Author

@dholbert are you able to reproduce in the latest nightly?

@sflorean
Copy link
Contributor

sflorean commented Nov 17, 2021

I wasn't able to reproduce the issue on latest Nightly (11/17) with Pixel 3 (Android 12) or Pixel 4XL (Android 12)

@sflorean sflorean removed the eng:qa:needed QA Needed label Nov 17, 2021
@sflorean sflorean added the eng:qa:verified QA Verified label Nov 17, 2021
Android Engineering Team Kanban board automation moved this from In Testing/QA to Done Nov 17, 2021
@dholbert
Copy link
Contributor

@dholbert are you able to reproduce in the latest nightly?

No, I can't. This does indeed seem to be fixed. Thanks!

mergify bot pushed a commit that referenced this issue Nov 18, 2021
csadilek pushed a commit that referenced this issue Nov 19, 2021
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Mar 7, 2022
@dholbert
Copy link
Contributor

FWIW, I've just filed a bug for a different set of STR that trigger this same "Firefox Nightly pasted from your clipboard" system-notification, in a way that annoyingly prevents me from pasting for a few seconds (because the notification covers up our "Paste" menu item).

See details over in #26564.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android:12 Issues specific to Android 12 - API 31 eng:qa:verified QA Verified Feature:Toolbar Address bar, see also Feature:Search Hacktoberfest Highlights issues for Hacktoberfest contributors to help us! help wanted Help wanted from a contributor. More complex than good first issue.
Projects
No open projects
Development

No branches or pull requests

5 participants