Closed Bug 1586216 Opened 4 years ago Closed 4 years ago

Translations from language pack are not fully loaded in firefox 70.0

Categories

(Core :: Internationalization, defect)

Product:
Core
Component:
Internationalization
Version:
70 Branch
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 blocking verified
firefox71 + verified

People

(Reporter: olivier, Assigned: mossop)

References

Details

(Keywords: regression, Whiteboard: [rca: Coding Error])

Attachments

(1 file)

Bug 1586216: Fallback to a synchronous channel load if the url preloader cannot load the ftl file. r=kmag
47 bytes, text/x-phabricator-request
lizzard
: approval-mozilla-beta+
Details | Review

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.

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.

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.

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

Component: Untriaged → Preferences

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(gandalf)

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).

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

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?

Flags: needinfo?(gandalf) → needinfo?(dtownsend)

[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.

status-firefox69: --- → unaffected
status-firefox70: --- → affected
status-firefox71: --- → affected
tracking-firefox70: --- → ?
tracking-firefox71: --- → ?

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

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.

Flags: needinfo?(francesco.lodolo)

(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

Flags: needinfo?(francesco.lodolo)

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?

Flags: needinfo?(kmaglione+bmo)

(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.

Flags: needinfo?(dtownsend)

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.

(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).

(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.

Flags: needinfo?(kmaglione+bmo)

(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).

Component: Preferences → Internationalization
Product: Firefox → Core

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.

Flags: needinfo?(m_kato)
Severity: normal → major
Assignee: nobody → dtownsend
Flags: needinfo?(m_kato)

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

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.

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

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

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

Looks like it.

Pushed by dtownsend@mozilla.com:
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

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!

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

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox71: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

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
Attachment #9099938 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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

QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
status-firefox71: fixed → verified
Flags: qe-verify+

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

QA Whiteboard: [qa-triaged]
Flags: qe-verify+

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

Keywords: regression

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.

Attachment #9099938 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

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

QA Whiteboard: [qa-triaged]
status-firefox70: fixed → verified
Flags: qe-verify+

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.

Keywords: rca-needed

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.

Keywords: rca-needed
Whiteboard: [rca: Coding Error]
Regressions: 1621024
You need to log in before you can comment on or make changes to this bug.