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 756543 - bug in modifiers-only input source switching?
bug in modifiers-only input source switching?
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-10-14 00:15 UTC by Alberts Muktupāvels
Modified: 2016-01-13 14:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backend-x11: fix modifiers-only input source switching (2.04 KB, patch)
2015-10-14 00:16 UTC, Alberts Muktupāvels
committed Details | Review

Description Alberts Muktupāvels 2015-10-14 00:15:38 UTC
I have following input sources:
1) Latvian (apostrophe variant)
2) English (UK)
3) Polish (Dvorak)

Caps Lock is set as modifiers-only switch to next source.

Standard keybinding switching works, but modifiers-only not. Source is changed and indicator shows correct source.

In Latvian I can type 'ā', but pressing Caps Lock 3 times (that should switch throw all sources and return to first one) I can no longer type 'ā'.

It looks like following things happens:
1) change source first is called meta_backend_x11_lock_layout_group and correct group is locked.
2) then grp:caps_toggle causes another XkbLockGroup with group + 1.
Comment 1 Alberts Muktupāvels 2015-10-14 00:16:30 UTC
Created attachment 313226 [details] [review]
backend-x11: fix modifiers-only input source switching
Comment 2 Rui Matos 2015-10-18 15:43:45 UTC
(In reply to Alberts Muktupāvels from comment #0)
> I have following input sources:
> 1) Latvian (apostrophe variant)
> 2) English (UK)
> 3) Polish (Dvorak)
> 
> Caps Lock is set as modifiers-only switch to next source.
> 
> Standard keybinding switching works, but modifiers-only not. Source is
> changed and indicator shows correct source.
> 
> In Latvian I can type 'ā', but pressing Caps Lock 3 times (that should
> switch throw all sources and return to first one) I can no longer type 'ā'.

I can't reproduce this here.

> It looks like following things happens:
> 1) change source first is called meta_backend_x11_lock_layout_group and
> correct group is locked.
> 2) then grp:caps_toggle causes another XkbLockGroup with group + 1.

How did you reach this conclusion? The way it should work (and works AFAICT) is

1) press caps
2) group index is changed internally on the X server
3) mutter gets the ISO_Next_Group key event
4) mutter/gnome-shell chooses the next layout and XkbLockGroup()'s it

Note that 4 overrides 2.
Comment 3 Alberts Muktupāvels 2015-10-20 13:39:59 UTC
(In reply to Rui Matos from comment #2)
> (In reply to Alberts Muktupāvels from comment #0)
> > I have following input sources:
> > 1) Latvian (apostrophe variant)
> > 2) English (UK)
> > 3) Polish (Dvorak)
> > 
> > Caps Lock is set as modifiers-only switch to next source.
> > 
> > Standard keybinding switching works, but modifiers-only not. Source is
> > changed and indicator shows correct source.
> > 
> > In Latvian I can type 'ā', but pressing Caps Lock 3 times (that should
> > switch throw all sources and return to first one) I can no longer type 'ā'.
> 
> I can't reproduce this here.

But I can, so there must be problem, no? I am using nvidia proprietary driver, does it change something?

> > It looks like following things happens:
> > 1) change source first is called meta_backend_x11_lock_layout_group and
> > correct group is locked.
> > 2) then grp:caps_toggle causes another XkbLockGroup with group + 1.
> 
> How did you reach this conclusion? The way it should work (and works AFAICT)
> is

By adding XkbStateNotify and printing *->state.locked_group. After running 'gnome-shell --replace' (with same input sources) and pressing caps three times I got 5 events. locked_group was (1, 2), (3), (0, 1). Events from one press in brackets. Language indicator does show lv as selected input source, but it definitely is not lv. And it looks like I can press caps forever but it never will change input source to lv.

With my proposed patch pressing caps three times I got 9 events - (1, 2, 1), (2, 3, 2), (0, 1, 0).

> 1) press caps
> 2) group index is changed internally on the X server
> 3) mutter gets the ISO_Next_Group key event
> 4) mutter/gnome-shell chooses the next layout and XkbLockGroup()'s it
> 
> Note that 4 overrides 2.

This sounds logical, but apparently it is not always true. :(
Comment 4 Rui Matos 2015-10-20 14:50:25 UTC
Do you see the XkbStateNotify with a new group when you press or when you release the caps key ?
Comment 5 Alberts Muktupāvels 2015-10-20 15:00:46 UTC
(In reply to Rui Matos from comment #4)
> Do you see the XkbStateNotify with a new group when you press or when you
> release the caps key ?

First press:
1) press - locked_group == 1
2) release - locked_group == 2

