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 596897 - Ignores variants in $GDM_KEYBOARD_LAYOUT
Ignores variants in $GDM_KEYBOARD_LAYOUT
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: plugins
2.28.x
Other Linux
: Normal major
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2009-09-30 17:13 UTC by Martin Pitt
Modified: 2009-10-19 17:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix variant handling in $GDM_KEYBOARD_LAYOUT (1.84 KB, patch)
2009-10-01 09:12 UTC, Martin Pitt
reviewed Details | Review
fix variant handling in $GDM_KEYBOARD_LAYOUT (1.83 KB, patch)
2009-10-19 17:00 UTC, Martin Pitt
accepted-commit_now Details | Review

Description Martin Pitt 2009-09-30 17:13:53 UTC
This is the counterpart of bug #572765 in gdm.

In gdm you can select keyboard layouts including variants, such as "USA Dvorak" or "German with no dead keys". gdm exports the layout in $GDM_KEYBOARD_LAYOUT, with space-separating layout name and variant. E. g. "de nodeadkeys".

However, g-s-d does not take this into account. Now that gdm is quite insistant to install its layout (even if you never touched it in gdm) and writes it to ~/.dmrc, this pretty much breaks all layout variants.
Comment 1 Martin Pitt 2009-10-01 09:12:51 UTC
Created attachment 144474 [details] [review]
fix variant handling in $GDM_KEYBOARD_LAYOUT

gdm's configuration and $GDM_KEYBOARD_LAYOUT separates layout and
variant with a space, but GConf uses tabs. This patch convert spaces to tabs in
$GDM_KEYBOARD_LAYOUT to work with either format, for more robustness.

Tested with various cases:

$ gconftool -g /desktop/gnome/peripherals/keyboard/kbd/layouts
[us,de	nodeadkeys]

GDM_KEYBOARD_LAYOUT=="de nodeadkeys" selects the exactly matching layout, and "de" selects the same, in the spirit of the "closest match" code [1].

The inverse case, if you configured [us,de] in gconf, then passing "de nodeadkeys" still will not do anything. This is covered in bug 585290, and is considered "wontfix" (FWIW, I don't quite agree, but let's handle this separately, so my patch doesn't cover this case).

It also works correctly if gconf settings are empty ([]), then the "initial configuration" case kicks in [2] and sets gconf to the passed layout, with correct tab separation:

$ gconftool -u /desktop/gnome/peripherals/keyboard/kbd/layouts
$ gconftool -g /desktop/gnome/peripherals/keyboard/kbd/layouts
[]
$ GDM_KEYBOARD_LAYOUT="de nodeadkeys" gnome-settings-daemon --debug --no-daemon
[..]
$ gconftool -g /desktop/gnome/peripherals/keyboard/kbd/layouts
[de     nodeadkeys]


[1] http://git.gnome.org/cgit/gnome-settings-daemon/tree/plugins/keyboard/gsd-keyboard-xkb.c?id=632055166859aba574b8103a3f3d609c1e130b13#n261
[2] http://git.gnome.org/cgit/gnome-settings-daemon/tree/plugins/keyboard/gsd-keyboard-xkb.c?id=632055166859aba574b8103a3f3d609c1e130b13#n239
Comment 2 Jens Granseuer 2009-10-17 13:03:20 UTC
Comment on attachment 144474 [details] [review]
fix variant handling in $GDM_KEYBOARD_LAYOUT

Please commit when the corresponding gdm change goes in.

But please don't use nested blocks without braces. Thanks.
Comment 3 Martin Pitt 2009-10-19 16:56:22 UTC
(In reply to comment #2)
> Please commit when the corresponding gdm change goes in.

Please note that these are independent. The gdm patch is about gdm determining the correct default layout for the shown selectors, while this one is about actually using a selected variant. You can select variants in current gdm just fine, they will just not be selected by default if you have a variant in your hal keyboard config.

So this patch can be applied independently. Or did you mean something else?

I can't commit myself, but I'll ask a colleague of mine to do that after fixing the style issue below.
 
> But please don't use nested blocks without braces. Thanks.

Ah, in the for loop? Sure.
Comment 4 Martin Pitt 2009-10-19 17:00:29 UTC
Created attachment 145796 [details] [review]
fix variant handling in $GDM_KEYBOARD_LAYOUT

Same logic, but with fixed indentation and braces style.
Comment 5 Jens Granseuer 2009-10-19 17:11:03 UTC
Comment on attachment 145796 [details] [review]
fix variant handling in $GDM_KEYBOARD_LAYOUT

Ah, I thought this depended on some behavioural change in gdm as well.

Now, this

 	if (gdm_layout != NULL) {

seems to imply that gdm_layout can be NULL, which would break the g_strdup.
Comment 6 Martin Pitt 2009-10-19 17:20:45 UTC
How so? g_strdup() works just fine with NULL and returns NULL in that case.

http://library.gnome.org/devel/glib/stable/glib-String-Utility-Functions.html#g-strdup
Comment 7 Jens Granseuer 2009-10-19 17:27:07 UTC
Comment on attachment 145796 [details] [review]
fix variant handling in $GDM_KEYBOARD_LAYOUT

Oh, er, in that case, just go ahead.
Comment 8 C de-Avillez 2009-10-19 17:54:34 UTC
Chris commited the fix to GIT: http://git.gnome.org/cgit/gnome-settings-daemon/commit/?id=3d5189d3984980ec97d794f7bde6159bc97e1379

Marking Resolved/Fixed.