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 754110 - inconsistent interpretation of preserved modifiers with xkbcommon
inconsistent interpretation of preserved modifiers with xkbcommon
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.17.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 757550 763161 772506 (view as bug list)
Depends on:
Blocks: wayland WaylandRelated
 
 
Reported: 2015-08-26 09:20 UTC by Daniel Stone
Modified: 2018-05-02 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: Test partial consumed modifier matches more thoroughly (4.13 KB, patch)
2015-12-07 13:56 UTC, Carlos Garnacho
needs-work Details | Review

Description Daniel Stone 2015-08-26 09:20:24 UTC
Using the Wayland backend for GTK+ clients, Ctrl-Backspace no longer functions as a word delete, but instead as a regular delete. Ctrl-Left/Right still work; it's just Ctrl-Backspace.

Tried in GEdit and GNOME's composer: pressing Ctrl-Backspace deletes character-at-a-time, not word-at-a-time. Running GEdit under X11 (unset $WAYLAND_DISPLAY) restores Ctrl-Backspace to its rightful functionality, and it also works under Chrome, which uses GTK+2 and thus X11.

gtk3-3.17.7-1.fc23.x86_64
Comment 1 Jonas Ådahl 2015-08-26 12:26:22 UTC
Hmm. I cannot reproduce this, neither running GTK+ clients under weston or GNOME Shell. I tried GEdit and gtk3-demo, and I don't see anything relevant in the commit log between 3.17.7 and where I am (which is a few commits after). Does it happen when you test under weston? What version of mutter are you using?
Comment 2 Daniel Stone 2015-08-26 12:40:36 UTC
Tested under Shell (3.17.90-1.fc23) and Weston (git from a couple of weeks ago). Modifiers seem right:
key key: 29, unicode: 65507, state: pressed, modifiers: 0x0
key key: 14, unicode: 65288, state: pressed, modifiers: 0x4
key key: 14, unicode: 65288, state: released, modifiers: 0x4
key key: 29, unicode: 65507, state: released, modifiers: 0x4
Comment 3 Matthias Clasen 2015-08-26 13:28:18 UTC
Not sure what GNOME's composer is, but it works fine in gedit here as well.
Comment 4 Daniel Stone 2015-08-26 13:30:45 UTC
Composer? If you mean a wl_text equivalent, then that shouldn't matter much as it's the same under plain Weston ...
Comment 5 Matthias Clasen 2015-08-27 01:50:47 UTC
I was referring ot your original report: "Tried in GEdit and GNOME's composer,,,"
Comment 6 Daniel Stone 2015-08-27 09:46:20 UTC
Oh, sorry. Evolution's composer.
Comment 7 Matthias Clasen 2015-08-31 14:46:02 UTC
Since we can't reproduce this, not much we can do unless you debug this on your machine.
Comment 8 Lyude 2015-11-13 20:20:56 UTC
(In reply to Matthias Clasen from comment #7)
> Since we can't reproduce this, not much we can do unless you debug this on
> your machine.

But luckily I can! The keyboard I use at home is a Nighthawk X7, and it likes to be strange by exposing three different input devices. Unfortunately, the input device that sends the backspace keystroke is different from the one that sends the modifier key, so I'm assuming that's probably where this issue is coming from.
Comment 9 Daniel Stone 2015-11-13 21:09:14 UTC
(In reply to Lyude from comment #8)
> But luckily I can! The keyboard I use at home is a Nighthawk X7, and it
> likes to be strange by exposing three different input devices.
> Unfortunately, the input device that sends the backspace keystroke is
> different from the one that sends the modifier key, so I'm assuming that's
> probably where this issue is coming from.

Nothing so exciting here, I'm afraid (and bear in mind that ctrl+arrows do still work!). I do wonder if it's related to having ctrl:nocaps enabled though, i.e. Caps Lock also acts as Ctrl.
Comment 10 Lyude 2015-11-13 21:24:16 UTC
(In reply to Daniel Stone from comment #9)
> (In reply to Lyude from comment #8)
> > But luckily I can! The keyboard I use at home is a Nighthawk X7, and it
> > likes to be strange by exposing three different input devices.
> > Unfortunately, the input device that sends the backspace keystroke is
> > different from the one that sends the modifier key, so I'm assuming that's
> > probably where this issue is coming from.
> 
> Nothing so exciting here, I'm afraid (and bear in mind that ctrl+arrows do
> still work!). I do wonder if it's related to having ctrl:nocaps enabled
> though, i.e. Caps Lock also acts as Ctrl.

I have caps lock set to the default configuration here and I'm still running into the issue
Comment 11 Matthias Clasen 2015-11-17 21:02:50 UTC
Could this be a case of xkb configuration declaring the modifier consumed ? This is happening for Ctrl-F1 on my system: the app never receives the Ctrl modifier...
Comment 12 Jean-François Fortin Tam 2015-11-24 19:33:20 UTC
*** Bug 757550 has been marked as a duplicate of this bug. ***
Comment 13 Jean-François Fortin Tam 2015-11-24 19:37:33 UTC
Strange that this is apparently not reproducible everywhere… Could this have anything to do with the keyboard layout (French Canadian Dvorak in my case)?
Comment 14 Matthias Clasen 2015-11-24 19:45:16 UTC
It would be most instructive if you could capture both the wayland-level and GTK+ level event streams, with 

WAYLAND_DEBUG=1

and

GDK_DEBUG=events

(probably best separately, to not mix them up)

and see if the Control-Backspace event has its Control modifier stripped at some point.
Comment 15 Daniel Stone 2015-11-24 20:02:30 UTC
mclasen: no, it hasn't been stripped (29 -> KEY_LEFTCTRL, 14 -> KEY_BACKSPACE):

[2939736.726] wl_keyboard@20.key(63036, 15873579, 29, 1)
Gdk-Message: keyboard event, code 37, sym 65507, string , mods 0x0
[2939737.320] wl_keyboard@20.modifiers(63037, 4, 0, 0, 0)
[2940539.334] wl_keyboard@20.key(63038, 15874382, 14, 1)
Gdk-Message: keyboard event, code 22, sym 65288, string \u0008, mods 0x4
[2940543.220]  -> wl_compositor@4.create_region(new id wl_region@43)
[2940543.454]  -> wl_region@43.add(7, 0, 1586, 7)
[2940543.746]  -> wl_region@43.add(0, 7, 1600, 865)
[2940544.030]  -> wl_surface@18.set_opaque_region(wl_region@43)
[2940544.130]  -> wl_region@43.destroy()
[2940544.276]  -> wl_compositor@4.create_region(new id wl_region@41)
[2940544.369]  -> wl_region@41.add(-10, -10, 1620, 892)
[2940544.485]  -> wl_surface@18.set_input_region(wl_region@41)
[2940544.522]  -> wl_region@41.destroy()
[2940546.944]  -> wl_surface@18.attach(wl_buffer@34, 0, 0)
[2940547.044]  -> wl_surface@18.set_buffer_scale(2)
[2940547.080]  -> wl_surface@18.damage(0, 107, 1600, 15)
[2940547.180]  -> wl_surface@18.damage(1370, 849, 144, 21)
[2940547.353]  -> wl_surface@18.frame(new id wl_callback@42)
[2940547.404]  -> wl_surface@18.commit()
[2940559.855] wl_display@1.delete_id(43)
[2940560.031] wl_display@1.delete_id(41)
[2940560.124] wl_display@1.delete_id(42)
[2940560.248] wl_callback@42.done(990783035)
Gdk-Message: frame 0x5622b6ef2000
[2940689.243] wl_keyboard@20.key(63039, 15874532, 14, 0)
Gdk-Message: keyboard event, code 22, sym 65288, string \u0008, mods 0x4


Taken from pressing Ctrl-Backspace in GEdit. I do have ctrl:nocaps enabled (i.e. CAPS also sends Control_L), but changing that doesn't seem to have any effect. Again, Ctrl+{Left,Right} work fine, which are also declared as GDK_CONTROL_MASK + the appropriate key in gtktextview.c. I can't see what could possibly be wrong from the winsys side. :\
Comment 16 Kamil Páral 2015-11-25 09:10:52 UTC
I can reproduce this issue (Ctrl+Backspace doesn't delete whole word in gedit under Wayland). I also have CapsLock key remapped, just differently.

This is under X11:
$ setxkbmap -query
rules:      evdev
model:      pc105
layout:     us,cz,cz
variant:    cz_sk_de,qwerty,
options:    grp:caps_toggle,terminate:ctrl_alt_bksp

And this is under Wayland:
$ setxkbmap -query
rules:      evdev
model:      pc105
layout:     us

(However even under Wayland, all keymaps and toggles work correctly, they're just not displayed in the command output).
Comment 17 Daniel Stone 2015-11-25 13:48:11 UTC
Note that setxkbmap -query won't work well under XWayland; all you can really do is 'xkbcomp -xkb :0 -' and look at the actual keymap.
Comment 18 Matthias Clasen 2015-11-30 15:22:08 UTC
How about all of you who see this issue try with a standard keymap, with no remapping or options, and see if the issue goes away ?
Comment 19 Matthias Clasen 2015-11-30 15:24:49 UTC
Just for kicks, I enabled the caps<>ctrl swap option here, and things still worked as expected in gedit (with the roles of the keys indeed swapped).
Comment 20 Daniel Stone 2015-11-30 15:36:15 UTC
Ha, it's not that as I suspected, but one I'd forgot I'd enabled: Ctrl+Alt+Backspace to kill the X server.

This changes backspace to:
    type "CTRL+ALT" {
        modifiers= Shift+Control+Alt+LevelThree;
        map[Shift]= Level2;
        preserve[Shift]= Shift;
        map[LevelThree]= Level3;
        map[Shift+LevelThree]= Level4;
        preserve[Shift+LevelThree]= Shift;
        map[Control+Alt]= Level5;
        level_name[Level1]= "Base";
        level_name[Level2]= "Shift";
        level_name[Level3]= "Alt Base";
        level_name[Level4]= "Shift Alt";
        level_name[Level5]= "Ctrl+Alt";
    };
    key <BKSP> {
        type= "CTRL+ALT",
        symbols[Group1]= [       BackSpace,       BackSpace,        NoSymbol,        NoSymbol, Terminate_Server ]
    };

This shouldn't make a difference, as Ctrl+BackSpace will generate backspace, but it's exactly this issue:
https://github.com/xkbcommon/libxkbcommon/issues/17

I still think it should get fixed in GTK+. But for the meantime, adding an explicit map for both Control and Control+Shift to Level1, with preserve entries for both, 'fixes' it.
Comment 21 Daniel Stone 2015-11-30 15:38:00 UTC
(tl;dr of the linked bug: XKB itself defines a way to interpret consumed modifiers, which xkbcommon follows. GTK+ copied the Xlib source to do this and changed the semantics, but doesn't use this method for the xkbcommon backend, as we don't expose enough of the keymap to let you do that.)
Comment 22 Carlos Garnacho 2015-12-07 13:54:30 UTC
I think this is the same issue that I tried to figure out on https://bugs.freedesktop.org/show_bug.cgi?id=92818

I'm attaching the GTK+-side patch I came up with, it at least fixes the function keys issue I was seeing, but my keymaps seem to work alright wrt Ctrl+Backspace here, so more testing welcome.
Comment 23 Carlos Garnacho 2015-12-07 13:56:17 UTC
Created attachment 316879 [details] [review]
wayland: Test partial consumed modifier matches more thoroughly

Keycodes in the "CTRL+ALT" group used to trigger server-side actions
(VT switching, terminate server, ...) and are traditionally never seen
by GTK+. However, libxkbcommon observes those, and gives those a
treatment suitable for the server-side, but unfortunately incompatible
with what GTK+ was doing there:

 - To xkbcommon, the consumed modifiers mask for any key in this group
   (eg. F1) would contain Ctrl|Shift|Mod1|Mod5, but however only the
   presence of Ctrl+(Mod1|5) would produce a different keysym.
 - GTK+ OTOH has never seen these keycombos, so to it, all the modifier
   combinations that can be seen result on the same keysym (eg. Shift+F1,
   ctrl+F1), as such GTK+ doesn't announce these modifiers as "consumed".

In order make GTK+ compatible with other backends, perform extra checks
on the consumed modifiers, if we're positive that the current combination
of modifiers doesn't result in a different keysym than it would happen
without those active, they are removed from the consumed modifiers mask.
Comment 24 Matthias Clasen 2015-12-07 17:38:51 UTC
Review of attachment 316879 [details] [review]:

I can't say that I've fully grasped the need for bit counting (could use builtin_popcount there if you want to be fancy), but this fixes my problem with Ctrl-F1, at least.
Comment 25 Daniel Stone 2015-12-08 18:05:50 UTC
I don't think this patch is right, given that it only very explicitly checks the Ctrl+Alt case (despite an appearance of generality). I'm still not sure if the best solution is new xkbcommon API or explicit entries in xkeyboard-config, but am perhaps leaning towards the latter.
Comment 26 Matthias Clasen 2015-12-08 18:51:45 UTC
Review of attachment 316879 [details] [review]:

.
Comment 27 Carlos Garnacho 2015-12-15 18:36:39 UTC
The bit counting (and why I think Daniel things it's not such a general fix) was added because I'm not sure what's best to do if there's more than one modifier pressed in the mask, I see 2 plausible options:

- Checking all the combinations of the enabled bits, because a subset of those might be triggering a different keysym.
- Checking only "all bits enabled" vs "all bits disabled", that might be enough to let us know whether the current combination of modifiers was actually consumed, but I don't know how widely this assumption applies.

So I added the 1bit-only check as both options collapse together in that case, and it coincides with the cases we most usually see in GTK+ clients, where two-modifiers keybindings are not that common, and definitely not with keys in the CTRL+ALT group anyway, it is worth noting that keys in other groups (eg. "ALPHABETIC") will still work fine because libxkbcommon and GTK+ seem to agree on what the consumed modifiers are.

Anyway, I'll be happy if a better way to handle this is devised, any "we sometimes don't fully trust xkbcommon" change will come across as specific.
Comment 28 Ran Benita 2016-01-20 23:21:47 UTC
Some background:
https://bugzilla.gnome.org/show_bug.cgi?id=100439
https://mail.gnome.org/archives/gtk-devel-list/2003-August/msg00173.html

And the GDK behavior is documented, so presumably it's considered API by now, and the wayland backend should reproduce it as faithfully as possible:
https://developer.gnome.org/gdk3/3.18/gdk3-Keyboard-Handling.html#gdk-keymap-translate-keyboard-state

Really, the root of all evil seems to be the CTRL+ALT type, it's the only one people complain about. It would have been nice if it was just fixed by adding the sensible preserves, but right now I don't think it would be smart to change - too risky.

I haven't read Carlos' patch deeply, but if after reading the above links you think it's correct I can review it if you want.

I don't think libxkbcommon should be changed, unless you find that a scratch xkb_state etc. does not suffice. This does leave occasional non-toolkit users out in the cold (like the one in the xkbcommon bug), which is not so nice. So maybe CTRL+ALT should be changed after all. But such users are in the same boat as pure-Xlib users, and shortcuts need *a lot* of hacks/special-cases besides this one.
Comment 29 Daniel Stone 2016-01-21 09:51:53 UTC
Yeah, looking at things just as they stand, I agree that Carlos's patch is probably fine, in that it fixes things for just the CTRL+ALT type. However, it's also pretty nasty, and I can nearly guarantee it will have unforseen issues later on down. We now have a bunch of pretty weird small keyboards which rely heavily on multiple modifiers for chording - the full 80s Emacs experience - so I worry that encoding this will just cause trouble later on down the line.

I really like Owen's conclusion here:
>  - consumed_modifiers contains modifiers that effect the translation
>   of the key *that are found in event->state*

although I would perhaps reword as:
> - consumed_modifiers contains modifiers that directly affected the translation of the last keypress

and I think that would be something good to encode generically through xkbcommon. Whether that's something we do by default (i.e. break current interface and also legacy XKB's definition of 'consumed'), or something we require people to opt in through an xkb_context flag for is up for discussion. My suggestion though, would be doing it by default, with an xkb_context flag to _disable_ (i.e. force old behaviour), which happens to work fine as we don't currently check for unsupported flags in xkb_context_new. :\

If you're fine with that, I can get the patch together and test with GTK+, since this hits me every day.
Comment 30 Ran Benita 2016-01-21 20:45:36 UTC
If we go the libxkbcommon route, I'd suggest the following:

Add new xkb_state_key_get_consumed_mods2 which takes a `enum xkb_consume_mode` parameter. One mode would be `XKB_CONSUME_MODE_XKB` (existing behavior), then we can add `XKB_CONSUME_MODE_DANIEL` for your suggested behavior (name is debatable :), maybe `XKB_CONSUME_MODE_GTK` for current GTK behavior, and more if (when) people complain. The existing function is same as calling 2 with `XKB_CONSUME_MODE_XKB`, for backward compatibility. The other existing functions `xkb_state_mod_index_is_consumed`, `xkb_state_mod_mask_remove_consumed` can be deprecated without replacement as far as I'm concerned.

Even though doing this while still working transparently with older libxkbcommon is nice, I don't like the suggestion to add a flag to the context for this - doesn't make sense there. And maybe you want to pass the xkb_state to some other code which wants a different mode? So users of new API would need to add the appropriate >= or autoconf stuff as usual.

I prefer mode to flags, because I think that if we ever add more than one flag they will probably not compose anyway. And a "mode" is easier to talk about.

What do you think? Once we have some code which implements the new "modes" we can see how they work out in practice.
Comment 31 Daniel Stone 2016-01-21 20:48:07 UTC
Heh! That works for me, though maybe we could avoid the API wart by adding xkb_state_key_get_significant_mods, taking the flag and documenting the asymmetry.
Comment 32 Ran Benita 2016-01-21 21:06:59 UTC
We already use the term "significant mods" for another (related) concept so that's not a good name: http://xkbcommon.org/doc/0.5.0/group__state.html
I believe I stole the term from GTK but not sure. (That doc section needs some fixing or scraping BTW).

I actually like the 2 because it's pretty unambiguous what it means - works for linux :) And adding a new term will make things even more confusing. There's also microsoft-style "_ex" or "_ext" instead of 2: http://stackoverflow.com/questions/3963374/what-does-it-mean-when-ex-is-added-to-a-function-method-name
Comment 33 Carlos Garnacho 2016-01-21 22:04:30 UTC
We luckily have plenty of room in the "bad function names" game: https://msdn.microsoft.com/en-us/library/windows/desktop/bb776479(v=vs.85).aspx

Jokes aside, thanks for considering a solution here guys :)
Comment 34 Matthias Clasen 2016-02-13 22:23:26 UTC
Daniel, Carlos, did we come to an agreed solution here ?
Comment 35 Ran Benita 2016-02-27 23:49:06 UTC
Here are patches for xkbcommon, following comment #30: https://github.com/xkbcommon/libxkbcommon/pull/31 if we decide to do it in xkbcommon.
Comment 36 Daniel Stone 2017-01-17 11:38:28 UTC
This was merged a while ago, and available as of 0.7.0:
https://github.com/xkbcommon/libxkbcommon/commit/babc9e0c30
Comment 37 Emmanuele Bassi (:ebassi) 2017-01-17 11:43:32 UTC
*** Bug 763161 has been marked as a duplicate of this bug. ***
Comment 38 Christian Stadelmann 2017-10-27 22:50:41 UTC
This bug is related to bug #772506 or maybe a duplicate of it (or the other way round).
Comment 39 Florian Müllner 2018-01-25 10:30:42 UTC
*** Bug 772506 has been marked as a duplicate of this bug. ***
Comment 40 GNOME Infrastructure Team 2018-05-02 16:43:25 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/570.