GNOME Bugzilla – Bug 736125
The default gtk accelerator mod mask does not include all default backend modifiers
Last modified: 2015-08-30 14:46:25 UTC
On quartz, the primary modifier (i.e. the command key) is mapped to the MOD2 modifier key. Additionally, GDK_MODIFIER_INTENT_MODIFY_SELECTION is mapped to the primary key as well. However, MOD2 is not included (by default) in the default gtk accelerator mod mask. This leads to surprising behavior in various widgets, because keybindings are installed for the modmask corresponding to GDK_MODIFIER_INTENT_MODIFY_SELECTION. However, since these are then filtered through the default gtk accel mod mask, you end up installing certain keybindings twice (with an empty modmask). The resulting behavior is then that for example for GtkListBox, pressing the down arrow skips a row (since the keybinding is activated twice). I'm unsure what exactly the correct fix would be. If someone can point in the right direction I can create the corresponding patch.
does adding MOD2 to the default mask make things work as expected ?
This does indeed work as expected, and previously observable problem goes away. I do get both MOD2 and META in the modmask for any keypress where the command key is involved. Is that normal, or could that be an issue?
you get a modifier mask that includes both the physical (mod2-mod5) and virtual (meta,super,...) modifiers. GTK+ is supposed to deal with it when matching shortcuts against key events and when displaying shortcuts - does that work ?
Yes, it works. I was wondering though if just adding MOD2 to the default mask is really desired for other platforms. The default mask should probably query gdk to complete the mask to put this into the hand of specific backends.
Any comments on this?
adding MOD2 to the default mask is not desirable under X indeed - MOD2 is typically mapped to NumLock, which is notoriously unreliable, and best masked away.
I think getting the default accel mod mask from the backend is the only thing that makes sense. I didn't think of that when mapping Command to MOD2, and it would most likely fix a few obscure bugs that happen in GIMP on OS X.
What about simply adding GDK_MODIFIER_INTENT_DEFAULT_ACCEL_MODS (or a better name) and returning the right mask from gdk_keymap_get_modifier_mask().
seems ok to me
jessevdk, were you going to write a patch for this ?
Created attachment 293330 [details] [review] Implement GDK_MODIFIER_INTENT_DEFAULT_MOD_MASK Here's an implementation, but there's a wee problem: The default_accelerator_mod_mask is needed by gail initialization before the default GdkKeymap is created, at least on Quartz. Gail is the first thing that needs it, anyway: There are 129 pairs of errors emitted (no display, no keymap) during startup of gtk3-demo. Once started, selection with mod keys seems to work, so that part's not broken, but other things seem to be.
Created attachment 294133 [details] [review] Fix a startup ordering problem Defer a11y initialization until we have a display. A11y initialization causes widget classes to be initalized, which in turn needs some backend-specific information about modifier masks that can't be obtained before we have a display.
That fixes the error messages and the mod mask change still works. How do I test the a11y stuff to make sure that it hasn't been broken? For that matter, does it even work on OSX? Testsuite/ally bails because it apparently depends on X11 to work.
Comment on attachment 294133 [details] [review] Fix a startup ordering problem Attachment 294133 [details] pushed as a5e2256 - Fix a startup ordering problem
I've pushed these
Comment on attachment 293330 [details] [review] Implement GDK_MODIFIER_INTENT_DEFAULT_MOD_MASK Should it be backported to 2.24?
Attachment 294133 [details] introduces a build-time regression, in the form of requiring that you have an open display connection when you create a class. This breaks introspection and API generation on headless build servers, such as the ones that build GTK packages for distributions, or GNOME Continuous. It's not an entirely trivial thing to fix, though; GtkBindingSet uses the default mod mask as soon as a binding is created, which means that now it will try to get the default mod mask for the platform, which means it will involve the GDK backend, which requires an open display connection — and now we're off to the races. I have an idea on how to fix this in a non-terrible way, though…
Created attachment 310262 [details] [review] accelgroup: Add a fall back in case of no display Bug 736125 introduced code that added a requirement on GDK being initialized when initializing GTK-related classes, in order to get the default modifiers mask. GtkBindingSet will call into gtk_accelerator_get_default_mod_mask() when adding a new key binding to the set, which usually happens when a class is initialized. This means that anything that introspects GTK classes, like the gtk-doc or the introspection scanners will now either fail or start spewing critical warnings because nothing called gtk_init(); and if we start requiring gtk_init() to be called, then GTK won't build on headless machines any more. In order to avoid that, we cheat a bit, and see if there is a display connection available when asking for the default modifiers mask; if there is one (i.e. the user called gtk_init() on a machine with a display) then we go through the existing code paths, and all is well; whereas if that did not happen, we fall back to the old, default modifiers mask that we used until commit c55ff6e4.
This works on OSX, and since you've undoubtedly checked on X11, I think you can push.
Hey, sorry - I pushed essentially the same patch already yesterday. If you want to add your comment on top of my patch, please do.