GNOME Bugzilla – Bug 683534
temporary shift-mode stucks - does not switch back to normal after releasing shift-button
Last modified: 2012-10-02 21:01:17 UTC
Using Ubuntu 12.04 LTS, Meld 1.5.3 Keyboard layout: German, in special mode/variant: NEO 2.0 when pushing one of the shift keys on my lenovo usb keyboard the little arrow in the middle (two-file-compare-mode) change to crosses. But when I release the same shift button, the crosses (deleting mode) don't switch back to little arrows (replacing mode). They keep on, like a kind of sticky behaviour. Workaround: I have to open any arbitrary dialog-window inside Meld, e.g. Help > About. After closing the window, everything is fine back in original state (little arrows instead of crosses) Surprisingly this only happens with the shift button to me, the Control one works like it should. (pushed: add-mode, released: replace-mode). Seems to be a problem with keyboard layout NEO 2.0 in standard german layout, bug does not appear. Any help? How is this shortcut encoded in Meld?
Essentially, Meld listens for a key-up event for Ctrl, Shift, etc., but some layouts don't actually emit key-ups for some modifier keys. I don't know anything about the NEO 2.0 layout, so I don't know why it would fail where others work. This sounds exactly like an old problem with layout switching and modifiers (bug 584342). Could you please have a look at that bug, and see if any of the debugging there gives us extra information? It may be that we just need to add another key symbol to the existing workaround.
Sorry, but all of this long communication there did not help me. I am using three different PC (totally different hardware) and on all three there is Xubuntu 12.04 running and the neo 2.0 layout. see: http://www.neo-layout.org/ all these work around with "press ctlr, then..." and so on, no help for me. as I am not at all experienced with xkb settings and stuff... sorry, can't help a lot and I do not have the time to dive into these tecnical issues...
The easiest way to get the key event data is to run 'xev' from the command line. Ignore the initial output. Press and then release Shift, press and then release Ctrl. Then please copy xev's output for just those four events and and paste it here. (If you look at the bug I linked, comment 17 has an example of the kind of output I need.)
KeyPress event, serial 34, synthetic NO, window 0x2600001, root 0xc5, subw 0x0, time 908497, (255,-185), root:(966,343), state 0x0, keycode 50 (keysym 0xffe1, Shift_L), same_screen YES, XLookupString gives 0 bytes: XmbLookupString gives 0 bytes: XFilterEvent returns: False KeyRelease event, serial 34, synthetic NO, window 0x2600001, root 0xc5, subw 0x0, time 909713, (255,-185), root:(966,343), state 0x1, keycode 50 (keysym 0xffe5, Caps_Lock), same_screen YES, XLookupString gives 0 bytes: XFilterEvent returns: False KeyPress event, serial 34, synthetic NO, window 0x2600001, root 0xc5, subw 0x0, time 922425, (255,-185), root:(966,343), state 0x0, keycode 37 (keysym 0xffe3, Control_L), same_screen YES, XLookupString gives 0 bytes: XmbLookupString gives 0 bytes: XFilterEvent returns: False KeyRelease event, serial 34, synthetic NO, window 0x2600001, root 0xc5, subw 0x0, time 923201, (255,-185), root:(966,343), state 0x4, keycode 37 (keysym 0xffe3, Control_L), same_screen YES, XLookupString gives 0 bytes: XFilterEvent returns: False
Thanks. This looks to me like a bug with the neo layout. The problem is that when you push down Shift, it sends KeyPress Shift, but when you release Shift it sends KeyRelease Caps_Lock. Looking at the Neo bugs it looks (to my very broken German) like the layout already has bugs reported against it for its Shift/Capslock behaviour. e.g., http://wiki.neo-layout.org/ticket/243 Given that, I'm going to close this bug as being not Meld. If you want a workaround, you should be able to change on_key_release_event in filediff.py. The last clause there currently looks like: elif event.keyval == gtk.keysyms.ISO_Prev_Group: If you change it to: elif event.keyval == gtk.keysyms.ISO_Prev_Group or event.keyval == gtk.keysyms.Caps_Lock: then I suspect that that should work, though I haven't tested this. I really can't justify adding this workaround to Meld though, as this looks like a straight-up bug with the layout.
Thanks so much Kai! The workaround works perfect, just at first glance it didn't because there is a linebreak in your "change it to" hint. This linebreak copied in filediff.py doesn't even allow meld to start anymore. After fiddling around a bit I realized that the linebreak could be obsolete. and yep, I am a long time user of linux/ubuntu, still don't know to much about the infrastructure behind it... so it was quite a big thing to locate filediff.py on my harddisk. But finally I found it to be inside: (Xubunutu 12.04) /usr/lib/meld/meld/ yes, you can close it here and I am going to open one on the neo-layout site. Thanks again. Very helpful.
The xev output above does indeed seem suspicious at first glance, but actually it is wrong to listen for keypress and keyrelease events based on the keysym. Consider the following keypresses on a standard german layout: Shift-press -> "1"-press -> Shift-release -> "1" release you get the following output by xev: KeyPress event, serial 32, synthetic NO, window 0x3c00001, root 0x15e, subw 0x0, time 57938171, (-926,358), root:(697,498), state 0x0, keycode 62 (keysym 0xffe2, Shift_R), same_screen YES, XLookupString gives 0 bytes: XmbLookupString gives 0 bytes: XFilterEvent returns: False KeyPress event, serial 32, synthetic NO, window 0x3c00001, root 0x15e, subw 0x0, time 57938379, (-926,358), root:(697,498), state 0x1, keycode 10 (keysym 0x21, exclam), same_screen YES, XLookupString gives 1 bytes: (21) "!" XmbLookupString gives 1 bytes: (21) "!" XFilterEvent returns: False KeyRelease event, serial 32, synthetic NO, window 0x3c00001, root 0x15e, subw 0x0, time 57938523, (-926,358), root:(697,498), state 0x1, keycode 62 (keysym 0xffe2, Shift_R), same_screen YES, XLookupString gives 0 bytes: XFilterEvent returns: False KeyRelease event, serial 32, synthetic NO, window 0x3c00001, root 0x15e, subw 0x0, time 57938803, (-926,358), root:(697,498), state 0x0, keycode 10 (keysym 0x31, 1), same_screen YES, XLookupString gives 1 bytes: (31) "1" XFilterEvent returns: False You see the press-release events of the key with keycode 10 are different and do not have the same keysym associated ("!" on press and "1" on release). Keypress and keyrelease events are bound to a keycode, not to a keysym! Checking for the keysym in such an event is not the right thing to do, because the keysym only ever has a meaning when the keypress event is fired. The reason why this hasn't occured with any other keyboard layout is because neo might be the only one which assigns different keysyms for the shift-key on different levels. If you want to fetch the shift-state, consider checking the xkb modifiers (as does the diagnostic tool "xkbwatch").
I am aware of the brokenness of assuming that KeyPress and KeyRelease events match (and in fact that a release event even happens). However, the existing code works for the vast majority of cases. Also, if I remember correctly, trusting the hardware keycode breaks a different set of use cases. As for using the modifiers, monitoring press/release events of the modifers themselves doesn't work that way with GTK/GDK. When the user presses Shift, the event comes through saying that Shift was pressed, and the modifier state is empty. When they release, the event says that Shift was released and the modifier state is <Shift>. This is obviously not what we want. (Note that this *is* what we do when checking for the actual actions, because modifier checking for e.g., <Shift>+Click works just fine.) Anyway, if someone wants to work on this, then that's great. However, any solution needs to not break anything cross-platform, and must work within the bounds of GTK/GDK.
Created attachment 225227 [details] [review] Proposed fix Please check/test if this patch satisfies your requirements. I have sucessfully tested it on linux with the german neo2 keyboard layout.
Yeah, sure, that looks good to me. I'll have to test it on Windows/OSX as well however. Did you actually test this with the ISO_Prev_Group configuration? not a problem if not, I'll just have to recreate it before committing. For my benefit, could you give me some idea of when this method might be expected to break? I can understand why it works, but I'm not sure that I appreciate when it *won't* work. And thanks, this is really helpful.
Created attachment 225379 [details] [review] Proposed patch using translate_keyboard_state instead of lookup_key I've changed the patch to use translate_keyboard_state as lookup_key seems to return wrong values in some multilayout configurations. Instead of explicitly specifying the first level, it uses the state parameter 0 to specify the default level (in all xkb-types currently equivalent to the first level) to look for the keysym. I've successfully tested it with the ISO_Prev_Group/ISO_Next_Group configuration. It will probably fail if a keyboard layout does not use the Shift keysym on the first level. Considering that one should be able to use modifier keys standalone (that is without first switching to a different level than the default one) this will always be the case. Not doing so is possible but would defy the meaning of a modifier key. The "right way" to do this would probably be to use the gdk_keymap_get_modifier_state method which is available since gdk3: http://developer.gnome.org/gdk3/stable/gdk3-Keyboard-Handling.html#gdk-keymap-get-modifier-state PyGTK doesn't support this though.
I've made a minor Python formatting fix, added the bug number, and pushed the commit. Thanks very much for this. I very much appreciate you chasing down a better solution than the known-broken one we had. When Meld finally moves to GTK 3 and associated infrastructure, we can update to use the GDK modifier state handling. For now, I've commented it.