Second press:
1) press - nothing
2) release - locked_group == 3

Third press:
1) press - locked_group == 0
2) release - locked_group == 1

Fourth press:
1) press - nothing
2) release - locked_group == 2

Fifth press:
1) press - nothing
2) release - locked_group == 3

Sixth press:
1) press - locked_group == 0
2) release - locked_group == 1
Comment 6 Rui Matos 2015-10-20 15:45:46 UTC
Which distribution are you using? Which X server version?
Comment 7 Alberts Muktupāvels 2015-10-20 15:53:48 UTC
Ubuntu 15.10 - development version with X.Org X Server 1.17.2, but I have built full GNOME Shell with JHBuild. X version in GNOME Shell session is 1.17.99.901 (1.18.0 RC 1).
Comment 8 Rui Matos 2015-10-20 16:20:29 UTC
Great... check https://bugs.launchpad.net/ubuntu/+source/gnome-control-center/+bug/36812

I'll think about your patch.
Comment 9 Alberts Muktupāvels 2015-10-20 17:08:53 UTC
(In reply to Rui Matos from comment #8)
> Great... check
> https://bugs.launchpad.net/ubuntu/+source/gnome-control-center/+bug/36812

I have no time to read all comments there, but from descriptions sounds like different bug... Is it X bug? But then why you can not reproduce it?

> I'll think about your patch.

I guess patch title needs to be changed to - 'force locked group' or something like that. If you are going to accept that patch please make any needed changes yourself.

Can that patch cause something bad?
Comment 10 Rui Matos 2015-10-21 14:34:01 UTC
Review of attachment 313226 [details] [review]:

(In reply to Alberts Muktupāvels from comment #9)
> (In reply to Rui Matos from comment #8)
> > Great... check
> > https://bugs.launchpad.net/ubuntu/+source/gnome-control-center/+bug/36812
> 
> I have no time to read all comments there, but from descriptions sounds like
> different bug... Is it X bug? But then why you can not reproduce it?

Ubuntu ships a patch in the X server that makes the group switch keybindings only work on key release, i.e. the X server internal group locking happens only on key release which means that mutter gets the XKB_KEY_ISO_Next_Group key press event, does its XLockGroup() call with a new index and then, on key release, the X server moves the index further again. In an unpatched X server, the internal group index lock happens before mutter's XLockGroup() call. That's why this is only a bug in Ubuntu.

> I guess patch title needs to be changed to - 'force locked group' or
> something like that. If you are going to accept that patch please make any
> needed changes yourself.

Ok, the patch looks mostly fine so I'm going to push it with the fixes below and the explanation above in the commit message. Thank you

::: src/backends/x11/meta-backend-x11.c
@@ +83,3 @@
   gchar *keymap_variants;
   gchar *keymap_options;
+

no needs for this empty line

@@ +309,3 @@
             case XkbMapNotify:
               keymap_changed (backend);
+            case XkbStateNotify:

missing a break; here
Comment 11 Jasper St. Pierre (not reading bugmail) 2015-10-21 16:10:20 UTC
(In reply to Rui Matos from comment #10)
> Ubuntu ships a patch in the X server that makes the group switch keybindings
> only work on key release, i.e. the X server internal group locking happens
> only on key release which means that mutter gets the XKB_KEY_ISO_Next_Group
> key press event, does its XLockGroup() call with a new index and then, on
> key release, the X server moves the index further again. In an unpatched X
> server, the internal group index lock happens before mutter's XLockGroup()
> call. That's why this is only a bug in Ubuntu.

...
Comment 12 Alberts Muktupāvels 2015-10-21 21:15:42 UTC
(In reply to Rui Matos from comment #10)
> Review of attachment 313226 [details] [review] [review]:
> 
> (In reply to Alberts Muktupāvels from comment #9)
> > (In reply to Rui Matos from comment #8)
> > > Great... check
> > > https://bugs.launchpad.net/ubuntu/+source/gnome-control-center/+bug/36812
> > 
> > I have no time to read all comments there, but from descriptions sounds like
> > different bug... Is it X bug? But then why you can not reproduce it?
> 
> Ubuntu ships a patch in the X server that makes the group switch keybindings
> only work on key release, i.e. the X server internal group locking happens
> only on key release which means that mutter gets the XKB_KEY_ISO_Next_Group
> key press event, does its XLockGroup() call with a new index and then, on
> key release, the X server moves the index further again. In an unpatched X
> server, the internal group index lock happens before mutter's XLockGroup()
> call. That's why this is only a bug in Ubuntu.

Ubuntu has 1.17.2 version, but JHBuild session has 1.17.99.901 (1.18.0 RC 1) - so this has nothing to do with Ubuntu version. Xorg is from xwayland module.
Comment 13 Rui Matos 2015-10-22 09:50:32 UTC
(In reply to Alberts Muktupāvels from comment #12)
> Ubuntu has 1.17.2 version, but JHBuild session has 1.17.99.901 (1.18.0 RC 1)
> - so this has nothing to do with Ubuntu version. Xorg is from xwayland
> module.

a) xwayland is only used for wayland sessions and you're not using a wayland session (or you wouldn't have this problem in the first place).

b) are you sure the actual running X server is the one compiled in jhbuild? how are you starting the session?
Comment 14 Alberts Muktupāvels 2015-10-22 11:55:20 UTC
(In reply to Rui Matos from comment #13)
> (In reply to Alberts Muktupāvels from comment #12)
> > Ubuntu has 1.17.2 version, but JHBuild session has 1.17.99.901 (1.18.0 RC 1)
> > - so this has nothing to do with Ubuntu version. Xorg is from xwayland
> > module.
> 
> a) xwayland is only used for wayland sessions and you're not using a wayland
> session (or you wouldn't have this problem in the first place).

With 'Xorg is from xwayland module' I mean this:
https://git.gnome.org/browse/jhbuild/tree/modulesets/gnome-suites-core-deps-3.20.modules#n1298

> b) are you sure the actual running X server is the one compiled in jhbuild?
> how are you starting the session?

Yes, unless we are speaking about different things. Also how would you explain different versions? Both are from 'X -version' command.

Session is started from display manager. Do I need to post script that is used to start session? It is similar to this example:
https://git.gnome.org/browse/jhbuild/tree/examples/jhbuild-session
Comment 15 Rui Matos 2015-10-22 12:05:15 UTC
(In reply to Alberts Muktupāvels from comment #14)
> Yes, unless we are speaking about different things. Also how would you
> explain different versions? Both are from 'X -version' command.
> 
> Session is started from display manager. Do I need to post script that is
> used to start session? It is similar to this example:
> https://git.gnome.org/browse/jhbuild/tree/examples/jhbuild-session

I'm guessing the display manager doesn't use the X server from your jhbuild prefix. What does 'xdpyinfo | head' say?
Comment 16 Alberts Muktupāvels 2015-10-22 13:08:16 UTC
Oh, now I feel stupid...
Comment 17 van.de.bugger 2015-12-22 08:49:04 UTC
Hi, guys, I am very glad you have fixed modifiers-only switch on Ubuntu. But why did you broke all the third-party keyboard switchers on all the other Linuxes?

I work in Fedora and use a 3rd-party keyboard switcher. It worked greatly until I have updated Gnome to v3.18. Now, when I try to switch keyboard layout, Scroll Lock LED (I use it as keyboard layout indicator) blinks for a moment, and mutter resets keyboard layout back to original one.

When I rolled back this patch and rebuild mutter, everything works as expected.

This "fix" is intended to work with Ubuntu-patched X server. Would not it better to provide this behavior in Ubuntu-patched mutter?
Comment 18 Rui Matos 2016-01-04 16:46:02 UTC
(In reply to van.de.bugger from comment #17)
> Hi, guys, I am very glad you have fixed modifiers-only switch on Ubuntu. But
> why did you broke all the third-party keyboard switchers on all the other
> Linuxes?

Third-party keyboard switchers are not supported in GNOME. Plenty of other XKB knobs/behaviors were already impossible or at least impractical when set from outside mutter's control. That's a conscious design decision that's not going to change unless there's a very good case for it.
Comment 19 van.de.bugger 2016-01-13 14:27:44 UTC
(In reply to Rui Matos from comment #18)
> Third-party keyboard switchers are not supported in GNOME. Plenty of other
> XKB knobs/behaviors were already impossible or at least impractical when set
> from outside mutter's control. That's a conscious design decision that's not
> going to change unless there's a very good case for it.

Ok.

Do I understand correctly that patch you wrote about

> Ubuntu ships a patch in the X server that makes the group switch keybindings...

is *not* included into upstream X Server?

I found this patch, it is 208_switch_on_release.diff.diff, and it is present *neither* in X server "master" branch *nor* in "server-1.18-branch" branch.

So, what the *good* reason to fix upstream mutter specially for *patched* X Server? Why don't let Debian/Ubuntu patch both X Server and mutter?