Translations from language pack are not fully loaded in firefox 70.0
Categories
(Core :: Internationalization, defect)
Tracking
()
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)
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:
- Download firefox 70.0 beta 11 (default English locale)
- Remove ~/.mozilla/firefox folder
- Unpack and run firefox, observe that the UI is in English as expected
- Go to about:preferences, "Language" section, search for more languages and install the French langpack and make French the default language
- 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.
Reporter | ||
Comment 1•6 years ago
|
||
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.
Reporter | ||
Comment 2•6 years ago
|
||
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•6 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
Comment 4•6 years ago
|
||
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•6 years ago
|
||
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•6 years ago
|
||
Disabling and re-enabling the language packs seems to trigger something, and things start to get localized as expected.
Comment 7•6 years ago
|
||
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•6 years ago
|
||
[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•6 years ago
|
||
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
Updated•6 years ago
|
Comment 10•6 years ago
|
||
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•6 years ago
|
||
(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
- Build and run
- Create a new profile
- In
about:config
setintl.multilingual.enabled
to true - 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/ - 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•6 years ago
|
||
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?
Assignee | ||
Comment 13•6 years ago
|
||
(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.
Assignee | ||
Comment 14•6 years ago
|
||
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.
Reporter | ||
Comment 15•6 years ago
|
||
(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•6 years ago
|
||
(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:
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.
Reporter | ||
Comment 17•6 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #11)
But removing
data-l10n-sync="true"
frombrowser/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).
Updated•6 years ago
|
Comment 18•6 years ago
|
||
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.
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 19•6 years ago
|
||
Figured this out. Having some unrelated build issues right now but should have a patch up tomorrow.
Assignee | ||
Comment 20•6 years ago
|
||
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.
Assignee | ||
Comment 21•6 years ago
|
||
Comment 22•6 years ago
|
||
Ah, another one of those. Is this also gonna fix bug 1557713 ?
Assignee | ||
Comment 23•6 years ago
|
||
(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•6 years ago
|
||
Comment 25•6 years ago
|
||
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•6 years ago
|
||
bugherder |
Assignee | ||
Comment 27•6 years ago
|
||
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
Assignee | ||
Updated•6 years ago
|
Comment 28•6 years ago
|
||
Yay, thank you! This will make it into the RC build on Monday.
Updated•6 years ago
|
Comment 29•6 years ago
|
||
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•6 years ago
|
||
Sorry, i have removed the [qa-verified+] by accident.
Comment 31•6 years ago
|
||
Bugbug thinks this bug is a regression, but please revert this change in case of error.
Comment 32•6 years ago
|
||
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.
Updated•6 years ago
|
Comment 33•6 years ago
|
||
Assignee | ||
Comment 34•6 years ago
|
||
I filed bug 1588550 on getting the URLPreloader to support language packs properly.
Comment 35•5 years ago
|
||
Verified Fixed - Checked on Release 70 - the language is swapped in all areas.
Comment 36•5 years ago
|
||
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.
Assignee | ||
Comment 37•5 years ago
|
||
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.
Description
•