1805341 - long pressing/clicking with a mouse doesn't bring up the context menu so I can't open links in a new tab

Status: RESOLVED DUPLICATE of bug 1891221

(GeckoView :: General, defect, P3)

Description

[Jonathan Almeida [:jonalmeida]]

From github: https://github.com/mozilla-mobile/fenix/issues/14412.

Steps to reproduce

Hook a mouse a mouse up to an android device (phone, tablet, laptop, SBC, whatever) and long press/click on a link in firefox.

Expected behavior

Long pressing/clicking on a link should cause a context menu to pop-up allowing you to do things like open the link in a new tab. This is the way it worked in the previous version of firefox.

Actual behavior

The menu does not pop-up, nothing happens.

Device information

• Android device: ?
  lenovo T440p laptop running the Bliss ROM

• Fenix version: ?
  79.0.5 (Build #2015758623)

Additional Information

On my phone (running android 7 or so) long pressing on a link with a finger on a link brings up the expected context menu, long pressing/clicking with a mouse does not bring up the context menu.

My laptop (the one running android) does not have a touchscreen so poking things with my finger will not do anything, this mouse issue leaves me with no way to open a link in a new tab.

I should also mention that right clicking on a link with a mouse does not bring up a context menu either (this feature has not worked since quite a few versions back).

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Chris Peterson [:cpeterson]]

Does right-clicking work in Android WebView? Mihai says he remembers seeing somewhere that it does not work on web views either.

Comment 2

[Jonathan Almeida [:jonalmeida]]

For anyone working on this bug that doesn't have an android tablet or OTG enabled device: I was able to reproduce this bug by using scrcpy with otg mode on a physical device to share my mouse on it: scrcpy --otg.

Comment 3

[Makoto Kato [:m_kato]]

• Android doesn't have context menu related event like Windows and macOS. View.OnLongClickListener may be help, but I am not sure how to detect whether current action is tap or mouse click.
• On Linux/gtk, Gecko emulates context menu when right click. (We have a prefs whether down or up).
• We has gesture detector in APZ for touch event. (AsyncPanZoomController::HandleGestureEvent etc)

If we support context menu event by mouse's right click, I guess that we emulate it by widgtet side like Linux/gtk.

Comment 4

[v]

Please add context menu with RMB for Android

Comment 5

[Jonathan Almeida [:jonalmeida]]

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

If we support context menu event by mouse's right click, I guess that we emulate it by widgtet side like Linux/gtk.

Usability-wise, this makes the most sense to me.

• We has gesture detector in APZ for touch event. (AsyncPanZoomController::HandleGestureEvent etc)

This seems to be what Chrome's behaviour is.

Comment 6

[Makoto Kato [:m_kato]]

Botond, does APZ have a plan to dispatch context menu by mouse long tap or mouse right click? Android's GestureDetector seems to consider it and I am not sure whether we should handle this issue by widget or APZ.

Comment 7

[Botond Ballo [:botond]]

APZ mostly ignores mouse events (with the exception of handling a fast-path for scrollbar thumb dragging).

For right-click, I think it would be easiest to trigger the context menu similar to how it's done for GTK (in widget code).

For long-press, presumably we also want to not dispatch a click event in that case? If so, the best place to have this logic might be in EventStateManager in the content process, which is where click is currently synthesized (in DispatchClickEvents). We could add logic to record the timestamp of the mouse-down, and, on Android, if the interval between the mousedown and the mouseup is above a threshold, dispatch a contextmenu instead of a click.

Comment 8

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #7)

We could add logic to record the timestamp of the mouse-down, and, on Android, if the interval between the mousedown and the mouseup is above a threshold, dispatch a contextmenu instead of a click.

Actually, it looks like we already have such logic, gated on the ui.click_hold_context_menus pref. So maybe it's just a matter of turning this pref on for Android?

Comment 9

[Makoto Kato [:m_kato]]

