1710474 - Native context menu sometimes does not open when using two-finger-click on touchpad

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P1)

Description

[Cam]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

Two finger (right) click and moved the mouse down in anticipation of where the context menu item would be that I wanted to select.

Actual results:

Context menu did not appear.

Expected results:

Context menu should appear, then when the click is released, the hovered menu item (if any) should be chosen.

This is the expected behaviour on macOS.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Cocoa' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Stephen A Pohl [:spohl]]

Are you able to reproduce this reliably? And can you confirm that you are using the trackpad for the two-finger click? I don't seem to be able to reproduce this at the moment.

Comment 3

[Cam]

I can 100% reproduce this if I for example try to right click the Preview link above the comment text field on this page using my trackpad while the text field is not selected. No idea why that mattes but it does reproduce very reliably.

Comment 4

[Mehmet]

I can reproduce this with widget.macos.native-context-menus enabled with Nightly 90.0a1 (2021-05-22) on macOS 11.3.1, MacBookAir M1.

Steps to reproduce:

1.) Make sure that widget.macos.native-context-menus is enabled
2.) Configure 2-finger-click as a right click in your OS Settings
3.) Visit any page
4.) Do a 2-finger-click to open the context menu
5.) Do a 1-finger-click to hide the context menu
6.) Do again a 2-finger-click to open the context menu again

Actual: Context menu does not open.

Expected: Context menu should open.

This isn't reproducible when you create a right-click by holding the Control-Key and doing a single click on the trackpad.

Comment 5

[Mehmet]

Adding "Blocks: bug 1700679". Please remove if this is not the case. Thanks.

Comment 6

[Markus Stange [:mstange]]

I can reproduce this intermittently - two-finger clicking sometimes doesn't open the menu.

What seems to happen is that we attempt to open the menu, but then cancel it because a scrollWheel event calls maybeRollup. The scrollWheel call is because having two fingers on the touchpad shows scrollbars, and lifting the fingers hides scrollbars, and this notification is delivered as a scroll event.

Stack:

nsMenuX::Close()
nsXULPopupManager::RollupNativeMenu()
-[ChildView maybeRollup:]
-[ChildView scrollWheel:]
-[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:]
(root)

Comment 7

[Markus Stange [:mstange]]

This is easier to hit if you rest the fingers on the touchpad for a slight moment (~500ms) before pressing down. But even then the menu opens most of the time.

Comment 8

[Mehmet]

Thanks for confirming the issue and taking a look into it. I can also confirm that this isn't happening regulary. It mostly happen, when you hide the menu with a left-click (1-finger-click) and then again do a right-click (2-finger-click). If the menu is already open and you do again a 2-finger-click, then the menu reopens immediately - but as you noted, also not regulary :(

Comment 9

[Mehmet]

And it works always when you visit a website freshly. That means, reload the page and do 2-finger-click. The menu will open immediately. But if you then hide the menu und try to open it again, it will fail.

Comment 10

[Markus Stange [:mstange]]

Attachment: Bug 1710474 - Don't roll up native context menus in response to wheel events. r=#mac-reviewers

I didn't add this check originally because the thinking was that macOS wouldn't dispatch
these events to us anyway as long as a menu is open.
However, our menu opening is asynchronous. So we can still get wheel events between the
mousedown that asks to open the menu, and the delayed perform which then ends up opening
the menu for real. We don't want to cancel opening when we get these straggler events.

Comment 11

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/jobs?repo=try&revision=8707e64e4f22d15a0282f8c3be847bcaa10d5c40

Comment 12

[Markus Stange [:mstange]]

[Tracking Requested - why for this release]: Regression. Seems to only happen intermittently, but can surely be annoying.

Comment 13

[Markus Stange [:mstange]]

I missed this bug when it was filed because I didn't get bugmail for UNCONFIRMED bugs in components I watch. I have now found the fateful checkbox in the bugzilla preferences and unchecked it, so hopefully this shouldn't happen again.

Comment 14

[Markus Stange [:mstange]]

Cam, Mehmet, can you test this build which contains the fix? https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/cLrccqbWQyuHnm2-S2H4FA/runs/0/artifacts/public/build/target.dmg

Comment 15

[Mehmet]

