1753414 - Contextmenu not working properly after switching tab

Status: VERIFIED FIXED

(Firefox :: Menus, defect, P3)

Description

[Mirko Brodesser (:mbrodesser-Igalia), offline until 10th of Feb]

STR:

1) Open some page.
2) Select some text.
3) Right click to open the context menu.
4) Switch tabs with Ctrl+PageDown.
5a) Click on "Inspect (Q)".
5b) Click "Copy".

After 5a) observe the errors:

JavaScript error: resource://devtools/client/fronts/inspector.js, line 286: TypeError: can't access property "getFront", target is null
JavaScript error: resource://devtools/server/connectors/js-window-actor/DevToolsFrameParent.jsm, line 217: TypeError: can't access property "notifyTargetDestroyed", watcher is undefined

After 5b) the clipboard content didn't change.

However, other functionality like "Open Link in New Tab" works after step 4).

It could be worth comparing what other browsers do here. On Ubuntu, I wasn't able to switch tabs using Ctrl+PageDown after the context menu was opened.

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia), offline until 10th of Feb]

With Chrome, switching tabs via pressing the keys Alt+1 is blocked when a context menu is opened.

For end-users, I see little benefit of being able to switch tabs, while a context menu is opened. Also, the actual context is invisible when having switched tabs.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Ignoring keyboard commands to switch tabs while a context menu is open would be pretty straightforward, though we would need to add that code in quite a few places I assume.

We already ignore "Ctrl+N" to open a new window when a context menu is opened. I'm not sure if that is because they key has reserved=true (https://searchfox.org/mozilla-central/rev/072f9e6b7f10a00e12d0a02ac713431d0a7ee368/browser/base/content/browser-sets.inc#119) or if there is some other code that does this.

Neil, what do you think about making this change?

Comment 3

[Neil Deakin]

Menus should already be cancelling shortcut keys while they are open, but this doesn't look like a regression, or at least the same issue occurs in a build I tried from 2019 at least.

My testing shows that the menu code in XULPopupManager appears to cancel the event properly, so something else is failing here.

Of note, shortcuts like Ctrl+T are properly ignored, but Ctrl+PageDn is not.

Since this appears to only fail on Linux, I wonder if this is related to native key binding support. Maybe masayuki knows where to start looking?

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Neil Deakin from comment #3)

Of note, shortcuts like Ctrl+T are properly ignored, but Ctrl+PageDn is not.

Since this appears to only fail on Linux, I wonder if this is related to native key binding support. Maybe masayuki knows where to start looking?

This also fails on Windows.

• Ctrl+PageDn switches tabs and leaves the context menu open
• Ctrl+T switches tabs but closes the menu
• Ctrl+N fails to open a new window while the page context menu is open

Comment 5

[Neil Deakin]

This also fails on Windows.

• Ctrl+PageDn switches tabs and leaves the context menu open
• Ctrl+T switches tabs but closes the menu

I can't reproduce the Ctrl+T behaviour. The menu stays open and no tab is opened. I do see the Ctrl+PageDn behaviour however.

Comment 6

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

It seems that it's caused by defaultPrevented not checked.
https://searchfox.org/mozilla-central/rev/4615b544a0f7166233c409c619b426c4025467a7/browser/base/content/tabbrowser.js#5335-5336,5342,5358

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia), offline until 10th of Feb]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)

Ignoring keyboard commands to switch tabs while a context menu is open would be pretty straightforward, though we would need to add that code in quite a few places I assume.

We already ignore "Ctrl+N" to open a new window when a context menu is opened. I'm not sure if that is because they key has reserved=true (https://searchfox.org/mozilla-central/rev/072f9e6b7f10a00e12d0a02ac713431d0a7ee368/browser/base/content/browser-sets.inc#119) or if there is some other code that does this.

Likely not, but perhaps https://searchfox.org/mozilla-central/rev/2a0b0ababd4541ecffb74cbe0820a9d0a25da636/browser/locales/en-US/browser/browserContext.ftl#334-336 prevents this.

Neil, what do you think about making this change?

Comment 8

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)

(In reply to Neil Deakin from comment #3)

Of note, shortcuts like Ctrl+T are properly ignored, but Ctrl+PageDn is not.

Since this appears to only fail on Linux, I wonder if this is related to native key binding support. Maybe masayuki knows where to start looking?

This also fails on Windows.

• Ctrl+PageDn switches tabs and leaves the context menu open
• Ctrl+T switches tabs but closes the menu
• Ctrl+N fails to open a new window while the page context menu is open

Like Neil (comment #5), I can't reproduce the Ctrl-T behaviour. Jared, can you doublecheck/screencast/clarify the circumstances in which you're seeing this?

I think Masayuki's diagnosis in comment #6 seems accurate as far as ctrl-pageup/down goes; we can use defaultPreventedByChrome to avoid web content being able to prevent tab navigation shortcuts from working. That should be a straightforward patch.

The comments in this bug are now a bit confusing. Leaving aside Jared seeing this for Ctrl-T on Windows, is the ctrl-pageup/dn case the only one where this breaks on linux, or do "regular" shortcuts like Ctrl-N / Ctrl-T also bypass the menu-based blocking of shortcuts that Neil mentions in comment #3?

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: ctrltab.gif

This attachment shows me right-clicking on a page, then pressing "Ctrl+T" on my keyboard. The selected tab changes and the menu closes. This is on Firefox Nightly 99.0a1 (2022-02-08) (64-bit) on Windows 10.

Comment 10

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)

Created attachment 9263795 [details]
ctrltab.gif

This attachment shows me right-clicking on a page, then pressing "Ctrl+T" on my keyboard. The selected tab changes and the menu closes. This is on Firefox Nightly 99.0a1 (2022-02-08) (64-bit) on Windows 10.

I'm confused - the screencast shows us switching to (selecting) the next tab, instead of opening a new tab (which is what Ctrl-T is supposed to do). What am I missing?

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

I'm sorry. I was confused and was using Ctrl+Tab in my previous testing and in the screenshot. That explains why nobody could reproduce what I saw. Indeed, Ctrl+T does not open a new tab. I must say I'm embarrased.

Comment 12

[:Gijs (he/him)]

Attachment: Bug 1753414 - stop switching tabs with the context menu open, r?NeilDeakin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2ccb6c1d2e9
stop switching tabs with the context menu open, r=NeilDeakin

Comment 14

[Cristina Cozmuta (:CrissCozmuta)]

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

Comment 15

[Monica Chiorean]

I tried to reproduce the issue on Win10 using a build without fix 98.0a1(20220203190329), and I was able to switch tabs using CTRL+PgDn while context menu is opened.
On fixed version Beta 99.0 (20220328190900) on Win10 and Ubuntu 20.4 I am not able to switch tabs using CTRL+PgDn while context menu is opened. Is this the intended behavior? Thank you.

Comment 16

[:Gijs (he/him)]

(In reply to Monica Chiorean from comment #15)

I tried to reproduce the issue on Win10 using a build without fix 98.0a1(20220203190329), and I was able to switch tabs using CTRL+PgDn while context menu is opened.
On fixed version Beta 99.0 (20220328190900) on Win10 and Ubuntu 20.4 I am not able to switch tabs using CTRL+PgDn while context menu is opened. Is this the intended behavior? Thank you.

Yes.

Comment 17

[Monica Chiorean]

Mark issue as verified based on above comment.