Created attachment 237621 [details] [review]
keyboard: Get layouts and variants from localed

We get the current X server's XKB layouts and variants when populating
empty settings or running under GDM. This means that the g-s-d
instance running under GDM changes the X server's configuration and
thus, after a freshly created user account logs in, the new g-s-d
instance will no longer see the system's xorg.conf layouts and
variants but instead the ones that were last active in GDM due to the
way we configure XKB.

Using localed to get the system settings allows us to always get what
we actually want.

Created attachment 237622 [details] [review]
keyboard: Get XKB options from localed

Some users might have XKB options configured in their xorg.conf. In
case the current user's setting is empty or when running under GDM we
should honor the system options.

Review of attachment 237621 [details] [review]:

::: plugins/keyboard/gsd-keyboard-manager.c
@@ +1307,3 @@
+        if (v) {
+                const gchar *s = g_variant_get_string (v, NULL);
+                if (s && *s)

The NULL check on |s| is unnecessary since g_variant_get_string *never* returns NULL.

@@ +1318,3 @@
+        if (v) {
+                const gchar *s = g_variant_get_string (v, NULL);
+                if (s && *s)

Same here.

Review of attachment 237621 [details] [review]:

Why not just avoid writing the configuration when running under GDM?
The XKB config comes from the X.org config file which are generated by localed. Furthermore, GDM's configuration should really be cleaned up for each session.

Review of attachment 237622 [details] [review]:

::: plugins/keyboard/gsd-keyboard-manager.c
@@ +1517,3 @@
+
+        options = g_settings_get_strv (settings, KEY_KEYBOARD_OPTIONS);
+        if (g_strv_length (options) < 1)

Seems that this makes it impossible to disable features that were enabled for GDM.
I would only inherit the server's/localed's x11options if we also inherited the layouts.

Ok, this is a bit nuanced - I got it wrong myself when I wrote this for 3.6!

I think you might be overlooking the fact that the gsettings store under GDM isn't the same as the one we then get for the user session. What does get propagated from the GDM session to the user session is the X property containing the XKB layouts/variants/options and that property is set by g-s-d when running under GDM and thus, in the user session, it will no longer be the original data that was there from the start, i.e. the xorg.conf data that the X server gets when it starts.

For example, suppose you have "us,fr,ru" as layouts in your xorg.conf. GDM's g-s-d will turn those into 3 input sources "us", "fr" and "ru". We need to do this so that the gnome-shell panel indicator actually shows up with 3 different entries.

Then it applies the first one "us" to the X server and thus the X property gets set to "us" only. Then the user's g-s-d could only create the "us" input source (for a fresh, settings-less user anyway).

Does that address both your comments?

Created attachment 238720 [details] [review]
keyboard: Get layouts and variants from localed

We used to get the current X server's XKB layouts and variants from
the X property when populating empty settings or running under
GDM. This means that the g-s-d instance running under GDM would then
change the X server's configuration (as a side-effect of applying the
input source) and thus, after a freshly created user account logged
in, the user session's g-s-d instance would no longer see the
system's xorg.conf layouts and variants but instead the ones that were
last active in GDM.

Using localed to get layouts and variants will allow us to always get
what we actually want i.e. the system settings.
--

Hopefully clarified the commit message.

I also squashed a fix I had locally to explicitly clear the
gsetting. Note that the previous code didn't need this since it always
started from an empty list but to make the code a bit more generic I'm
always starting from the current setting and just add stuff to it so
in the GDM path we need to explicitly clear it first.

Created attachment 238721 [details] [review]
keyboard: Get XKB options from localed

--
rebased

Review of attachment 238720 [details] [review]:

Looks good.

Review of attachment 238721 [details] [review]:

Looks good.

Attachment 238720 [details] pushed as 79b0f1b - keyboard: Get layouts and variants from localed
Attachment 238721 [details] pushed as 212fde3 - keyboard: Get XKB options from localed