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 786422 - gdm-session: user's session temporarily setup with the wrong language
gdm-session: user's session temporarily setup with the wrong language
Status: RESOLVED FIXED
Product: gdm
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GDM maintainers
GDM maintainers
Depends on:
Blocks:
 
 
Reported: 2017-08-17 12:16 UTC by Mario Sánchez Prada
Modified: 2017-09-18 17:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdm-session: always pass down user saved language (3.32 KB, patch)
2017-08-17 12:23 UTC, Mario Sánchez Prada
none Details | Review
gdm-session: always pass down user saved language (1.47 KB, patch)
2017-09-18 16:54 UTC, Mario Sánchez Prada
committed Details | Review

Description Mario Sánchez Prada 2017-08-17 12:16:37 UTC
As we found out internally there is at least a case where changing the language of the login screen from gnome-control-center[*] does not work quite as expected until you reboot the machine.

Steps to reproduce this problem (I just test this on my Fedora 26 VM):
  1. Start with both shell and login languages set to the same language (e.g English)
  2. Change login language to Spanish (but leave the shell language as English)
  3. Log out --> login screen's UI properly displays in Spanish
  4. Log in --> the shell language is now in Spanish!
  5. Reboot --> all is correct: login language Spanish, shell language English

Felipe Erias wrote a patch for us a while ago to fix this problem, which I just came across of while upgrading Endless's GDM to 3.24, and which I can confirm fixes the issue (which is still present in GDM's master), so maybe that's something could be considered upstream: https://github.com/endlessm/gdm/commit/2e4780168e09f2f6a2c00cbc67c9817683669e95

[*] You need to click on the "Login Screen" button in the "Region & Language" panel, but that button will only show up if you have more than one user configured, so make sure you add a second one first if needed.
Comment 1 Mario Sánchez Prada 2017-08-17 12:23:07 UTC
Created attachment 357808 [details] [review]
gdm-session: always pass down user saved language
Comment 2 Ray Strode [halfline] 2017-09-08 18:51:58 UTC
Review of attachment 357808 [details] [review]:

::: daemon/gdm-session.c
@@ +914,3 @@
         GdmSession *self = conversation->session;
 
+        if (strlen (language_name) > 0) {

so i guess this is the part you described in the commit message.

@@ +2521,3 @@
+            g_debug ("GdmSession: using saved language %s", self->priv->saved_language);
+            gdm_session_set_environment_variable (self, "LANG", self->priv->saved_language);
+        }

what's this part about?  isn't this pretty much equivalent to the code you removed in set_up_session_environment?
Comment 3 Mario Sánchez Prada 2017-09-11 08:24:28 UTC
Review of attachment 357808 [details] [review]:

::: daemon/gdm-session.c
@@ +914,3 @@
         GdmSession *self = conversation->session;
 
+        if (strlen (language_name) > 0) {

Yes, that's the main point I think.

@@ +2521,3 @@
+            g_debug ("GdmSession: using saved language %s", self->priv->saved_language);
+            gdm_session_set_environment_variable (self, "LANG", self->priv->saved_language);
+        }

As far as I understand the patch, the main difference is that the user language is now always saved in the previous step, and then used from here directly instead of retrieving it from get_default_language().

That said, I didn't write the patch myself so I might be missing something, or perhaps there's a more reasonable way to do this, but what I can tell is that I test GDM with and without it, and it fixes the issue observed.
Comment 4 Ray Strode [halfline] 2017-09-11 13:45:21 UTC
okay, mind posting a new patch with that hunk removed then? will commit after the release
Comment 5 Mario Sánchez Prada 2017-09-13 10:58:07 UTC
(In reply to Ray Strode [halfline] from comment #4)
> okay, mind posting a new patch with that hunk removed then? will commit
> after the release

You mean removing the call to gdm_session_set_environment_variable (self, "LANG", self->priv->saved_language) and keeping the bits in set_up_session_environment()?
Comment 6 Ray Strode [halfline] 2017-09-13 14:12:07 UTC
i think the only part you actually need is in worker_on_saved_language_name_read right?
Comment 7 Mario Sánchez Prada 2017-09-13 15:19:07 UTC
I'm not 100% sure, to be honest. Let me try that out before proposing a patch and then I'll be able to answer that question :-). But that will have to wait a couple of days, I'm afraid, as I can't context switch to GDM easily right now.

Will try to do it before the end of this week, though!
Comment 8 Mario Sánchez Prada 2017-09-18 16:54:03 UTC
Created attachment 360001 [details] [review]
gdm-session: always pass down user saved language

(In reply to Mario Sánchez Prada from comment #7)
> I'm not 100% sure, to be honest. Let me try that out before proposing a
> patch and then I'll be able to answer that question :-). But that will have
> to wait a couple of days, I'm afraid, as I can't context switch to GDM
> easily right now.
> 
> Will try to do it before the end of this week, though!

I couldn't do it before the end of past week, but I could test this today and I can confirm that your (simpler) suggestion works just as well as the original patch, so here you have an updated patch to review.

Thanks!
Comment 9 Ray Strode [halfline] 2017-09-18 17:08:45 UTC
Review of attachment 360001 [details] [review]:

sure
Comment 10 Mario Sánchez Prada 2017-09-18 17:13:55 UTC
(In reply to Ray Strode [halfline] from comment #9)
> Review of attachment 360001 [details] [review] [review]:
> 
> sure

Thanks. Committed:
https://git.gnome.org/browse/gdm/commit/?id=14cae3f93891e454a8b1a476626fa397dea63d6e