GNOME Bugzilla – Bug 141425
While in Alt-Tab mode, Alt-Esc doesn't cancel
Last modified: 2006-02-13 19:26:27 UTC
$ metacity --version metacity 2.8.0 (on debian unstable, this is gnome from cvs, compiled with jhbuild a week or two ago) When I press and hold Alt, then press Tab a few times, then decide I want to return to the previously active application and press esc (still holding alt), it doesn't work. For some reason, metacity cycles through the windows when pressing esc.
That's just because Alt-Esc is metacity's instant-window-cycle shortcut, which immediately switches windows. To change this, remove the keybinding for "Move between window immediately" in the Keyboard Shortcuts applet.
hm, but should that keybinding also be active while in alt-tab mode?
*** Bug 309483 has been marked as a duplicate of this bug. ***
I wonder why this is NOTABUG. If the purpose is to implement an MS Windows style program switching behaviour, IMO *both* uses of alt+esc should be available for the user. In other words, the meaning of alt+esc should depend on whether the user is in alt+tab mode. Note: in alt+tab mode, the user is holding the alt key down. I presume that pressing esc there doesn't make the user think he/she is pushing the alt+esc combination. That's why I don't think this would confuse the user. Is this a technical restriction (hard to override a global alt+esc binding) or a non-technical discussion over what would be best for the user? See also: http://bugzilla.gnome.org/show_bug.cgi?id=309484
I think hitting Escape with Alt+tab down should cancel the alt+tab (though stricly speaking the purpose is not to implement MS Windows behavior, having Escape cancel seems useful.)
Created attachment 58690 [details] [review] Make alt-Esc cancel alt-Tabs, and vice versa This can be done by returning FALSE from process_tab_grab in keybindings.c when we see that we've received an alt-Esc keystroke, but the current cycling was started with an alt-Tab. This is further a good thing because there's a FIXME in process_tab_grab saying that being able to continue with alt-Esc when you started with alt-Tab, or vice-versa, is weird. It's easy for process_tab_grab to know it's received an alt-Esc, but it's harder for it to know whether the current cycling was started with an alt-Tab. This information is hidden away in the MetaTabPopup struct, which only tabpopup.c knows about. So what we need is to add a function in tabpopup.c that can tell us whether there's a visible tab window.
(In reply to comment #6) > but it's harder for it to know whether the current cycling was started with > an alt-Tab. Actually, you can get it from display->grab_op. If that equals META_GRAB_OP_KEYBOARD_TABBING_NORMAL or META_GRAB_OP_KEYBOARD_TABBING_DOCK then it was started with an alt-tab. s/TABBING/ESCAPING/ for alt-Esc. So, the tabpopup.[ch] part of the patch is unnecessary. Also, there's another problem that you may not have noticed. It's due to the fact that Alt-esc immediately raises the potential window and also unminimizes it if it was minimized, but lowers and re-minimizes the window if the user continues on and selects a different window. So, if the user tries to cancel the operation and it was an alt-esc one, you need to undo any raising and unminimizing that happened (look near the end of process_tab_grab() to find some code that does this). Also, although comparing to popup_not_showing works, it makes the code a little more confusing because popup_not_showing is just a side-effect of what you really want to test against. Because of this and the fix needed for re-lowering & re-minimizing, I'd just add another switch/case statement above the current one where popup_not_showing and those other vars are determined.
Created attachment 59275 [details] [review] Make alt-Escape cancel alt-Tab, and vice versa Here's a second try. I put the ChangeLog entry in the patch: is this a good idea?
The patch looks good. For future reference, please try to remove sections like @@ -2261,7 +2261,7 @@ process_tab_grab (MetaDisplay *display, gboolean popup_not_showing; gboolean backward; gboolean key_used; - + if (screen != display->grab_screen) return FALSE; from the patches by fixing the number of spaces on the line to be the original number. These kind of things just make the patch longer and thus harder to follow. >I put the ChangeLog entry in the patch: is this a good idea? This varies a little bit from project to project so you may find a different rule elsewhere. Most of them that I'm aware of, though, don't like it as part of the patch because if anyone commits any other change your patch will no longer apply. In this particular example, your patch happened to fail because there have been 4 other changes committed that weren't in your ChangeLog when you made the patch. Anyway, this can manually be fixed up without too much hassle, but it's typically easier to just have the ChangeLog entry be separate. Anyway, those are both small things so I just fixed it up and committed it so that it can make it into the release today and become part of Gnome 2.13.91. Thanks!
Thanks :) I'll post it in the Bugzilla comments from now on, then.
No need to change the patch status, though; it's still committed. ;-)