GNOME Bugzilla – Bug 599273
gdm doesn't reread system language settings when they change
Last modified: 2009-12-21 18:34:33 UTC
Created attachment 146023 [details] [review] Patch for Makefile.am, gdm-display.c, gdm.in If we modify /etc/sysconfig/i18n with system-config-language, It's better to change the GDM system locale without rebooting the system. We fixed it in GNOME 2.20 (384603). I think it's good to port the fix in trunk. Now I think using a script might be an easy implementation to load the system file. gdm-binary restarts gdm-simple-slave so if we load the LANG value before the slave is invoked, greeter can show the latest LANG.
Review of attachment 146023 [details] [review]: I'm not a fan of the script thing. In 2.20 we just parsed the file directly I think. Maybe you could just crib that code? ::: gdm-2.26.1/daemon/Makefile.am.orig @@ +16,3 @@ + do + export $l + done This is a little clumbsy. I think I'd rather see either a white list of allowed variables for export or set -a. @@ +39,3 @@ +*) + exec @sbindir@/gdm-binary "$@" + ;; This is a little strange, I think i'd rather a separate wrapper script over reusing the gdm script. But honestly I'd rather no script at all.
Created attachment 146413 [details] [review] Patch for gdm.in, gdm-slave-proxy.c > I'm not a fan of the script thing. In 2.20 we just parsed the file directly I think. Maybe you could just crib that code? OK, I moved the patch from 2.20. We don't have to call setlocale() since setlocale() will be called by the forked applications. The previous bonobo had a LC_MESSAGES issue but it's fixed at present so probably it's better to remove LC_MESSAGES in gdm.in.
Created attachment 146540 [details] [review] Patch for configure.in, gdm.in, gdm-slave-proxy.c Revised the patch to make sure match_info = NULL. Probably $(sysconfdir) is better than ${sysconfdir} since it's quoted by the single quotes and used in Makefile.
Review of attachment 146540 [details] [review]: ::: gdm/configure.ac.orig @@ +117,3 @@ + g_unsetenv ("LC_MESSAGES"); + return; + } What's the point of this? I'd just drop the GDM_LANG stuff. @@ +122,3 @@ + + if (!g_file_test (config_file, G_FILE_TEST_EXISTS)) { + g_warning ("Cannot access '%s'", config_file); no need for a warning here @@ +130,3 @@ + g_warning ("Failed to parse '%s': %s", + config_file, + (error && error->message) ? error->message : "(null)"); should probably be g_debug not g_warning. It's not a bug if the file doesn't exist. @@ +171,3 @@ + if (!g_match_info_matches (match_info)) + goto next_line; + need braces around hte if statements to match the coding style @@ +178,3 @@ + g_setenv (key, value, TRUE); + else if (key && *key) + g_unsetenv (key); here too @@ +227,3 @@ spawn_child_setup (SpawnChildData *data) { +#ifdef LANG_CONFIG_FILE don't think we need (or want) an #ifdef here.
Created attachment 146730 [details] [review] Patch for configure.in, gdm.in, gdm-slave-proxy.c (In reply to comment #4) > What's the point of this? I'd just drop the GDM_LANG stuff. In GDM 2.20, GDM_LANG was used for the system LANG against the user LANG. GDM 2.20 had a menu item likes "Set system language" but it's now removed in the latest GDM. I put it as the back compatibility. If we set $GDM_LANG, normally it used to set the default system LANG and to show the default greeter locale. If system has LC_ALL=ja_JP.UTF-8 and LC_MESSAGES=C and GDM_LANG=ja_JP.UTF-8, it would expect other CLI processes show C messages but GDM GUI shows the messages with GDM_LANG. And then I put unsetenv (LC_MESSAGES) here. However I removed GDM_LANG lines at the moment with your suggestion. Actually I'm not sure if we still need GDM_LANG. If we'll need GDM_LANG with other fixes, I'll file another bug. > should probably be g_debug not g_warning. It's not a bug if the file doesn't > exist. OK, I replaced g_warning wiht g_debug. > need braces around hte if statements to match the coding style OK, I put the braces. > don't think we need (or want) an #ifdef here. OK, I removed it.
yea we removed GDM_LANG from 2.22 or something because it didn't seem relevant anymore.
I'm not sure you're really calling this in the right place. Wouldn't get_welcome_environment be a more natural place to hook in?
(In reply to comment #7) > I'm not sure you're really calling this in the right place. Wouldn't > get_welcome_environment be a more natural place to hook in? OK, it seems better to put the fix in get_welcome_environment(). I will test it.
Created attachment 146904 [details] [review] Patch for configure.in, gdm.in, gdm-welcome-session.c I revised the patch to update get_welcome_environment() with your suggestion. I used optional_environment value.
Comment on attachment 146904 [details] [review] Patch for configure.in, gdm.in, gdm-welcome-session.c thanks, pushed.