GNOME Bugzilla – Bug 613681
Removes system keyboard layouts for session
Last modified: 2013-11-18 15:16:27 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!
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.
I worked on the same issue with bug 610903.
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.
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.
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.
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.
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.
closing obsolete since we don't ship a greeter in GDM anymore.