After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 112560 - keybindings have to be fully released before the grab ends
keybindings have to be fully released before the grab ends
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
2.4.x
Other Linux
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
: 162191 172342 321535 336878 469688 (view as bug list)
Depends on:
Blocks: 155457
 
 
Reported: 2003-05-08 09:19 UTC by erik
Modified: 2007-12-09 22:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make meta_display_process_key_event call itself with keystrokes which end grabs (736 bytes, patch)
2006-03-09 03:04 UTC, Thomas Thurman
committed Details | Review

Description erik 2003-05-08 09:19:19 UTC
Hi

I have ctrl-alt-arrowkeys bound to switch desktop and I have alt-up bound
to raise windows and alt-down to lower window.

The problem is that if switch desktop and then only release ctrl (and not
alt) I expect down and up to raise and lower the window, instead I'm
"stuck" in switch desktop mode and I'm still switching desktop when I do
alt-up/down.

I have to let go of BOTH ctrl and alt and then press alt again to be able
to use the alt-up/down keybinding again.

Hope that was clear?

/Erik
Comment 1 Havoc Pennington 2003-09-25 03:59:07 UTC
I guess to fix this we need to make the keybinding end when any of the
involved modifiers is released, not just the "primary modifier"; 
should be a fairly simple keybindings.c change. However have to be
careful about the Alt+Shift+Tab case where releasing shift doesn't end
the grab.
Comment 2 Elijah Newren 2004-12-28 19:13:21 UTC
*** Bug 162191 has been marked as a duplicate of this bug. ***
Comment 3 Elijah Newren 2004-12-28 19:13:37 UTC
Also be sure to handle the specific case mentioned in bug 162191: using the two
"windows keys" to the right of the right alt key to switch workspaces.
Comment 4 Elijah Newren 2005-04-01 11:06:48 UTC
*** Bug 172342 has been marked as a duplicate of this bug. ***
Comment 5 Elijah Newren 2005-10-07 16:08:15 UTC
Remove old target milestones on old bugs; sorry for the spam.
Comment 6 Olav Vitters 2005-11-15 16:34:34 UTC
*** Bug 321535 has been marked as a duplicate of this bug. ***
Comment 7 Thomas Thurman 2006-03-09 03:04:21 UTC
Created attachment 60943 [details] [review]
Make meta_display_process_key_event call itself with keystrokes which end grabs

This can be quite neatly solved by having meta_display_process_key_event call itself when a keystroke ends a grab, so that the keystroke can be processed again in its own right. Because this only happens when display->grab_op == META_GRAB_OP_NONE (since the grab has just ended), there's no risk of infinite recursion.
Comment 8 Elijah Newren 2006-03-25 00:36:46 UTC
Very clever.  :)

Doesn't seem to work for following up Alt+Tab with something else, though.

Also, does this cause us to release a grab and then trying to obtain a new one -- if so, does that cause a race condition?  I'm wondering if we should try to just modify the grab op type instead (though it may be a lot of ugly code), similar to process_keyboard_resize_grab_op_change().  But I don't really know; I may need to get Havoc or Soeren to comment.
Comment 9 Havoc Pennington 2006-03-30 06:17:44 UTC
There is a race where the new grab could fail, but in practice it should not ever matter afaik. When it does happen, I believe we properly check whether the grab succeeds so we'd just ignore the keystroke and let whoever grabbed it have it.

A perhaps-clearer way to write this code would be to check at the top of the key event handler whether to cancel the current grab, and if so cancel that existing grab and continue processing the new keystroke. I don't think an approach like that would be functionally different (would it?), it's probably a larger patch, but probably also more what someone reading the code would expect.

In the end it might not matter too much. I'd put a comment at the top of process_key_event saying "we may be called recursively" or something though...
Comment 10 Elijah Newren 2006-04-02 00:10:45 UTC
*** Bug 336878 has been marked as a duplicate of this bug. ***
Comment 11 Marius Gedminas 2007-02-17 22:13:24 UTC
The patch applies cleanly to metacity 2.16.3 (and to 2.17.5, but some missing build-dependencies prevented me from testing 2.17).

It fixes the irritation I've always had with metacity: that I couldn't switch to the next workspace and open a gnome-terminal there without releasing the modifier key in between.

By the way, Alt+Tab followed by Alt+F1 (without releasing Alt) correctly opens the GNOME main menu for me.  I wonder what Elijah tried.

Any chances for getting it committed?
Comment 12 Elijah Newren 2007-04-03 17:59:17 UTC
Hmm, alt+tab followed by alt+f1 does work for me.  

Ctrl-alt-right, then ctrl-alt-left, then ctrl-alt-shift-right does not work.  (i.e. moving workspaces, then trying to later move workspaces *and* move a window with you does not work).

ctrl-alt-right, then ctrl-alt-left, then alt-tab, then ctrl-alt-shift-right *twice* does work (I guess the first ctrl-alt-shift-right just cancels the alt-tab operation?)

So the patch isn't perfect, but it is short and sweet and better than what we have now.  Thomas, could you commit after adding the "may be called recursively" comment Havoc requested and add a FIXME referring to comments 8 and 9 of this bug about possible future improvements?  (I'll mark as accepted-commit_now, but the assumption is you'll add these comments to the code first)
Comment 13 Elijah Newren 2007-08-24 00:32:05 UTC
*** Bug 469688 has been marked as a duplicate of this bug. ***
Comment 14 Thomas Thurman 2007-12-09 22:59:32 UTC
Good grief, I suck for losing this one off the list. Committed.