(In reply to Botond Ballo [:botond] from comment #8)

(In reply to Botond Ballo [:botond] from comment #7)

We could add logic to record the timestamp of the mouse-down, and, on Android, if the interval between the mousedown and the mouseup is above a threshold, dispatch a contextmenu instead of a click.

Actually, it looks like we already have such logic, gated on the ui.click_hold_context_menus pref. So maybe it's just a matter of turning this pref on for Android?

Although I set this pref, no change unfortunately.

Comment 11

[jjdagdelen]

Hey there, have there been any updates on this? We're developing a geckoview-based browser for android devices that operates more like a desktop computing environment where the user will be using a physical mouse. It would be really nice for right click to trigger the contextmenu event. Would be happy to help in any way I can, although I have never checked in code to a Mozilla project before.

Comment 12

[Botond Ballo [:botond]]

(In reply to jjdagdelen from comment #11)

It would be really nice for right click to trigger the contextmenu event. Would be happy to help in any way I can, although I have never checked in code to a Mozilla project before.

If you'd like to try implementing the right-click part of this, I can give you some pointers to the relevant code:

• Here is the place in the GTK widget code where, during the processing of a native mouse event, we synthesize a contextmenu event.
• Here is the corresponding place in the Android widget code where we process a native mouse event.

The general idea would be to look at what we do in the GTK codepath, and add similar code to the Android codepath. The native representations of the mouse event are different, but in both cases the relevant information that needs to be checked (e.g. was the button that was pressed the right mouse button?) should be available.

Happy to discuss in more detail / answer questions if you're interested.

Comment 13

[jjdagdelen]

(In reply to Botond Ballo [:botond] from comment #12)

(In reply to jjdagdelen from comment #11)
Happy to discuss in more detail / answer questions if you're interested.

Sounds good. I'll take a look and get back to you if I have any questions/need more detail.

Comment 14

[jackyzy823]

Hello, all.

I try working on long-click to open contextmenu.

Actually, it looks like we already have such logic, gated on the ui.click_hold_context_menus pref. So maybe it's just a matter of turning this pref on for Android?

Although I set this pref, no change unfortunately.

I tested to set the pref ui.click_hold_context_menus to true in GeckoView Example , actually , it dispatches contextmenu event ,however in here , some conditions are not met, so it wouldn't send message to GeckoView part.

Take https://example.com as a example: If you long click on more information ... link
The event's composedTarget (and all related *Target attributes) is #text "More information..."

However if you right-click or long-tap (touch) on more information ... link
The event's composedTarget (and all related *Target attributes except explicitOriginalTarget) is <a href="https://www.iana.org/domains/example"> . explicitOriginalTarget is the same as long click one :#text "More information..."

Hope :botond could offer some help on this. Thanks.

Comment 15

[Botond Ballo [:botond]]

Attachment: A test page

I can reproduce the discrepancy described in comment 14 on desktop:

1. Open the attached test page
2. Right-click on the text
3. Observe that what is printed to the console is <p>, i.e. the target of the contextmenu event is the <p> element enclosing the text node
4. In about:config, set ui.click_hold_context_menus to true
5. Long-click on the text
6. Observe that what is printed to the console is #text "Some text", i.e. the target of the contextmenu event is the text node itself

I don't have a theory about the reason for this discrepancy, perhaps Masayuki has an idea about it.

Comment 16

[Botond Ballo [:botond]]

I did a bit of debugging in rr, and from what I can tell:

• The event's target is determined by the aTarget parameter of EventDispatcher::Dispatch()
• In the right-click case, the call site of EventDispatcher::Dispatch is this one, and the target comes from PresShell::mCurrentEventContent
  • The target starts off as being the text node, but slightly earlier during the event dispatching code, it's adjusted here to be the nearest enclosing element.
• In the long-click case, the call site of EventDispatcher::Dispatcher is this one, and the target comes from EventStateManager::mGestureDownContent, and no adjustment is made to it (it remains the text node)

So, a potential thing to try could be to make an adjustment to the gestureDownContent in EventStateManager::FireContextClick(), to walk up to the nearest enclosing element, before passing it to EventDispatcher::Dispatch().

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The target of contextmenu event is defined as Element, so, it's a bug of EventStateManager::FireContextClick() which you found. Does it cause the original report?

Comment 18

[jackyzy823]

Thanks for your help, :botond.

I found that desktop version also complains about this when getting refererrInfo .

JavaScript error: resource:///actors/ContextMenuChild.sys.mjs, line 600: NS_ERROR_XPC_BAD_CONVERT_JS: Could not convert JavaScript argument arg 0 [nsIReferrerInfo.initWithElement]

---

After modifying like this in dom/events/EventStateManager.cpp, Fenix and Desktop could trigger context menu.

@@ -2121,8 +2133,14 @@ void EventStateManager::FireContextClick() {
       AutoHandlingUserInputStatePusher userInpStatePusher(true, &event);
 
       // dispatch to DOM
-      RefPtr<nsIContent> gestureDownContent = mGestureDownContent;
+      nsIContent* targetEvent = mGestureDownContent;
+      while (targetEvent && !targetEvent->IsElement()) {
+        targetEvent = targetEvent->GetFlattenedTreeParent();
+      }
+
+      RefPtr<nsIContent> gestureDownContent = targetEvent;
       RefPtr<nsPresContext> presContext = mPresContext;

---

However there's a problem.

In Fenix, if i mousedown on the link and hold , the contextmenu will show, and then when i don't move mouse and then mouseup on the link , then navigating to the link will happens. If i move mouse out of the link then mouseup, this won't happen.

In desktop this won't happen.

I'm not sure if this is intentional .

---

Update:
It looks desktop won't dispatch "mouseup" event when contextmenu shown and mouse up. (so "click" won't be dispatched too).
I think this is not correct too :(
"mouseup" event should be dispatched, "click" should be prevented.

Further investigate on Desktop, When contextmenu shown, mouse down then mouse up in the area beside the target and the menu, then only one "mouseup" event will dispatched (no mousedown event ) .

Comment 19

[jackyzy823]

Also FireContextClick seems not respecting ui.context_menus_after_mouseup

Comment 20

[jackyzy823]

To prevent click event after contextmenu event triggered by click-holding

diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
index 5c90af7d8ba5a..d6bc8a133fd8b 100644
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -439,6 +439,7 @@ EventStateManager::EventStateManager()
       mGestureDownPoint(0, 0),
       mGestureModifiers(0),
       mGestureDownButtons(0),
+      mGestureDownButton(0),
       mPresContext(nullptr),
       mShouldAlwaysUseLineDeltas(false),
       mShouldAlwaysUseLineDeltasInitialized(false),
@@ -2128,6 +2129,14 @@ void EventStateManager::FireContextClick() {
 
       RefPtr<nsIContent> gestureDownContent = targetEvent;
       RefPtr<nsPresContext> presContext = mPresContext;
+
+      // Disable clickEvent!
+      event.mClickEventPrevented = true;
+      LastMouseDownInfo& mouseDownInfo = GetLastMouseDownInfo(event.mButton);
+      mouseDownInfo.mLastMouseDownContent = nullptr;
+      mouseDownInfo.mClickCount = 0;
+      mouseDownInfo.mLastMouseDownInputControlType = Nothing();
+
       EventDispatcher::Dispatch(gestureDownContent, presContext, &event,
                                 nullptr, &status);
 
@@ -2181,6 +2190,7 @@ void EventStateManager::BeginTrackingDragGesture(nsPresContext* aPresContext,
   }
   mGestureModifiers = inDownEvent->mModifiers;
   mGestureDownButtons = inDownEvent->mButtons;
+  mGestureDownButton = inDownEvent->mButton;
 
   if (inDownEvent->mMessage != eMouseTouchDrag &&
       StaticPrefs::ui_click_hold_context_menus()) {
@@ -2254,6 +2264,7 @@ void EventStateManager::FillInEventFromGestureDown(WidgetMouseEvent* aEvent) {
       mGestureDownPoint - aEvent->mWidget->WidgetToScreenOffset();
   aEvent->mModifiers = mGestureModifiers;
   aEvent->mButtons = mGestureDownButtons;
+  aEvent->mButton = mGestureDownButton;
 }
 
 void EventStateManager::MaybeFirePointerCancel(WidgetInputEvent* aEvent) {
@@ -3632,8 +3643,6 @@ nsresult EventStateManager::PostHandleEvent(nsPresContext* aPresContext,
       // we need to forget the clicking content and click count for the
       // following eMouseUp event.
       if (mouseEvent->mClickEventPrevented) {
-        RefPtr<EventStateManager> esm =
-            ESMFromContentOrThis(aOverrideClickTarget);
         switch (mouseEvent->mButton) {
           case MouseButton::ePrimary:
           case MouseButton::eSecondary:
diff --git a/dom/events/EventStateManager.h b/dom/events/EventStateManager.h
index a4e709d507025..6f9710b282aad 100644
--- a/dom/events/EventStateManager.h
+++ b/dom/events/EventStateManager.h
@@ -1271,6 +1271,7 @@ class EventStateManager : public nsSupportsWeakReference, public nsIObserver {
   // State of keys when the original gesture-down happened
   Modifiers mGestureModifiers;
   uint16_t mGestureDownButtons;
+  uint16_t mGestureDownButton;
 
   LastMouseDownInfo mLastLeftMouseDownInfo;
   LastMouseDownInfo mLastMiddleMouseDownInfo;