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 613681 - Removes system keyboard layouts for session
Removes system keyboard layouts for session
Status: RESOLVED OBSOLETE
Product: gdm
Classification: Core
Component: general
2.29.x
Other Linux
: Normal normal
: ---
Assigned To: GDM maintainers
GDM maintainers
Depends on:
Blocks: 622305
 
 
Reported: 2010-03-23 11:47 UTC by Martin Pitt
Modified: 2013-11-18 15:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Keep multiple system keyboard layouts for session (3.45 KB, patch)
2010-03-23 12:04 UTC, Martin Pitt
none Details | Review
Keep multiple system keyboard layouts for session (3.70 KB, patch)
2010-03-29 08:01 UTC, Martin Pitt
none Details | Review
Keep multiple system keyboard layouts for session (3.87 KB, patch)
2010-03-30 20:10 UTC, Martin Pitt
none Details | Review

Description Martin Pitt 2010-03-23 11:47:28 UTC
I'm currently fixing gdm and g-s-d to properly handle situations where the system (udev/xorg.conf/X) configures multiple keyboard layouts. This is important for non-Latin languages. E. g. if you normally use a Russian,Greek, or Hebrew layout, you always need an additional alternative Latin layout (usually US) to be able to enter URLs, use keyboard shortcuts and the like.

The g-s-d side of the problem is in bug 613666.

For gdm, the problem is that gdm_layout_activate() overwrites the configured keyboard layouts with just a single one. E. g. when the system configures two layouts "us" and "de nodeadkeys", X will start up with this property:

  _XKB_RULES_NAMES(STRING) = "evdev", "pc105", "us,de", ",nodeadkeys", "terminate:ctrl_alt_bksp,grp:shifts_toggle"

But when selecting either keyboard in gdm, the property gets overwritten to just have the selected one. What should happen instead is that the selected one becomes the first one, and the other configured system layouts should be appended (with avoiding duplicate entries).

With that, gnome-settings-daemon can then do the right thing and make all those layouts available if /desktop/gnome/peripherals/keyboard/kbd/layouts is empty (i. e. if you didn't manually configure layouts in the session again).

Thanks for considering!
Comment 1 Martin Pitt 2010-03-23 12:04:24 UTC
Created attachment 156853 [details] [review]
Keep multiple system keyboard layouts for session

I tested this patch with having "us" and "de nodeadkeys" configured in the
system, i. e. 

XKB_RULES_NAMES(STRING) = "evdev", "pc105", "us,de", ",nodeadkeys",
"terminate:ctrl_alt_bksp,grp:shifts_toggle"

I tested four situations:

 * Do not touch the keyboard selector in gdm at all. It defaults to "us" (first
entry in the XKB list). This essentially doesn't change the configuration for
the session at all, I get "us,de" ",nodeadkeys" in the session.

 * Select a different layout in gdm which is already configured: I pick "de
nodeadkeys" in gdm, and end up with "de,us" "nodeadkeys,".

 * Select a layout which is configured, but with a different variant: I pick
"de" (which is with dead keys). This ends up as "de,us,de" ",,nodeadkeys".

 * Select an entirely different layout: I pick "France", and get "fr,us,de",
",,nodeadkeys".

In all cases I booted into an "xterm" session, to ensure that g-s-d etc. does
not further modify the settings. I verified that I got the correct keyboard
layout and deadkey-ness.
Comment 2 Takao Fujiwara 2010-03-24 08:59:11 UTC
I worked on the same issue with bug 610903.
Comment 3 Martin Pitt 2010-03-29 08:01:58 UTC
Created attachment 157356 [details] [review]
Keep multiple system keyboard layouts for session

The previous patch sometimes caused layout duplication, due to treating a NULL variant differently than a "" variant (https://launchpad.net/bugs/548778).

This updated patch fixes this by treating them equally.
Comment 4 Martin Pitt 2010-03-30 20:10:20 UTC
Created attachment 157525 [details] [review]
Keep multiple system keyboard layouts for session

Yet another updated patch. For once this fixes an invalid duplicated g_new() for variants.

We also noticed that variants were not actually set in some cases (https://launchpad.net/bugs/550887) because libxklavier is really mean: For multiple layouts, an empty variant is returned as NULL in xkl_config_rec_get_from_server(), but xkl_config_rec_activate() silently ignores the configuration if a layout in the config record is NULL, such as here:

  layout[0] = "de"
  layout[1] = "us"
  variant[0] = "nodeadkeys"
  variant[1] = NULL

This is arguably a bug in libxklavier, but this patch works around this by always setting consistent lists for layouts and variants.
Comment 5 Takao Fujiwara 2010-03-31 11:07:15 UTC
I tried to merge this fix into the fix of bug 610903.
I'd like to use the system multiple layouts only when user's chosen layout is the system keyboard layout.
Comment 6 Martin Pitt 2010-03-31 12:33:09 UTC
Takao,

thanks. 

(In reply to comment #5)
> I'd like to use the system multiple layouts only when user's chosen layout is
> the system keyboard layout.

I think that would make sense if the layout is an entirely new one. However, if the user picks one of the layouts which the system already configures, I think gdm should still keep all of them, and just put the selected one at the front. I. e. on a system with "us" and "gr", users can pick which one to get by default, but still keep an empty gconf key for their layouts in gnome-settings-daemon.
Comment 7 Christophe Fergeau 2010-06-04 16:39:49 UTC
Patch is pretty wrong in its memory handling:
* config->layouts is allocated memory for g_strv_length (initial_config->layouts) + 2 elements
* config->variants is allocated memory for g_strv_length (initial_config->variants) + 2

* ii will take values from 0 to g_strv_length (initial_config->layouts)-1
* ic is incremented at most once per iteration and is initially 1 so it will take values from 1 to g_strv_length (initial_config->layouts)

And then we can see
* config->layouts[ic]
* initial_config->layouts[ii]
* config->variants[ic]
* initial_config->variants[ii]

ie layouts and variants are handled as if they have the same bounds, which isn't true on at least one machine here (standard mandriva cooker installation in russian using the "ru(phonetic)" keyboard). On this box, g_strv_length (initial_config->layouts) is 2, g_strv_length (initial_config->variants) is 0 which causes various out of bound accesses, and finally a crash because config->variants contains pointers to invalid memory.
Comment 8 Ray Strode [halfline] 2013-11-18 15:16:27 UTC
closing obsolete since we don't ship a greeter in GDM anymore.