1776829 - Considering clipboard access permission API for clipboard.readText()

Status: RESOLVED FIXED

(GeckoView :: Permissions, enhancement, P1)

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Gave it a try with current Nightly on my Android phone: https://jsfiddle.net/68hng59s/.

No "Paste" button appeared, the jsfiddle's console didn't show any error message.

Comment 2

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

No "Paste" button appearing is expected, because Android doesn't know how to deal with <menupopup>.

Comment 3

[Makoto Kato [:m_kato]]

XUL popup won't work on GeckoView. I think that GeckoView has to add anything permission API in GeckoSession.PermissionDelegate. PERMISSION_CLIPBOARD_READTEXT_PASTE?

Comment 4

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Makoto Kato [:m_kato] from comment #3)

XUL popup won't work on GeckoView. I think that GeckoView has to add anything permission API in GeckoSession.PermissionDelegate. PERMISSION_CLIPBOARD_READTEXT_PASTE?

Thanks for the response. AFAIU, GeckoView (GV) is a layer between Gecko and Fenix. Would additional changes in GV and Fenix be required to support this?

GeckoSession.PermissionDelegate might not be what's desired here, given that on Desktop, deliberately a "Paste" button instead of a permission dialog (e.g. like for GeoLocation requests) is shown. @csadilek: are there reasons to do this differently on Android?

Any context here is appreciated, I know little above GV and Fenix.

Comment 5

[Makoto Kato [:m_kato]]

