ui.click_hold_context_menus's synthetic contextmenu event should suppress follow-on click event

NEW
Unassigned

Status

()

7 years ago
2 years ago

People

(Reporter: asuth, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(blocking-basecamp:-)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Created attachment 635158 [details]
Example page with event listeners if you have changed your prefs

Bug 563329 formalized the ability for a long-press to trigger a contextmenu event.  This event is annoying because it is still followed by a "click" event when the press is released, unlike a normal contextmenu.  In my case, I am using contextmenu to pop-up a menu, but the follow-on click then tricks me into closing the menu again.  The click might be useful if I wanted the user to select and trigger the menu in a single finger press-and-release, but arguably I could use mouseup in that case.

Using the attached example file, witness the following event sequences (reversed to be in normal chronological order) using firefox with the preferences set like below:

== Long press and hold of left-mouse-button w/1000 milli delay
event: mousedown button: 0 @ 0.388 secs after start
event: contextmenu button: 2 @ 1.387 secs after start
event: mouseup button: 0 @ 2.187 secs after start
event: click button: 0 @ 2.189 secs after start

== Right-click
event: mousedown button: 2 @ 8.995 secs after start
event: contextmenu button: 2 @ 9.010 secs after start
event: mouseup button: 2 @ 9.123 secs after start


Preferences one should set, and yes, this works in desktop firefox:

// definitely required on 
pref("ui.click_hold_context_menus", true);
// defaults to 500 if omitted
pref("ui.click_hold_context_menus.delay", 1000);


Preferences one might set:

// required if the element in question is not styled "user-select: none"
pref("browser.ignoreNativeFrameTextSelection", true);


NB: "browser.ignoreNativeFrameTextSelection" stops text selection and is required for the logic to do anything in many cases unless a "user-select: none" style rule is in effect because nsEventStateManager::GenerateDragGesture will immediately kill the drag-gesture if it sees frame selection is active.
(Reporter)

Comment 1

7 years ago
Oh right, and this is important/relevant for B2G because I am writing the B2G e-mail app and we want to fix the platform rather than fill our apps with hacks.
Component: DOM: Events → Event Handling
(Reporter)

Comment 2

7 years ago
Nominating for base-camp since I think Vivien wanted to avoid having a workaround in content space for ignoring the click.
blocking-basecamp: --- → ?
Blocking-. It's not clear we're using context menus often, nor that this is required to ship V1. Please provide rationale and re-nominate if you think the release should block on this.
blocking-basecamp: ? → -
(In reply to Dietrich Ayala (:dietrich) from comment #3)
> Blocking-. It's not clear we're using context menus often, nor that this is
> required to ship V1. Please provide rationale and re-nominate if you think
> the release should block on this.

It's not about contextmenu. It's more about longpress. Context menu events are fired after a certain amount of time on a press.
Can you clarify whether or not this needs to block the release of V1?
(Reporter)

Comment 6

7 years ago
E-mail depends on the long-presses per UX spec.  E-mail currently gets them via this native implementation because Vivien likes it and it is cleaner.  The current implementation screws up the e-mail UI.  If you bring up the e-mail app right now and long press and are careful not to move your mouse/finger too much during the long hold (moving turns it into a drag), you will find that the menu shows up 1 second after you start pressing, but goes away immediately when you lift your finger.

There is a workaround, which is to ignore the platform contextmenu event and instead do the logic in JS like the gesture detector does.  Or only use b2g-desktop and just right-click your mouse instead of long-pressing with the left mouse mutton.
(Reporter)

Comment 7

7 years ago
Sorry, long-press on an e-mail message in the list of messages in a folder.  Works on the fake account.  You can also long-press on an account in the settings page to delete an account.
When we fix this bug in gecko, we'll also need to fix it in gaia/shared/js/mouse_event_shim.js
Created attachment 8774377 [details]
Coded workaround to suppress follow-on click event

FireContextClick() method of dom/events/EventStateManager.cpp. Workaround on line 74.
This bug has become prominent in the most recent beta version (48.0) of Firefox for Android. Pre-48.0, long-clicking on a URL, for example, produced a context menu. Post-48.0 (tested on 48.0b6 through to 48.0b10 and on the nightly 50.0a1), long-clicking isn't enabled unless the ui.click_hold_context_menus option is set to true. This seems like correct behaviour. However, on releasing the left-mouse button, the target URL loads in the current tab, as originally reported by Andrew.

Workaround is to move the mouse away from the link before releasing the left-mouse button.

If using Firefox for Android with a mouse (as I am), there is no bug-free way to produce a context menu post-48.0. Right-mouse click does nothing and the context menu is no longer produced by clicking-and-holding without the ui.click_hold_context_menus option, as described.

I'm currently using a custom build that has a coded workaround for the problem to suppress the follow-on click event, although it's certainly not graceful. See attachment.
(Reporter)

Comment 11

2 years ago
You probably want to provide a diff/patch instead of the modified function.  See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch for general docs relating to this.
(In reply to Andrew Sutherland [:asuth] from comment #11)
> You probably want to provide a diff/patch instead of the modified function. 
> See
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> How_to_Submit_a_Patch for general docs relating to this.

Thanks for the tip. This is just a workaround that I don't intend to have reviewed or merged at any point. Is it still pertinent to provide a diff?
(Reporter)

Comment 13

2 years ago
The patch is certainly more useful in general.  But you're right, it doesn't matter too much if no one is going to try and move it forward, and the patch can be re-derived from what you've provided.  Gecko hacking and Bugzilla can be very non-intuitive, and even finding pages like that can be a endeavor, so I thought I'd drop the link in case you were interested in getting a fix in tree.  (To be clear, it's perfectly fine and still helpful to leave the modified code here!)
Many thanks for the advice, I'll keep it in mind for next time!
You need to log in before you can comment on or make changes to this bug.