Merge lp:~attente/ubuntu-ui-toolkit/1263163 into lp:ubuntu-ui-toolkit

[William Hua]

Also update LANGUAGE when setLanguage is called for dynamic translation. (LP: #1263163)

Also update LANGUAGE when setLanguage is called for dynamic translation. (LP: #1263163)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:907
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1557/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2178
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2071/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/505
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/505
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/505/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/1902
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2180
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2180/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2071/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1557/rebuild

review: Needs Fixing (continuous-integration)

[David Planella (dpm)]

Which value does lang return?

- If it's something like ca_ES.UTF-8, then it's probably LANG or LC_MESSAGES that would need to be updated
- If it's something like ca, then I guess LANGUAGE might be fine, but with the caveat that we'd be overwriting any values that the user has put there. That should be fine on the phone, but not on the desktop, where a user might have set LANGUAGE to e.g. ca:de:it

LANG and LC_MESSAGES are the standard variables supported everywhere, whereas LANGUAGE is a gettext extension that's not supported by all apps and frameworks.

Also, does just changing the environment variable change the language of the whole UI on the fly? Generally a reboot is required for that.

[William Hua (attente)]

So lang is whatever was passed in by the consumer, as in "i18n.language = <lang>". Since apps can make this assignment generally, it'd probably be better to have some validation in there anyways.

I'm not entirely sure why it works, I'd have to investigate. When the domain is updated, even if it is unchanged, the UI is re-translated using the value of LANGUAGE it seems.

[William Hua (attente)]

David, I think you're right. It's probably better to figure out what is doing the re-translation and doing it manually rather than clobbering LANGUAGE.

[Cris Dywan (kalikiana)]

We intentionally don't set LANGUAGE (anymore) but rely on setlocale to apply the locale. setLanguage in the same code also uses setLocale followed by a languageChanged signal - this is what triggers re-loading the UI, the code listening to that is in modules/Ubuntu/Components/plugin/plugin.cpp .

If your goal is, looking at the bug, to switch the locale of the running app, you should use i18n.language to change it and it will update automatically.

review: Needs Fixing

[William Hua (attente)]

Hi Christian, thanks for explaining the languageChanged signal, but setting i18n.language still doesn't seem to trigger dynamic translations with the current trunk.

I suspect calling setlocale isn't setting LANGUAGE at all, so it remains the same value, and gettext prioritizes that value of LANGUAGE over LANG and LC_*, so that the subsequent calls to i18n.tr() after languageChanged is emitted still use the old LANGUAGE.

In this MP, doing setenv("LANGUAGE", ...) only clobbers the variable in the current process, so it seems harmless as we're not actually changing the user's permanent LANGUAGE.

[David Planella (dpm)]

The question is also, why is LANGUAGE set in the system in the first place?

If I'm not mistaken, on the desktop we don't set LANGUAGE unless the user explicitly does it, and on the phone, it being set might be legacy from when we just hardcoded it to English?

So if we don't set LANGUAGE on the phone (I don't think we need it anywhere, as long as LANG or LC_MESSAGES are set) would then setting setting i18n.language "just work"?

> In this MP, doing setenv("LANGUAGE", ...) only clobbers the variable in the current process, so it seems harmless as we're not actually changing the user's permanent LANGUAGE.

Does that mean that language change on-the-fly will only work for the System Settings app, but not for the rest of the UI, which would require a reboot or killing unity8?

[Cris Dywan (kalikiana)]

The use case of LANGUAGE is to enable multiple languages at once, for example sv:de if you speak Swedish and German. It is indeed not meant to be set by setlocale because that would break the functionality.

[William Hua (attente)]

It seems that /usr/share/language-tools/save-to-pam-env is setting the value of LANGUAGE in the user's .pam_environment. We can try removing that, but we still have the issue that if the user's LANGUAGE is ever set manually, dynamic language switching will not work. As far as I can see, there is no way to make gettext() calls for a specific language, which means we would have to set the variable manually to get it to work...

David, yes, doing setenv("LANGUAGE", ...) will only affect the one process that it is called in. My plan was to add a handler to detect when the user's accountsservice language changes, and set i18n.language whenever that happens. I believe this would allow all apps to re-translate when the language is changed in ubuntu-system-settings, but again, that behaviour would depend on this getting merged.

[William Hua (attente)]

Some of the solutions I've seen online temporarily set the LANGUAGE while doing the call to gettext() and reset it after they're done. I could propose this instead...

[Cris Dywan (kalikiana)]

The blindfold has just fallen off… William, you're right. It makes sense to set LANGUAGE. The whole time I was focussed on finding a problem with re-loading and updating components when simply the old ones were loaded again.

Could you replace the bug reference in the comment with an explicit reasoning? Something like this:

/*
 LANGUAGE may be set to one one or more languages for example "fi" or "sv:de".
 Override it here so that setlocale won't stick to the old locale.
*/

And please do a 'bzr merge --pull lp:ubuntu-ui-toolkit' for good measure, then I think we're good.

[William Hua (attente)]

Thank you, Christian! I'll try the accountsservice language changes, and see if that gets us dynamic translation device-wide without too much of a performance impact.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:909
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1626/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2562
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2365
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/574
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/574
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/574/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2238
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2564
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2564/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2366
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2366/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4808
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3298

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1626/rebuild

review: Approve (continuous-integration)

[Cris Dywan (kalikiana)]

Nice!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-autolanding/569/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2589
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2380
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-autolanding/172/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-autolanding/172
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-autolanding/172/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2263
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2591
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2591/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2381
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2381/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4824
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3321

review: Needs Fixing (continuous-integration)

[Cris Dywan (kalikiana)]

Unrelated failure:

FAIL! : components::DatePickerAPI::test_4_linearSecondsPicker() day picker should be linear
   Actual (): true
   Expected (): false
   Loc: [/tmp/buildd/ubuntu-ui-toolkit-0.1.46+14.04.20131216/tests/unit_x11/tst_components/tst_datepicker.qml(627)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/plugin/i18n.cpp'
--- modules/Ubuntu/Components/plugin/i18n.cpp	2013-11-19 17:26:04 +0000
+++ modules/Ubuntu/Components/plugin/i18n.cpp	2014-01-23 15:15:36 +0000
@@ -113,6 +113,21 @@
 
 void UbuntuI18n::setLanguage(const QString &lang) {
     m_language = lang;
+
+    /*
+     This is needed for LP: #1263163.
+
+     LANGUAGE may be set to one or more languages for example "fi" or
+     "sv:de". gettext prioritizes LANGUAGE over LC_ALL, LC_*, and
+     LANG, so if the session has already set LANGUAGE, calls to
+     gettext will only use that.  We must override it here so that
+     future calls to gettext are done in the new language.
+
+     This only affects the current process. It does not override the
+     user's session LANGUAGE.
+     */
+    setenv("LANGUAGE", lang.toUtf8().constData(), 1);
+
     /*
      The inverse form of setlocale as used in the constructor, passing
      a valid locale string updates all category type defaults.