(In reply to Mirko Brodesser (:mbrodesser) from comment #4)

(In reply to Makoto Kato [:m_kato] from comment #3)

XUL popup won't work on GeckoView. I think that GeckoView has to add anything permission API in GeckoSession.PermissionDelegate. PERMISSION_CLIPBOARD_READTEXT_PASTE?

Thanks for the response. AFAIU, GeckoView (GV) is a layer between Gecko and Fenix. Would additional changes in GV and Fenix be required to support this?

Yes, If GeckoView need additional UX such as permission and popup/prompt, we will delegate this action to Fenix/Android-Component (a-c). Mostly, it is implemented by a-c.

GeckoSession.PermissionDelegate might not be what's desired here, given that on Desktop, deliberately a "Paste" button instead of a permission dialog (e.g. like for GeoLocation requests) is shown. @csadilek: are there reasons to do this differently on Android?

Any context here is appreciated, I know little above GV and Fenix.

Why doesn't we use nsIContentPermissionPrompt like other permissions? Doesn't UX team want doorhanger notification for this permission?

Comment 6

[Makoto Kato [:m_kato]]

Reference for GV UX issue is https://firefox-source-docs.mozilla.org/mobile/android/geckoview/contributor/geckoview-architecture.html#delegates. Also, permission is https://firefox-source-docs.mozilla.org/mobile/android/geckoview/contributor/geckoview-architecture.html#permissions

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Makoto Kato [:m_kato] from comment #5)

(In reply to Mirko Brodesser (:mbrodesser) from comment #4)

(In reply to Makoto Kato [:m_kato] from comment #3)

XUL popup won't work on GeckoView. I think that GeckoView has to add anything permission API in GeckoSession.PermissionDelegate. PERMISSION_CLIPBOARD_READTEXT_PASTE?

Thanks for the response. AFAIU, GeckoView (GV) is a layer between Gecko and Fenix. Would additional changes in GV and Fenix be required to support this?

Yes, If GeckoView need additional UX such as permission and popup/prompt, we will delegate this action to Fenix/Android-Component (a-c). Mostly, it is implemented by a-c.

GeckoSession.PermissionDelegate might not be what's desired here, given that on Desktop, deliberately a "Paste" button instead of a permission dialog (e.g. like for GeoLocation requests) is shown. @csadilek: are there reasons to do this differently on Android?

Any context here is appreciated, I know little above GV and Fenix.

Why doesn't we use nsIContentPermissionPrompt like other permissions? Doesn't UX team want doorhanger notification for this permission?

We haven't talked to the UX team about this yet. However, with multiple people, we came to the conclusion that a "Paste" button should be shown to the user. Some more context about this can be found in https://bugzilla.mozilla.org/show_bug.cgi?id=1578321#c8.

Comment 8

[Christian Sadilek [:csadilek]]

@csadilek: are there reasons to do this differently on Android?

Just to add to what :m_kato already shared, we want any popups and prompts to be rendered natively on Android so we have consistent platform-specific look-and-feel. The good news is that we already have a solution for this in GeckoView, since the desired UX here is a "Paste" button that should appear relative to its trigger.

You can see the details in https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/mobile/android/actors/SelectionActionDelegateChild.jsm#68 and how it handles org.mozilla.geckoview.PASTE for example. It will send a ShowSelectionAction message containing the clientRect, which GeckoView will use to render the action:

• https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/mobile/android/actors/SelectionActionDelegateChild.jsm#319-358
• https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java#324

Rendering a single paste action looks very similar to your solution on desktop. After talking to Agi, this wouldn't need any A-C/Fenix/Focus changes. You should be able to translate and handle actions in GeckoView's SelectionActionDelegateChild as a starting point.

Comment 9

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Christian Sadilek [:csadilek] from comment #8)

@csadilek: are there reasons to do this differently on Android?

Thanks for the bigger picture, that's helpful.

Just to add to what :m_kato already shared, we want any popups and prompts to be rendered natively on Android so we have consistent platform-specific look-and-feel.

Makes sense.

The good news is that we already have a solution for this in GeckoView, since the desired UX here is a "Paste" button that should appear relative to its trigger.

"relative to its trigger" isn't entirely true. On Desktop, it's always next to the mouse position. That's usually relative to the trigger, but not necessarily. A call of navigator.clipboard.readText() may happen a few seconds (at most five, see https://searchfox.org/mozilla-central/rev/4654ce1e24a6af17bc96ab60f1f70c218755142f/modules/libpref/init/StaticPrefList.yaml#3879-3880) after a user action.

Moreover, readText() may be called from different frames (e.g. from an <iframe> and its parent) and only one "Paste" button should be shown for them. I guess it should be possible handle that in GeckoView.

You can see the details in https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/mobile/android/actors/SelectionActionDelegateChild.jsm#68 and how it handles org.mozilla.geckoview.PASTE for example. It will send a ShowSelectionAction message containing the clientRect, which GeckoView will use to render the action:

• https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/mobile/android/actors/SelectionActionDelegateChild.jsm#319-358
• https://searchfox.org/mozilla-central/rev/2946e9b450cb35afaf8dad927a8d187975dcd74d/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java#324

Rendering a single paste action looks very similar to your solution on desktop. After talking to Agi, this wouldn't need any A-C/Fenix/Focus changes. You should be able to translate and handle actions in GeckoView's SelectionActionDelegateChild as a starting point.

Thanks for the starting point; will need to learn how this can be accessed from the C++ side of Gecko.

Comment 10

[Makoto Kato [:m_kato]]

Moving to GV

Comment 11

[Makoto Kato [:m_kato]]

WIP: https://treeherder.mozilla.org/jobs?repo=try&revision=6ad579cb483193fbde2e43342eb912e11fb96484

Comment 12

[Makoto Kato [:m_kato]]

Attachment: Bug 1776829 - Implement "Paste" permission action for clipboard.readText. r=#geckoview-reviewers

clipboard.readText requires clipboard read permission. But this isn't
implemented by nsIContentPermission, so we have to handle
ClipboardReadTextPaste actor, then call
PermissionDelegate.onContentPermission with PERMISSION_CLIPBOARD_READ for
this support.

Also, since it is possible to implement iOS style clipboard read permission
prompt, I add last touch/mouse location to the parameter as clientPoint.

Comment 13

[Chris Peterson [:cpeterson]]

P1 since Makoto is working on this bug.

Comment 14

[Chris Peterson [:cpeterson]]

Rolling this bug over to 106

Comment 15

[Chris Peterson [:cpeterson]]

Makoto, you have some code changes planned based on review feedback. Will you have time to make the changes and land them in the Nightly 106 cycle?

Comment 16

[Makoto Kato [:m_kato]]

Makoto, you have some code changes planned based on review feedback. Will you have time to make the changes and land them in the Nightly 106 cycle?

I hope this will be fixed in 106 cycle. But dom.events.asyncClipboard.readText is still turned off even if desktop.

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Makoto Kato [:m_kato] from comment #16)

Makoto, you have some code changes planned based on review feedback. Will you have time to make the changes and land them in the Nightly 106 cycle?

I hope this will be fixed in 106 cycle. But dom.events.asyncClipboard.readText is still turned off even if desktop.

Yup, we're still addressing some issues blocking turning dom.events.asyncClipboard.readText on on desktop. We plan to turn this feature on on both desktop and android the same time

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc01b1431aa4
Implement "Paste" permission action for clipboard.readText. r=geckoview-reviewers,owlish

Comment 19

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/bc01b1431aa4