(In reply to Markus Stange [:mstange] from comment #14)

Cam, Mehmet, can you test this build which contains the fix? https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/cLrccqbWQyuHnm2-S2H4FA/runs/0/artifacts/public/build/target.dmg

Hi Markus,

yes, your build fixes that bug for me. But I noticed that it breaks bug 1704080, which was fixed 2 days ago. The bug that was fixed there was "overscroll should work on pages without scrollbars", eg. at google.com. Is your build made from the latest Nightly or was it an older one?

Comment 16

[Markus Stange [:mstange]]

Thanks for testing! Yes, the build uses a slightly outdated base changeset; it doesn't include the patch from bug 1704080.

Comment 17

[Mehmet]

(In reply to Markus Stange [:mstange] from comment #16)

Thanks for testing! Yes, the build uses a slightly outdated base changeset; it doesn't include the patch from bug 1704080.

Ah, okay. Thanks :) Then as already confirmed, your fix looks good to me in your provided build. Thank you :)

Comment 18

[Pascal Chevrel:pascalc (PTO until April 26)]

Markus, is that something that needs fixing in 89 RC (we build RC later today). I would be happy to delay building RC a few hours to uplift a safe patch as this is marked as P1 for the proton launch and you requested tracking. If you think we can ship RC without it, I could take it in a RC2 as a ride-along or a potential 89 dot release. Thanks

Comment 19

[Markus Stange [:mstange]]

(In reply to Pascal Chevrel:pascalc from comment #18)
If 89 RC hasn't been cut yet, I'd appreciate if you could take this patch in it. Otherwise ride-along or dot release is fine too but it would be better to not ship this regression at all.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6b8125adf26
Don't roll up native context menus in response to wheel events. r=mac-reviewers,spohl,bradwerth

Comment 21

[Markus Stange [:mstange]]

Comment on attachment 9223110 [details]
Bug 1710474 - Don't roll up native context menus in response to wheel events. r=#mac-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Two-finger touchpad clicks are sometimes ignored and don't open a context menu. Intermittent issue and a regression.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 4 and comment 7
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Extremely low-risk targeted fix
• String changes made/needed:

Comment 22

[Narcis Beleuzu [:NarcisB]]

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

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9223110 [details]
Bug 1710474 - Don't roll up native context menus in response to wheel events. r=#mac-reviewers

Approved for 89.0rc1.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/f62a712208b3368f415c5b96260a366c16e9b637

Comment 25

[Bogdan Maris, Desktop QA]

We tried to reproduce this issue on three different machines (two 10.15 and one 11.3) but we were unable to do so using builds before the fix (89.0b15 and Nightly from 2021-05-10), so we can't verify that this is fixed.

Mehmet, will you be so kind and verify that this is fixed using Firefox 89.0 release build and latest Nightly?

Firefox 89.0 - https://archive.mozilla.org/pub/firefox/candidates/89.0-candidates/build1/mac/en-US/Firefox%2089.0.dmg
Latest Nightly - https://archive.mozilla.org/pub/firefox/nightly/2021/05/2021-05-24-21-58-32-mozilla-central/firefox-90.0a1.en-US.mac.dmg

Comment 26

[Mehmet]

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #25)

We tried to reproduce this issue on three different machines (two 10.15 and one 11.3) but we were unable to do so using builds before the fix (89.0b15 and Nightly from 2021-05-10), so we can't verify that this is fixed.

Mehmet, will you be so kind and verify that this is fixed using Firefox 89.0 release build and latest Nightly?

Firefox 89.0 - https://archive.mozilla.org/pub/firefox/candidates/89.0-candidates/build1/mac/en-US/Firefox%2089.0.dmg
Latest Nightly - https://archive.mozilla.org/pub/firefox/nightly/2021/05/2021-05-24-21-58-32-mozilla-central/firefox-90.0a1.en-US.mac.dmg

Hi Bogdan,

I can confirm that it is working for me in both builds on macOS 11.4. The native context menu appears as expected every time when using the 2-finger-click as a right click.

Thanks Markus for fixing this!

Comment 27

[Bogdan Maris, Desktop QA]

(In reply to Mehmet from comment #26)

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #25)

We tried to reproduce this issue on three different machines (two 10.15 and one 11.3) but we were unable to do so using builds before the fix (89.0b15 and Nightly from 2021-05-10), so we can't verify that this is fixed.

Mehmet, will you be so kind and verify that this is fixed using Firefox 89.0 release build and latest Nightly?

Firefox 89.0 - https://archive.mozilla.org/pub/firefox/candidates/89.0-candidates/build1/mac/en-US/Firefox%2089.0.dmg
Latest Nightly - https://archive.mozilla.org/pub/firefox/nightly/2021/05/2021-05-24-21-58-32-mozilla-central/firefox-90.0a1.en-US.mac.dmg

Hi Bogdan,

I can confirm that it is working for me in both builds on macOS 11.4. The native context menu appears as expected every time when using the 2-finger-click as a right click.

Thanks Markus for fixing this!

Thanks for the prompt verification!
Marking this bug as verified fixed based on the above.