GNOME Bugzilla – Bug 641253
panel keynav between Activities and menus is quirky
Last modified: 2011-07-14 15:05:32 UTC
Navigating between the panel menus and the activities button is slightly quirky, and actually has three different behaviors, depending on whether the menus are closed, open-but-not-navigated-into (eg, hit space/enter on a menu title, but don't select a menu item), or open-with-an-active-menu-item. In particular, if the Quit menu item on the app menu is selected, then typing "Left" will bring you to the user menu, not the Activities button. Menus need to have their own special keynav handling because the menus themselves are not siblings of anything else in the panel, so various magic is needed to make that work right. But currently, that magic makes navigating to the Activities button NOT work right in certain cases. One easy fix might be to make the Activities button be a PanelMenuButton, although making it not have an associated menu might complicate things. Another possibility is to use captured-event to subvert the specific broken case. A third possibility is to declare that we don't actually want Left and Right to navigate between already-popped-up menus. That would solve the volume slider bug as well, and the impending "it takes multiple Lefts/Rights to get all the way past the calendar menu" bug.
Created attachment 180419 [details] [review] popupMenu, panelMenu: split up panel and non-panel keynav PopupMenuManager was pretending that it knew nothing about the menu's sourceActors, while also trying to handle keynav between them. This was a big mess, and resulted in bugs in navigation between panel menus and the Activities button, and it totally gets in the way when trying to add keynav to the dash (whose menu sources are arranged vertically rather than horizontally). Fix this up by moving the panel-specific parts to PanelMenuButton instead.
Created attachment 180948 [details] [review] dateMenu: make the menu un-key-navigable It already doesn't work right, because the PanelMenuButton code assumes that Left and Right won't be used as part of keynav within a menu. And the gnome-panel calendar isn't keyboard accessible either, so this isn't a regression. To be fixed later.
Created attachment 181832 [details] [review] popupMenu, panelMenu: split up panel and non-panel keynav rebased, and with some additional rearrangement of key processing (particularly, where Escape gets handled, so that we're not fighting with the overview's stage key-press-event handler)
Created attachment 182436 [details] [review] popupMenu, panelMenu: split up panel and non-panel keynav Update to apply to master.
Review of attachment 180948 [details] [review]: ::: js/ui/dateMenu.js @@ +90,3 @@ item = new PopupMenu.PopupMenuItem(_("Date and Time Settings")); item.connect('activate', Lang.bind(this, this._onPreferencesActivate)); + item.actor.can_focus = false; It would be better to add can_focus or canFocus as one of params to PopupMenuItem and PopupBaseMenuItem. (Not sure about proper name format because one of the params is style_class .) Have it have a default value true, but then be set as params.reactive && params.can_focus when GenericContainer is initialized.
Review of attachment 182436 [details] [review]: The code looks good to me. Behavior things I noticed: 1) If I click on one of the menu items and then press Left arrow key to move towards Activities button, the focus transfers back to the previously focused window and does not move to the Activities button. If I have one of the menu items focused after clicking Ctrl-Alt-Tab, the focus moves to the Activities button as expected when I press Left arrow key. 2) If I click on a menu item in the panel, and then click on it again to remove the menu or press Esc, the highlight for either the Activities button or the user menu flickers. 3) Esc doesn't take me out of the mode when the panel is in focus after clicking Ctrl-Alt-Tab. Perhaps this is the intended behavior, but just wanted to double check. ::: data/theme/gnome-shell.css @@ +261,1 @@ .panel-button:focus { There are two more places with this list of panel-button pseudo classes: .panel-button:active > .system-status-icon, .panel-button:checked > .system-status-icon, .panel-button:focus > .system-status-icon { icon-shadow: black 0px 2px 2px; } .panel-button:active > .system-status-icon, .panel-button:checked > .system-status-icon, .panel-button:focus > .system-status-icon { icon-shadow: black 0px 2px 2px; } Does the 'pressed' pseudo class not need to be included there?
*** Bug 643870 has been marked as a duplicate of this bug. ***
(In reply to comment #5) > It would be better to add can_focus or canFocus as one of params to > PopupMenuItem and PopupBaseMenuItem. Mmm... but this is *supposed* to be an ugly hack. There's no "normal" use case for having a menu item that is reactive but !can_focus. We're only doing it here to work around bugs somewhere else, so I don't think it makes sense to add infrastructure for it at the PopupMenuItem level. (In reply to comment #6) > 1) If I click on one of the menu items and then press Left arrow key to move > towards Activities button, the focus transfers back to the previously focused > window and does not move to the Activities button. If I have one of the menu > items focused after clicking Ctrl-Alt-Tab, the focus moves to the Activities > button as expected when I press Left arrow key. Yes. My only justification for that is "but why would you click on a menu and then use the arrow keys"? (The problem is that when you navigate off the edge, there is no longer a menu open, so PopupMenuManager does a popModal() and returns stage_input_mode to its pre-menu state. In the Ctrl-Alt-Tab case, that's FOCUSED, and so you navigate to the Activities button. But in the case where you activated the menu via the pointer, the previous stage_input_mode was NORMAL, and so the stage loses focus.) Of course... this also means that if you do this in the overview, then it *will* select Activities when you move left from the app menu... Bleah. So, I guess maybe I should make it work in NORMAL mode too. > 3) Esc doesn't take me out of the mode when the panel is in focus after > clicking Ctrl-Alt-Tab. Perhaps this is the intended behavior, but just wanted > to double check. It matches how Ctrl-Alt-Tab works in GNOME 2; if you want to get out via the keyboard, you can use Alt-Tab to get back to a normal window. But probably we do want to change this; panel keynav is really the only place where Esc *doesn't* escape, since it will close either the overview or the tray. > There are two more places with this list of panel-button pseudo classes: > Does the 'pressed' pseudo class not need to be included there? It does, they just weren't there when I wrote the patch :)
(In reply to comment #8) > (In reply to comment #5) > > It would be better to add can_focus or canFocus as one of params to > > PopupMenuItem and PopupBaseMenuItem. > > Mmm... but this is *supposed* to be an ugly hack. There's no "normal" use case > for having a menu item that is reactive but !can_focus. We're only doing it > here to work around bugs somewhere else, so I don't think it makes sense to add > infrastructure for it at the PopupMenuItem level. Ok, then you can commit it as is :). > > (In reply to comment #6) > > 1) If I click on one of the menu items and then press Left arrow key to move > > towards Activities button, the focus transfers back to the previously focused > > window and does not move to the Activities button. If I have one of the menu > > items focused after clicking Ctrl-Alt-Tab, the focus moves to the Activities > > button as expected when I press Left arrow key. > > Yes. My only justification for that is "but why would you click on a menu and > then use the arrow keys"? I have no idea how common that is :). Seems like something that would be good to fix, but is not a blocker for committing this patch. > > (The problem is that when you navigate off the edge, there is no longer a menu > open, so PopupMenuManager does a popModal() and returns stage_input_mode to its > pre-menu state. In the Ctrl-Alt-Tab case, that's FOCUSED, and so you navigate > to the Activities button. But in the case where you activated the menu via the > pointer, the previous stage_input_mode was NORMAL, and so the stage loses > focus.) > > Of course... this also means that if you do this in the overview, then it > *will* select Activities when you move left from the app menu... Bleah. So, I > guess maybe I should make it work in NORMAL mode too. In the overview, it would select Activities, but then pressing Right arrow would do nothing. Maybe the solution is to not let navigate off the edge? The way it doesn't when you move all the way right to the user menu. > > > 3) Esc doesn't take me out of the mode when the panel is in focus after > > clicking Ctrl-Alt-Tab. Perhaps this is the intended behavior, but just wanted > > to double check. > > It matches how Ctrl-Alt-Tab works in GNOME 2; if you want to get out via the > keyboard, you can use Alt-Tab to get back to a normal window. But probably we > do want to change this; panel keynav is really the only place where Esc > *doesn't* escape, since it will close either the overview or the tray. Makes sense. Again, not a blocker for committing this patch. What about: 2) If I click on a menu item in the panel, and then click on it again to remove the menu or press Esc, the highlight for either the Activities button or the user menu flickers.
(In reply to comment #9) > What about: > > 2) If I click on a menu item in the panel, and then click on it again to remove > the menu or press Esc, the highlight for either the Activities button or the > user menu flickers. I didn't comment on it before, because it was obviously a bug and obviously needed to be fixed. Except that now it turns out I can't reproduce that at all.
(In reply to comment #9) > What about: > > 2) If I click on a menu item in the panel, and then click on it again to remove > the menu or press Esc, the highlight for either the Activities button or the > user menu flickers. filed as bug 644124; it's there with or without this patch. I poked around at trying to address the remaining oddities you pointed out, but ran into some problems. I'm going to push the current patches as is (since nearly every other patch I have in bugzilla right now is blocking on them :), but I'll leave the bug open.
Attachment 180948 [details] pushed as df848fd - dateMenu: make the menu un-key-navigable Attachment 182436 [details] pushed as ef6cce8 - popupMenu, panelMenu: split up panel and non-panel keynav
duping this to the other "Activities button is weird" bug, since the patch there fixes both problems *** This bug has been marked as a duplicate of bug 645759 ***