1586216 - Translations from language pack are not fully loaded in firefox 70.0

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Olivier Tilloy]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

1. Download firefox 70.0 beta 11 (default English locale)
2. Remove ~/.mozilla/firefox folder
3. Unpack and run firefox, observe that the UI is in English as expected
4. Go to about:preferences, "Language" section, search for more languages and install the French langpack and make French the default language
5. Restart firefox

Actual results:

Most of the UI elements are localized in French, as expected, but some are not. The elements that are not localized include the about:preferences page, the "What's New" popover and the "About Firefox" dialog.

Expected results:

All UI elements should be localized in French, with no exception.

Comment 1

[Olivier Tilloy]

Note that this is a regression in Firefox 70. Firefox 69.0.2 works as expected in that regard.

Also note that after disabling and enabling again the French language pack in about:addons, then refreshing the tab with about:preferences, it is correctly localized in French, and so is the "What's New" popover. The "About Firefox" dialog remains in English though.

Comment 2

[Olivier Tilloy]

This bug was initially reported against the Firefox Ubuntu beta packages (https://launchpad.net/bugs/1846371), but it turns out it's not distribution-specific.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 4

[Francesco Lodolo [:flod]]

Thanks for the detailed report, I can confirm the behavior on 70.0b11 (64 bit).

@zibi
Did anything change in the caching system? That's all Fluent elements, which should even change without a restart.

Comment 5

[Francesco Lodolo [:flod]]

Even starting with -purgecaches doesn't help.

Requested locales is correct ["fr","it"], but even about:support stays in the language of the build (Italian in my case).

Comment 6

[Francesco Lodolo [:flod]]

Disabling and re-enabling the language packs seems to trigger something, and things start to get localized as expected.

Comment 7

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

Mossop - I remember you saying that we do purge XUL cache on language change, I think this bug indicates that we don't? Or is there something else in play?

Comment 8

[Francesco Lodolo [:flod]]

[Tracking Requested - why for this release]: It's very late, but we're providing users with language packs a very poor experience.

I'm trying to run mozregression to identify the bug causing this, but it's the most painful process imaginable.

Comment 9

[Francesco Lodolo [:flod]]

I can't narrow down further, because all language packs on FTP for that day cause a YSOD.

@zibi
The only relevant bug seems to be the one migrating menubar to Fluent (it does touch L10nRegistry.jsm, no clue if that could be the cause of this)
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e78733e0ffcfe52903aba0a02952cd800b89ca8d&tochange=ff0125384d06fefae3ca60435b85a9be0fd9744e

Comment 10

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

Can you revert that change in L10nRegistry.jsm and see if you can still reproduce? I doubt that this is the culprit because this change doesn't really affect anything except of protecting against race between sync/async.

Comment 11

[Francesco Lodolo [:flod]]

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #10)

Can you revert that change in L10nRegistry.jsm and see if you can still reproduce? I doubt that this is the culprit because this change doesn't really affect anything except of protecting against race between sync/async.

Artifact build from current mozilla-central tip

1. Build and run
2. Create a new profile
3. In about:config set intl.multilingual.enabled to true
4. Install French language pack from the latest mozilla-central-l10n available, luckily there might not be new DTD strings loaded on startup…. In my case this worked
   http://archive.mozilla.org/pub/firefox/nightly/2019/10/2019-10-04-09-46-56-mozilla-central-l10n/linux-x86_64/xpi/
5. Open preferences, switch the language to French and restart. At this point, the browser should reopen on Preferences in French, but they're in English, even after reloading the page.

I tried undoing the changes to L10nRegistry.jsm, but it didn't solve the issue. But removing data-l10n-sync="true" from browser/base/content/browser.xhtml seems to do it.
https://hg.mozilla.org/integration/autoland/rev/1f622156e9eab1d2c58d5d77bb5908fcc83db253#l4.13

Comment 12

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

thank you! My best guess is that bringing Fluent to browser.xhtml causes Fluent to kick-off earlier than langpacks are registered.

Unfortunately, I'll be on PTO next week, which is unfortunate if we want to take it into 70 :(

Kris - Does it sound plausible? can we make langpacks register prior to browser.xhtml?

Comment 13

[Dave Townsend [:mossop]]

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #7)

Mossop - I remember you saying that we do purge XUL cache on language change, I think this bug indicates that we don't? Or is there something else in play?

If starting with -purgecaches isn't helping then I don't think the startup cache is a problem here.

Comment 14

[Dave Townsend [:mossop]]

That said ... we don't flush the startup cache when the locales change, but we do flush the xul prototype cache: https://searchfox.org/mozilla-central/source/chrome/nsChromeRegistryChrome.cpp#201 which means whatever this is caching is not getting flushed (thought -purgecaches would flush it so maybe a red herring): https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ExtensionParent.jsm#2004.

From what I can see registering a new source should be causing the available locales to change and so the prototype cache to get flushed: https://searchfox.org/mozilla-central/source/intl/l10n/L10nRegistry.jsm#198. It should also cause us to retranslate any documents: https://searchfox.org/mozilla-central/source/intl/l10n/Localization.cpp#. I wonder what order those happen in though.

Comment 15

[Olivier Tilloy]

(In reply to Francesco Lodolo [:flod] from comment #11)

I tried undoing the changes to L10nRegistry.jsm, but it didn't solve the issue.

I can confirm that undoing the changes to L10nRegistry.jsm doesn't fix the problem (tested by building Ubuntu packages with that change reverted).

Comment 16

[Kris Maglione [:kmag]]

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #12)

thank you! My best guess is that bringing Fluent to browser.xhtml causes Fluent to kick-off earlier than langpacks are registered.
...
Kris - Does it sound plausible? can we make langpacks register prior to browser.xhtml?

No, langpacks are initialized about as early as it's possible for anything that requires the profile to be initialized, and the process is entirely synchronous:

https://searchfox.org/mozilla-central/rev/2f29d53865cb895bf16c91336cc575aecd996a17/toolkit/components/extensions/Extension.jsm#2501-2534

Unless the langpacks aren't already installed at startup, anyway. In that case, they'd become available whenever they happen to be detected/installed. But after the next restart, they should still always be available immediately at startup.

Comment 17

[Olivier Tilloy]

(In reply to Francesco Lodolo [:flod] from comment #11)

But removing data-l10n-sync="true" from browser/base/content/browser.xhtml seems to do it.
https://hg.mozilla.org/integration/autoland/rev/1f622156e9eab1d2c58d5d77bb5908fcc83db253#l4.13

And I can confirm that removing data-l10n-sync="true" from browser/base/content/browser.xhtml fixes the issue for me (tested by building Ubuntu packages with that change reverted).

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Is there anyone who can work on this for 70? This sounds like a serious problem so I'm concerned since we're at the end of the beta cycle very soon.

Comment 19

[Dave Townsend [:mossop]]

Figured this out. Having some unrelated build issues right now but should have a patch up tomorrow.

Comment 20

[Dave Townsend [:mossop]]

So the main problem here is that as things stand synchronously loading ftl files from langpacks fail. Synchronous loads use Cu.readUTF8URI which underneath uses the URLPreloader which only supports file or omnijar urls. So attempting to load strings for browser.xhtml which uses synchronous localization fails for the langpack and so falls back to the built-in localization (yay for fallback!).

Unfortunately we cache these load failures which effectively poison the langpack for future localization passes. In particular we cache the fact that branding/brand.ftl isn't available in the langpack and then any attempt to generate bundles that include branding/brand.ftl as a resource (which the majority of pages do) fail because that file is cached as missing. That seems like a different bug, I had the impression that fluent would fall back on a per-resource level, not ignore the entire locale if just one resource is missing, but we don't need to solve that to fix this.

There are a couple of ways to solve this. Probably the ideal solution would be to make the URLPreloader support loading out of langpacks. But for a safer patch for beta I've just made loadSync fallback to an alternative method of loading the ftl file in the event that Cu.readUTF8URI fails.

Comment 21

[Dave Townsend [:mossop]]

Attachment: Bug 1586216: Fallback to a synchronous channel load if the url preloader cannot load the ftl file. r=kmag

Comment 22

[Axel Hecht [:Pike]]

Ah, another one of those. Is this also gonna fix bug 1557713 ?

Comment 23

[Dave Townsend [:mossop]]

(In reply to Axel Hecht [:Pike] from comment #22)

Ah, another one of those. Is this also gonna fix bug 1557713 ?

Looks like it.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee1a9bce6d9e
Fallback to a synchronous channel load if the url preloader cannot load the ftl file. r=kmag

Comment 25

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

Yeah, one of the main themes of bug 1462839 is to handle missing resources. The Crux is to find a sweet spot between loading "missing" contexts and skipping "mostly ok" ones.

I hope someone will tackle that soon since it hinders us.

Thank you Dave for putting works to solve this bug!

Comment 26

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/ee1a9bce6d9e

Comment 27

[Dave Townsend [:mossop]]

Comment on attachment 9099938 [details]
Bug 1586216: Fallback to a synchronous channel load if the url preloader cannot load the ftl file. r=kmag

Beta/Release Uplift Approval Request

• User impact if declined: Users with langpacks will see parts of the UI not translated into their preferred locale.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: We're unlikely to see langpack users in nightly so worth a manual pass.
  See https://bugzilla.mozilla.org/show_bug.cgi?id=1586216#c0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Adds a fallback loading mechanism, worst case that fails too and we are still in the same situation but manual tests show that it works.
• String changes made/needed: None

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Yay, thank you! This will make it into the RC build on Monday.

Comment 29

[Daniel Kolsis]

Verified - Fixed - checked on Firefox Nightly 71.0a1 (2019-10-10) on Ubuntu 18.04. The about:preferences, "What's New" and "About Firefox" areas are now properly localized.

Comment 30

[Daniel Kolsis]

Sorry, i have removed the [qa-verified+] by accident.

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 32

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9099938 [details]
Bug 1586216: Fallback to a synchronous channel load if the url preloader cannot load the ftl file. r=kmag

Fix for release blocking issue, OK for beta uplift.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7b5b4eed47071acfcb0346bab29d8ffaa3d2054f

Comment 34

[Dave Townsend [:mossop]]

I filed bug 1588550 on getting the URLPreloader to support language packs properly.

Comment 35

[Daniel Kolsis]

Verified Fixed - Checked on Release 70 - the language is swapped in all areas.

Comment 36

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This bug has been identified as part of a pilot on determining root causes of blocking and dot release drivers.

It needs a root-cause set for it. Please see the list at https://docs.google.com/document/d/1FFEGsmoU8T0N8R9kk-MXWptOPtXXXRRIe4vQo3_HgMw/.

Add the root cause as a whiteboard tag in the form [rca - <cause> ] and remove the rca-needed keyword.

If you have questions, please contact :tmaity.

Comment 37

[Dave Townsend [:mossop]]

I think this falls under a coding error. When implementing the sync localization support in bug 1509609 the API chosen to load the flt file did not support language packs.