Closed Bug 492320 Opened 16 years ago Closed 4 years ago

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

Categories

(Firefox :: Keyboard Navigation, defect, P5)

x86
macOS
defect
Points:
5

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
blocking2.0 --- -
firefox82 --- verified

People

(Reporter: jruderman, Assigned: Gijs, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qx:spec][outreachy-12])

Attachments

(1 file)

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
Flags: blocking-firefox3.5?
Does this also happen if there are arbitrary open/close tab or window steps between 2 and 3? Trying to gauge effect.
Assignee: nobody → paul
(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)
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.
Flags: wanted-firefox3.5+
Flags: blocking-firefox3.5?
Flags: blocking-firefox3.5-
I hit this probably once a day. It's a really frustrating paper cut. Please? :)
blocking2.0: --- → ?
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?
This bug essentially means that the keyboard shortcut for "Undo close window" does not work for people who only keep one window open.
(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
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.
Assignee: paul → nobody
Doesn't block, but would love to see a safe patch that made Colin happier.
blocking2.0: ? → -
Can't believe this hasn't been fixed. I stopped using Firefox a really long time ago and was hoping this had been fixed.
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.
Whiteboard: [qx] → [qx:spec]
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....
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
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.
Assignee: bmcbride → kbroida
Whiteboard: [qx:spec] → [qx:spec][outreachy-12]
Points: --- → 5
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.
Assignee: kbroida → nobody
Status: ASSIGNED → NEW
Hi, Would love to work on this, kindly assign this to me.

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.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

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.

(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.

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4a8f14cfa665 allow cmd-shift-n to work when there are no open windows, r=jaws,mstange
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
Flags: qe-verify+

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

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: