GNOME Bugzilla – Bug 687024
x11: fix return value of gdk_keyval_from_name
Last modified: 2013-04-29 04:19:49 UTC
According to te doc, gdk_keyval_from_keyval shall return GDK_KEY_VoidSymbol when the key name is invalid. However, it returns 0 (NoSymbol) instead on X11. $ python >>> from gi.repository import Gdk >>> Gdk.keyval_from_name("foobarbaz") 0L >>> Gdk.KEY_VoidSymbol 16777215 Other backends seem to return VoidSymbol properly.
Created attachment 227438 [details] [review] x11: fix return value of gdk_keyval_from_name Cast the return value of XStringToKeysym to VoidSymbol if it is NoSymbol.
This will require some checking of callers to make sure we're not breaking code which has be adapted to the actual return value of 0
Just out of curiousity, what was the reason to use VoidSymbol rather than NoSymbol here?
No idea
Created attachment 227696 [details] [review] gdk: fix return value of gdk_keyval_from_name For all backends, return 0 (NoSymbol) instead of GDK_KEY_VoidSymbol if the key name is invalid (previously only x11 backend returned 0). Adjust the doc as well. -- So, maybe safer to fix other backends and the doc so it returns NoSymbol. The x11 code has been there since gtk+ 1.x and I have actually seen checks like "keyval != 0" in several places. Even though GDK_KEY_VoidSymbol might be useful in some cases (since it is a valid keysym), callers should check the return value.
grepping for VoidSymbol in gtk/ brings up several places where we do check for it, so I don't think we can get away with changing the semantics of this function
Comment on attachment 227696 [details] [review] gdk: fix return value of gdk_keyval_from_name Okay, so fwiw I grepped for "\<(Gdk\.|gdk_)keyval_from_name\>" in freshly checked out gnome-apps-3.8 moduleset and it seems the affected modules are: * spice-gtk spice-gtk-0.14/gtk/spice-grabsequence.c:88 * orca orca/src/orca/keybindings.py:76 * ibus ibus-1.4.99.20120822/setup/keyboardshortcut.py:179 * accerciser accerciser/macaroon/macaroon/record/main.py:440 Other modules either check with VoidSymbol or don't have any check. Probably we should also go through NoSymbol / VoidSymbol usage in gtk+/gdk/x11/*.c
I've already made up my mind: we can't change the semantics. We should fix all backends to consistently return VoidSymbol, as documented.
I'm not pushing the semantics change (read comment 7 again). I marked the 2nd patch as "rejected" and provided the list of modules that need fix to check keyval == VoidSymbol instead of keyval == 0. I think other backends return VoidSymbol already (through gdkkeynames.c:_gdk_keyval_from_name).
ah, ok
This is probably obsoleted by use stripping backends from keyval translation