1879701 - Show text selection context menu when mouse right click on the selected text

Status: VERIFIED FIXED

(GeckoView :: General, enhancement, P1)

Description

[jackyzy823]

STR

1. Select text range using mouse
2. Right click the mouse on the selected text area

Expected:

The text selection toolbar shows (just like Chrome)

Actual:

Nothing happens.

Comment 1

[jackyzy823]

For anyone who may concerned:

Setting layout.accessiblecaret.hide_carets_for_mouse_input to false allows geckoview automatically showing text selection menu when selecting text with mouse.

---

Ref: https://searchfox.org/mozilla-central/rev/00e348d6935d99a089c318f4512b22327c22154b/layout/base/AccessibleCaretManager.cpp#158
https://searchfox.org/mozilla-central/rev/00e348d6935d99a089c318f4512b22327c22154b/mobile/android/actors/SelectionActionDelegateChild.sys.mjs#288
https://searchfox.org/mozilla-central/rev/00e348d6935d99a089c318f4512b22327c22154b/mobile/android/chrome/geckoview/geckoview.js#673

Comment 2

[jackyzy823]

If bug 1757352 implemented , we could also 1) do something in https://searchfox.org/mozilla-central/rev/9013524d23da6523a7ec4479b5682407a1323f6c/mobile/android/actors/ContentDelegateChild.sys.mjs#39 (such as check if there's text selection then showActionMode ) or 2) let SelectionActionDelegateChild listening on contextmenu event too ?

Comment 3

[Cathy Lu [:calu]]

Following up on this, in option 1, do you know of a way to check for selected text with elementType or nearestParentAttribute?

