492320 - Keyboard shortcut for "Undo close window" fails if no windows are open (and "recently closed windows" submenu hasn't been shown)

Status: VERIFIED FIXED

(Firefox :: Keyboard Navigation, defect, P5)

Description

[Jesse Ruderman]

1. Launch Firefox
2. ⌘W (close the only browser window without quitting Firefox)
3. ⇧⌘N (try to reopen the window)

Result: fail beep

4. Open "History" menu
5. Open "Recently Closed Windows" submenu
6. Close the menus
7. ⇧⌘N

Result: now it works

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Does this also happen if there are arbitrary open/close tab or window steps between 2 and 3? Trying to gauge effect.

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #1)
> Does this also happen if there are arbitrary open/close tab or window steps
> between 2 and 3? Trying to gauge effect.

Still happens. Close any number of windows until you have 0 left and the keyboard shortcut doesn't work (so long as you don't access the menu at all).

The question is, is that a problem with out menu system? The keyboard shortcut works if you have a window open but have never accessed the History menu.

If not I guess the "easy" solution is to listen for "domwindowclosed" and force the menu to update every time (or call out from sessionstore if that's possible)

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Sucks, but doesn't block as I don't think we're gonna have a lot of people reaching for that oh-so-easy three-fingered command sequence.

We should fix it, though.

Comment 5

[Colin Barrett [:cbarrett]]

I hit this probably once a day. It's a really frustrating paper cut. Please? :)

Comment 6

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Random theory - does this not work because the menuitem with the keyboard shortcut is never generated until the menu is shown? Could we maybe fix this by adding a dummy entry in the menu with the keyboard shortcut?

Comment 7

[Jesse Ruderman]

This bug essentially means that the keyboard shortcut for "Undo close window" does not work for people who only keep one window open.

Comment 8

[Colin Barrett [:cbarrett]]

(In reply to comment #6)
> Random theory - does this not work because the menuitem with the keyboard
> shortcut is never generated until the menu is shown? Could we maybe fix this by
> adding a dummy entry in the menu with the keyboard shortcut?

If true, that's probably the reason for this bug as well:

https://bugzilla.mozilla.org/show_bug.cgi?id=328746

Comment 9

[Colin Barrett [:cbarrett]]

Real link to the above bug is:

https://bugzilla.mozilla.org/show_bug.cgi?id=429824

Comment 10

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Colin, it seems like it _might_ be related to that bug, but not sure.

I took a second and tested, but it didn't help. My next theory is that we're not looking to into 2nd level menupopups for keyboard shortcuts (unless they are viewed first?). Doesn't make a whole lot of sense but it's the next guess as to why it wouldn't even work with a dummy menu entry

I don't have any time right now to investigate, so if anybody wants to look into it, please do.

Comment 11

[Johnathan Nightingale [:johnath]]

Doesn't block, but would love to see a safe patch that made Colin happier.

Comment 12

[Sergio Castro]

Can't believe this hasn't been fixed. I stopped using Firefox a really long time ago and was hoping this had been fixed.

Comment 13

[M Spreij]

My workaround is to hit Cmd-J (opening the downloads/history/bookmarks window), then hit Cmd-Shift-N (now it works!), then Cmd-` or something (..whatever you've set that switches to the previous/next window), and Cmd-W. Only four combos! Still, having the one work would be nice.

I'm not sure why I hit Cmd-J instead of Cmd-N, maybe because it's usually smaller and less in-the-way, or it might have been faster that one time.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Note that this also causes the keyboard shortcut to fail if there are just no windows open on the currently selected Space, even if there are windows open on other Spaces....

Comment 15

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

Blair, please comment in the bug if you got anywhere on this. In the meantime I'll reassign it to Katie as she can work on it next week.

Comment 16

[Katie Broida [:ktbee]]

I haven't had the free time I had hoped I would to tackle this. I'm going to free it up for the next person.

Comment 17

[Tarun Raheja]

Hi,
Would love to work on this, kindly assign this to me.

Comment 19

[:Gijs (he/him)]

Normally keyboard shortcuts work by virtue of being sent to the window when it has focus. But in the hidden window case, there is no window to have focus.

Next, apparently, keyboard shortcuts in the hidden window on macOS work when the key being pressed is known by macOS in a menu for which the corresponding native widget has been constructed.

This (apparently?) happens automatically for toplevel menus, but not for submenus (and in any case the submenu is initially empty).

Just programmatically creating popupshowing events itself doesn't appear to be sufficient (though it's not clear to me why not...).

I experimented with an entry that we hide when the menu actually opens, and reshow when the menu hides. That sort of works, except then when we reshow it again as the menu hides, the native menu has now been constructed without this entry in it, and so the shortcut stops working if you show the history menu (without opening the "recently closed windows" submenu).

I considered adding a dummy entry just for the hidden window that was visible. That worked but looked odd.

In the end I figured out that a combination of these approaches (namely: dummy entry in the sub menu in the markup, and firing popupshowing on the parent menu) works without any visual changes, though to be quite honest I don't know exactly why.

Comment 20

[:Gijs (he/him)]

Attachment: Bug 492320 - allow cmd-shift-n to work when there are no open windows, r?jaws!,mstange!

Comment 21

[Markus Stange [:mstange]]

I wonder what the "ideal" solution to this would look like. We need the native menu system to know about the menu item in the submenu. And we need to tell it about this menu item eagerly: I don't think macOS gives us an event to say "the user pressed a keyboard combination while no windows were open, make sure your menus are up to date before I process this keyboard shortcut". So we need to make sure the native menu system knows about this menu item at the point when the last window is closed. In other words, the part of this patch that triggers a menu update on window close will probably be necessary even if the rest of the system works as expected.

Have you tried sending two popupshowing events? The first directed at the History menu, and the second directed at the Recently closed windows submenu.

Anyway, I think what we want is:

• A way to tell the native menu system about updated submenu items eagerly (and have those updated items be respected for keyboard shortcuts), and
• maybe a way to run the front-end code that updates the menus without dispatching a synthetic popupshowing event.

Comment 22

[:Gijs (he/him)]

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

Have you tried sending two popupshowing events? The first directed at the History menu, and the second directed at the Recently closed windows submenu.

I did try this before, and I just tried it again. It doesn't work. I don't understand why, but if I had to guess, macOS doesn't check for shortcuts after the "fake" popupshowing events, so the trick is having the item with shortcut ready for the first popupshowing event? It's hard to be sure...

Anyway, I think what we want is:

• A way to tell the native menu system about updated submenu items eagerly (and have those updated items be respected for keyboard shortcuts), and
• maybe a way to run the front-end code that updates the menus without dispatching a synthetic popupshowing event.

I'll file these.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a8f14cfa665
allow cmd-shift-n to work when there are no open windows, r=jaws,mstange

Comment 24

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/4a8f14cfa665

Comment 25

[Cristian Fogel [:cfogel]]

Verified with 82.0b2 + no more beeps when respecting the note on step 2.