1390822 - Gecko-side strings (Strings.browser.GetStringFromName) aren't updated after a locale change

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Locale switching and selection, defect)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR:
1. Start Mozilla Firefox Mobile
2. Press "Hamburger" button
3. Press "Settings" item in Hamburger menu
4. Press "Privacy" item in Settings menu
5. Enable "Clear private data on exit"
6. and select everything in next menu, except "Downloads" item
7. Press "SET"
8. Pres  <- ("Back") button to go to Settings menu
9. Press "Genera" item in Settings menu
10. Press "Language" item in General menu
11. Select "English (United States)" language
12. Pres  <- ("Back") button to go to General menu
13. Pres  <- ("Back") button to go to Settings menu
14. Pres  <- ("Back") button to go to Firefox tabs
15. Press "Hamburger" button
16. Press "Quit" item in hamburger menu
and see that "Clearing private data..." information is showing as localized one "Czyszczenie prywatnych danych...".
Changing Android language from Polish to English (Unite States) doesn't change anything.

Mozilla Firefox Mobile 55.0 on Android 6.0.1

Comment 1

[Jan Henning [:JanH]]

It looks like Strings.browser.GetStringFromName continues returning whatever locale was initially selected during the browsing session, at least for things handled by our JS chrome code, because this also affects our context menus, the various messages when opening/closing tabs, etc. Things like about:addons, about:firefox, our error messages on the other hand correct themselves after a reload (maybe that's a clue - we don't "reload" our UI code after a locale change), although interestingly enough the preinstalled H264 codec in about:addons continues being localised in whatever locale was initially active even after a reload of about:addons.

Comment 2

[Nevin Chen(Not active on Bugzilla)]

Hi Jan
Nice catch! Do you think we should restart the app when there's a configuration change? 


Hi snorp
Is there a way for Gecko to re-load the locale string?

Comment 3

[Jan Henning [:JanH]]

No, we should fix Strings.browser.GetStringFromName to work properly. I haven't looked how far back this problem goes, but maybe it's still some fallout from the relatively recent-ish locale system changes that also caused some other bugs.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Richard may have some info here too

Comment 5

[Richard Newman [:rnewman]]

(In reply to Jan Henning [:JanH] from comment #3)
> No, we should fix Strings.browser.GetStringFromName to work properly. I
> haven't looked how far back this problem goes, but maybe it's still some
> fallout from the relatively recent-ish locale system changes that also
> caused some other bugs.

I don't know if anyone ever tested locale switching with the clear private data on exit code flow.

browser.js uses thunks to accommodate dynamic locale changes:

  initContextMenu: function () {
    // We pass a thunk in place of a raw label string. This allows the
    // context menu to automatically accommodate locale changes without
    // having to be rebuilt.
    let stringGetter = name => () => Strings.browser.GetStringFromName(name);

    // TODO: These should eventually move into more appropriate classes
    NativeWindow.contextmenus.add(stringGetter("contextmenu.openInNewTab"),

and that _was_ tested: our context menus, last I was involved with Fennec, correctly reflected changes in in-app locale.

If they're no longer doing so, then it seems that somebody regressed GSFN, perhaps in a bug like Bug 1351873. Gandalf is the right person to ask.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Yep, I'll be debugging this tomorrow.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1390822 - Flush StringBundles in Fennec browser.js when ui locale changes.

Review commit: https://reviewboard.mozilla.org/r/177468/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/177468/

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

I regressed it in bug 1346617. Bringing back the flush fixes this issue.

Comment 9

[Richard Newman [:rnewman]]

I have the one-line fix for this in my patch for Bug 1378501. Waiting on hg account reactivation so I can push.

Comment 10

[Richard Newman [:rnewman]]

Fixed in Bug 1378501.