For option 2, to me it looks like SelectionActionDelegateChild makes actions on the accessibility caret, whereas maybe the BasicSelectionActionDelegate is calling (onCreateActionMode)[https://searchfox.org/mozilla-central/rev/21823adb7d8117239b7021fa5997d3234369568a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java#338] to actually creation and prepare the actions for selection. I'm not sure how onCreateActionMode gets called though. It works for long presses and needs to work for right clicks.

Comment 4

[jackyzy823]

Hi

1, do you know of a way to check for selected text with elementType or nearestParentAttribute?

I'm not sure if i understood the question. I think it is no, the event target won't be text node. so there's no info about the selected text range. (unlike mozcaretstatechanged event)

[0] l learned from here https://bugzilla.mozilla.org/show_bug.cgi?id=1805341#c16
[1] https://searchfox.org/mozilla-central/source/layout/base/PresShell.cpp#12071-12083

I'm not sure how onCreateActionMode gets called though. It works for long presses and needs to work for right clicks.

Some calling chain:
[0] Child dispatch ShowSelectionAction to parent https://searchfox.org/mozilla-central/source/mobile/android/actors/SelectionActionDelegateChild.sys.mjs#389
[1] parent dispatch GeckoView:ShowSelectionAction to Geckoview https://searchfox.org/mozilla-central/rev/21823adb7d8117239b7021fa5997d3234369568a/mobile/shared/actors/SelectionActionDelegateParent.sys.mjs#50
[2] https://searchfox.org/mozilla-central/rev/21823adb7d8117239b7021fa5997d3234369568a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#1152-1159
[3] Callback interface like onCreateActionMode for action modes Supplied to View#startActionMode(Callback) https://searchfox.org/mozilla-central/rev/21823adb7d8117239b7021fa5997d3234369568a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/BasicSelectionActionDelegate.java#477-501
[4] https://developer.android.com/reference/android/view/ActionMode.Callback

Comment 5

[Makoto Kato [:m_kato]]

If possible, we should handle it on Gecko side.

Comment 6

[Cathy Lu [:calu]]

Gotcha, thanks for providing the callback documentation for ActionMode, sounds like we'll need to update SelectionActionDelegateChild to include right clicks. It looks like "longpressonemptycontent", "releasecaret", "taponcaret", "updateposition" (https://searchfox.org/mozilla-central/rev/a44891c52387ca4bd7c35b50f0d335f3980ef36a/mobile/shared/actors/SelectionActionDelegateChild.sys.mjs#330-389) already starts the action menu. Is there an event we can add for right click, is that something the DOM selection team can help add on the Gecko side? (Tagging mcrr8, triage owner of the Dom selection team).

Comment 7

[Noah Bond [:007]]

Notes from today's pairing session with Cathy

Do we need to add events for a right click to start the existing action mode call chain?

potential events that deal with highlights
"longpressonemptycontent",
"releasecaret",
"taponcaret",
"updateposition",

We have found:

• Where the right click gesture lives in Gecko
• Where the right click is being handled for contextual menu in Gecko
• Where the highlight call happens (selection action delegate) (moz caret state changed?)

We tested that Chrome does not do anything for right clickingon text; it only shows a contextual menu for right clicking on highlighted text.

Next time todo:

• Figure out logging in CPP files
• Should the right click trigger the action menu or should it reveal a contextual menu with the same expected actions?
• Figure out how to either: propagate the right click to action mode OR show a contextual menu on highlighted with the same actions in Action Mode

Comment 8

[Andrew McCreight [:mccr8]]

I don't know who might be available to work on Selection. Maybe Hsin-Yi can provide some suggestions.

Comment 9

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

According to the discussion with Masayuki, the root cause here is that we don't support mouse input on mobile formally/by default. We agreed with makoto-san’s comment 5; ideally we should handle this on Gecko. We need thorough audits and tests to support the mouse input properly, and based on our best educated guess, this may not be easy tweaks. I will have to take this to our planning backlog.

In the meanwhile, I know that Makoto-san has more recent experiences in handling ui events on mobile. Please feel free to chime in to point us where the gecko change may be (looks like this?) or correct me. Thanks!

Comment 10

[Makoto Kato [:m_kato]]

Looking desktop implementation. contextmenu for text selection handles on browser side, not Gecko (and toolkit).

ContextMenu actor handles contextmenu event then use XUL popup via nsContextMenu. Also, on some situation, contextmenu handles by moz-input-box.js for input type=text or textarea.

Actually, we get a selected text, linked title and etc via SelectionUtils.sys.mjs. If contextmenu's event (MouseEvent) have some extra information (that is got from SelectionUtils now) in chrome only member, it may be more easy to implement this for non-Firefox desktop. But I guess that adding code isn't large even if we implement mobile only handler for contextmenu.

At least, it may be better to split contextmenu actor from ContentDelegate actor, like desktop's, or handle this event by SelectionActionDelegate actor too. And if we re-use desktop code, we add more common module or into SelectionUtils.

Comment 11

[Makoto Kato [:m_kato]]

(Also, we need to re-check contextmenu event on mobile whether this is by mouse event or touch event).

Comment 12

[Cathy Lu [:calu]]

Hey Makoto, thanks for assigning yourself to this.

Sounds like desktop already uses SelectionUtils.getSelectionDetails and passes that data as part of "contextmenu" event. Is that how mobile "contextmenu" event gets it or where does this event come from?

Additionally, Jackyzy mentioned in c1, setting layout.accessiblecaret.hide_carets_for_mouse_input to false could give the same behavior. Can you test this preference?

Comment 13

[Makoto Kato [:m_kato]]

(In reply to Cathy Lu [:calu] from comment #12)

Hey Makoto, thanks for assigning yourself to this.

Sounds like desktop already uses SelectionUtils.getSelectionDetails and passes that data as part of "contextmenu" event. Is that how mobile "contextmenu" event gets it or where does this event come from?

We already dispatch contextmenu event by right mouse click (or long click by left button when by preference). Actually ContentDelegate actor handles this event to call GeckoView APIs for browser application.

Additionally, Jackyzy mentioned in c1, setting layout.accessiblecaret.hide_carets_for_mouse_input to false could give the same behavior. Can you test this preference?

Not related for this. Simply, we can open action menu if there is selected item. Current implementation of action menu depended on accessible caret events.

Comment 14

[jackyzy823]

Attachment: firefox_hide_on_mouse_off_automatically_show_actionmode_when_left_mouse_mouseup.mp4

With "layout.accessiblecaret.hide_carets_for_mouse_input" , the action mode show automatically when left mouse mouseup after text range selection.

Comment 15

[jackyzy823]

Attachment: firefox_normal_mouse.mp4

Comment 16

[jackyzy823]

Attachment: chrome_show_actionmode_after_mouse_right_click.mp4

Take chrome's behavior as a reference.

Comment 17

[Cathy Lu [:calu]]

Gotcha, thanks for the clarification. Also updated the title of the bug to open contextmenu and not action mode, based on product decision.

Comment 18

[Cathy Lu [:calu]]

Makoto, what do you think the targeted release is for this? Do you have an idea?

Comment 19

[Makoto Kato [:m_kato]]

(In reply to Cathy Lu [:calu] from comment #18)

Makoto, what do you think the targeted release is for this? Do you have an idea?

I hope next train (129).

Comment 20

[Makoto Kato [:m_kato]]

Actually, no way to detect whether contextmenu is by mouse or touch (long tap) since event.buttons value is same value (although Chrome is different). If C++ code, we will be able to get event source type. APZ caller sets button is 2, and WidgetMouseEvent's ContextMenuTrigger doesn't have long tap type.

But dom side will change event type for contextmenu by Bug 1675847, so after it, we can detect whether touch or mouse for contextmenu.

Comment 21

[Cathy Lu [:calu]]

Thanks for the update, looks like the dependent patch has merged and context menu now specifies the input source.

Comment 22

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/jobs?repo=try&revision=40fe341020c52554c8c3b30609356ace264f7761

Comment 23

[Makoto Kato [:m_kato]]

Attachment: Bug 1879701 - Show action menu when contextmenu is dispatched by mouse. r=#geckoview-reviewers

When contextmenu event is fired by mouse and the selection of active element
isn't collapsed, we should show action menu instead of context menu.

Actually, action menu depends on accessible caret event, so I emulate this
event to handle action menu. Also, focus or selection for active element is
changed, action menu should be dismissed.

Desktop's Gecko uses some JavaScript modules for handling context menu, so
we also use it.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c8b5acde34f8
Show action menu when contextmenu is dispatched by mouse. r=geckoview-reviewers,calu

Comment 25

[Cosmin Sabou [:CosminS]]

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

Comment 26

[Cathy Lu [:calu]]

Hey QA, can you verify this fix that right clicking with a tablet mouse on tablets for selected text pulls up the action mode context mode?
Thanks!

Comment 27

[Delia Pop]

Attachment: 1723017774146.JPEG

Verified as fixed on the latest Nightly 131.0a1 from 08/07 with Samsung Tab S8 Ultra 5G (Android 14) and an external mouse connected. The action menu is displayed after right clicking the selected text.
We've also verified that the "Context_menu" events are correctly sent here.