GNOME Bugzilla – Bug 144808
AltGr combinations don't work with some X servers
Last modified: 2004-12-22 21:47:04 UTC
The problem occurs with some X servers like Hummingbird, ReflectionX and X-Win32. The X server is connected to a Linux server running GDM via XDMCP. When the server is configured to use the Swiss German keyboard layout, (or the Swiss French, or the France French for the matter), it is impossible to enter any of the Mode_Switched characters within GTK based applications. Traditional applications such as XTerm, Emacs operate correctly. A simple test is that entering AltGr+7 should output a pipe '|'. When gnome-terminal is run with XFree86 this works, but with ReflectionX it fails. After thorough investigation this problem has been tracked down to a bug in the GDK keyboard translation code. This assumes that the value of keysyms_per_keycode returned by the XGetKeyboardMapping method is an even number. On ReflectionX with SwissGerman keyboard layout the value is 3, thus breaking the code which does lookups into the keycode -> keysymbol map when Mode_Switch (AltGr) is pressed. Possible patch from Daniel Berrange <berrange@redhat.com> attached below.
Created attachment 28928 [details] [review] gtk+-2.2.4-reflectionx.patch I'm not sure if the GTK+ coding style is followed.
Re coding style: the C++ comment will have to be replaced by /* */. I wonder if it wouldn't be better to remove the wrong assumptions on keysyms_per_keycode from the code, considering that man XGetKeyboardMapping says "The X server arbitrarily chooses the keysyms_per_keycode_return value to be large enough to report all requested symbols. A special KeySym value of NoSymbol is used to fill in unused elements for individual KeyCodes."
Looking through the file, it seems we should replace syms[KEYSYM_INDEX (k,g,l)] by a function get_symbol(k,g,l) which would check whether the group,level combination falls outside of keysyms_per_keycode and returns NoSymbol in that case.
Created attachment 28933 [details] [review] gdk-keysym-nosymbol.patch A possible patch implementing the advices from Matthias. Needs testing.
There is one misplaced '{' in the new function, btw.
I don't believe that patch will work becuase it merely removes the potential out-of-bounds array indexing when Mode_Switch+Shift combination is active. It is still using the KEYSYM_INDEX macro & thus performs integer arithmetic which only works correctly when keysyms_per_keycode is even: The KEYSYM_INDEX macro is defined as #define KEYSYM_INDEX(keymap_impl, group, level) \ (2 * ((group) % (keymap_impl->keysyms_per_keycode / 2)) + (level)) Now keysyms_per_keycode has value 3 on ReflectionX & the group/level we are looking up is Mode_Switch + non-Shifted. So on ReflectionX this corresponds to (2 * ((1) % (3 / 2)) + (0)) Which evaluates to '0', where as on XFree86 this corresponds to (2 * ((1) % (4 / 2)) + (0)) Which evaluates to '2'. It might work if the KEYSYM_INDEX macro were to round keysyms_per_keycode upto the next even number, but you'd have to check rest to key handling code to ensure there wasn't any other simiarly dodgy integer arithmetic.
Good catch, it should probably be (keysyms_per_keycode + 1) / 2. I believe all other uses of keysyms_per_keycode don't involve arithmetic, but are just for iterating about all keysyms.
Created attachment 28954 [details] [review] gdk-keysym-nosymbol2.patch Updated patch.
I've tested this patch, and it fixes AltGr combinations with X-Win32 (which had the same problem as ReflectionX before).
Hmm, that patch misses the parentheses from comment #7. It doesn't actually seem to matter for the values used in the code (due to the (group) % ... part), but it should probably be corrected...
Created attachment 28955 [details] [review] gdk-keysym-nosymbol3.patch Correct the parenthesis. OK to commit then?
It would be good to add a comment explaining the + 1, but apart from that, it looks good.
Created attachment 28981 [details] [review] gdk-keysym-nosymbol4.patch With a little comment above...
Committed to HEAD and gtk-2-4 branch. 2004-06-25 Bastien Nocera <hadess@hadess.net> reviewed by: Matthias Clasen <maclas@gmx.de> * gdk/x11/gdkkeys-x11.c: (get_symbol), (update_keymaps), (gdk_keymap_lookup_key), (translate_keysym): fix keys parsing when the number of keysyms per keycode is odd. Fixes #144808.