1367079 - [geckoview] Figure out text selection actions

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P1)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

You can select text right now, but we don't show any type of menu or expose any interface to the application for handling that.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Dylan has been looking at this. The plan is to just handle all of the text selection stuff internally for now. GeckoView will take care of popping up the clipboard action UI.

Comment 2

[Nicole Yee (:nicoleyee)]

Sorina, can you please test this and see if it is reproducible on different devices?

Comment 3

[Nicole Yee (:nicoleyee)]

wrong bug...

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1367079 - 1. Update TextSelection implementations for GeckoView;

Make TextSelection implementations not depend on GeckoApp. Instead, make
them use GeckoView's EventDispatcher directly for communicating with
Gecko.

Review commit: https://reviewboard.mozilla.org/r/179334/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179334/

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1367079 - 2. Move Share:Text listener to GeckoApplication;

Move the "Share:Text" event listener from GeckoApp to GeckoApplication,
so ActionBarHandler.js can use it for the Share action from any window.

Review commit: https://reviewboard.mozilla.org/r/179336/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179336/

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1367079 - 3. Add utility for registering lazy WindowEventDispatcher listener;

Add `GeckoViewUtils.registerLazyWindowEventListener` that, similar to
`addLazyEventListener`, will register a lazy event listener with the
per-window EventDispatcher.

Review commit: https://reviewboard.mozilla.org/r/179338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179338/

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1367079 - 4. Move ActionBarHandler usage out of browser.js;

Use ActionBarHandler in BrowserCLH.js instead of browser.js, so it can
handle text selection for all windows. Also update ActionBarHandler to
reflect the new usage and to support multiple windows.

Review commit: https://reviewboard.mozilla.org/r/179340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179340/

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1367079 - 5. Add TextSelection support in custom tabs and PWA;

Add TextSelection support in custom tabs and PWA.

Review commit: https://reviewboard.mozilla.org/r/179342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179342/

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8907672 [details]
Bug 1367079 - 1. Update TextSelection implementations for GeckoView;

https://reviewboard.mozilla.org/r/179334/#review184556

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8907673 [details]
Bug 1367079 - 2. Move Share:Text listener to GeckoApplication;

https://reviewboard.mozilla.org/r/179336/#review184558

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8907674 [details]
Bug 1367079 - 3. Add utility for registering lazy WindowEventDispatcher listener;

https://reviewboard.mozilla.org/r/179338/#review184560

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8907675 [details]
Bug 1367079 - 4. Move ActionBarHandler usage out of browser.js;

https://reviewboard.mozilla.org/r/179340/#review184564

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8907676 [details]
Bug 1367079 - 5. Add TextSelection support in custom tabs and PWA;

https://reviewboard.mozilla.org/r/179342/#review184566

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58e575edf398
1. Update TextSelection implementations for GeckoView; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/1af8aae9f137
2. Move Share:Text listener to GeckoApplication; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f1d3a461631
3. Add utility for registering lazy WindowEventDispatcher listener; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/c185dd417782
4. Move ActionBarHandler usage out of browser.js; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/5dae173f8fe8
5. Add TextSelection support in custom tabs and PWA; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/69521f759e29
6. Fix testAccessibleCarets; r=me

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/58e575edf398
https://hg.mozilla.org/mozilla-central/rev/1af8aae9f137
https://hg.mozilla.org/mozilla-central/rev/7f1d3a461631
https://hg.mozilla.org/mozilla-central/rev/c185dd417782
https://hg.mozilla.org/mozilla-central/rev/5dae173f8fe8
https://hg.mozilla.org/mozilla-central/rev/69521f759e29

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1367079 - 6. Fix testAccessibleCarets;

Fix testAccessibleCarets and enable it for non-Nightly builds. r=me for
test changes.

Review commit: https://reviewboard.mozilla.org/r/180370/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180370/

Comment 17

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8907672 [details]
Bug 1367079 - 1. Update TextSelection implementations for GeckoView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179334/diff/1-2/

Comment 18

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8907673 [details]
Bug 1367079 - 2. Move Share:Text listener to GeckoApplication;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179336/diff/1-2/

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8907674 [details]
Bug 1367079 - 3. Add utility for registering lazy WindowEventDispatcher listener;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179338/diff/1-2/

Comment 20

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8907675 [details]
Bug 1367079 - 4. Move ActionBarHandler usage out of browser.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179340/diff/1-2/

Comment 21

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8907676 [details]
Bug 1367079 - 5. Add TextSelection support in custom tabs and PWA;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179342/diff/1-2/

Comment 22

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8907672 [details]
Bug 1367079 - 1. Update TextSelection implementations for GeckoView;

Beta request for all patches.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Custom tabs feature slated for 57 would not have the text selection toolbar feature.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Bug 1367077, bug 1398409
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: Well-tested locally and the patches only just missed the Beta merge by a few hours.
[String changes made/needed]: None

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8907672 [details]
Bug 1367079 - 1. Update TextSelection implementations for GeckoView;

This shouldn't need an uplift request to get into 57, since we are going to sync m-c to m-b again.