STR: 1) Install a non-en-US build of Nightly 2) Open about:preferences#privacy in it Expected result: Form autofill checkboxes appear Actual result: No autofill checkboxes and an error in the Browser Console: > NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIStringBundle.GetStringFromName] FormAutofillPreferences.jsm:93 If I try to load the .properties file directly in a tab I get: > Firefox can’t find the file at jar:file:///Applications/FirefoxNightly.app/Contents/Resources/browser/features/formautofill@mozilla.org.xpi!/en-US/locale/en-US/formautofill.properties. but since I'm in an en-CA build that file cannot be found, instead I have: > en-CA/locale/en-CA/formautofill.properties inside that xpi. Bug 1449055 added the first use[1] of `registerChrome`[2] that registers "locale" in shipping code but I think it has a fundamental problem because it doesn't properly handle locale negotiation AFAICT as the caller has to specify the locale code: > this.chromeHandle = aomStartup.registerChrome(manifestURI, [ > … > ["locale", "formautofill", "en-US", "en-US/locale/en-US/"], > ]); It seems like the registerChrome API should do proper locale negotiation. If not, this footgun should be clearly documented for the API so that callers know to do it themselves. [1] https://hg.mozilla.org/mozilla-central/file/tip/browser/extensions/formautofill/api.js#l82 [2] https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/toolkit/mozapps/extensions/AddonManagerStartup.cpp#746,794

This problem only affects Nightly for now since Form Autofill is only available in en-US builds outside of Nightly: https://hg.mozilla.org/mozilla-central/file/tip/browser/extensions/formautofill/api.js#l64

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #0) > Bug 1449055 added the first use[1] of `registerChrome`[2] that registers > "locale" in shipping code but I think it has a fundamental problem because > it doesn't properly handle locale negotiation AFAICT as the caller has to > specify the locale code: > > > this.chromeHandle = aomStartup.registerChrome(manifestURI, [ > > … > > ["locale", "formautofill", "en-US", "en-US/locale/en-US/"], > > ]); The caller always has to enter the locale code when registering locale entries. Locale negotiation happens at resolution time, not at registration time. There can be multiple entries for the same resource with different locales. The registry decides the most appropriate one to used based on the current locale. At least, that's what happens for properties files. The locale service does more complicated things for Fluent translations, but has similar semantics.

Ugh, I certainly overlooked this. I assume that the localized repacks descend into system addon .xpi files and replaces properties files and rewrite corresponding chrome.manifests? Would it be feasible to just move the formautofill strings into toolkit or do we need to be able to ship out-of-cycle string updates?

> I assume that the localized repacks descend into system addon .xpi files and replaces properties files and rewrite corresponding chrome.manifests? Seems like it. > Would it be feasible to just move the formautofill strings into toolkit or do we need to be able to ship out-of-cycle string updates? I would rather not lose the ability to update the whole feature as a system-addon, especially since it doesn't seem that hard to fix this and presumably other system add-ons will hit the same issue.

I don't understand how this should work from the l10n side. Can someone from l10n explain what the solution is or how the extension should register its strings?

Matt - I'm sorry, but I don't understand what's "this" in your previous comment. How should what work? I'm not sure what you're trying to achieve here, and the closest I get is Kris' comment about registration. chrome protocol, and StringBundle API are pretty dumb. They do only the very trivial language negotiation by trying to match the right resource based on the locale of its chrome entry.

How should the form autofill hybrid web extension get localized. What just landed but doesn't work is: > this.chromeHandle = aomStartup.registerChrome(manifestURI, [ > … > ["locale", "formautofill", "en-US", "en-US/locale/en-US/"], > ]); Should we try to make that work by passing in different values to replace "en-US" or is there some better approach? Does Fluent help?

How/why doesn't it work? From comment 0 it seems that the chrome registry did fallback on en-US and tried to load from that path: > Firefox can’t find the file at jar:file:///Applications/FirefoxNightly.app/Contents/Resources/browser/features/formautofill@mozilla.org.xpi!/en-US/locale/en-US/formautofill.properties. but the file was not packaged inside. Is that the cause? IIRC Chrome Registry will do a dummy fallback on en-US [0] if nothing matches, so if your extension provides en-US, and the user expects any other locale, it should fallback on the en-US properly. I'm not sure why the file is not in the xpi? [0] https://searchfox.org/mozilla-central/source/chrome/nsChromeRegistryChrome.cpp#490

Oh, are you sure that was fallback logic or just that en-US was the only locale and path registered? > this.chromeHandle = aomStartup.registerChrome(manifestURI, [ > … > ["locale", "formautofill", "en-US", "en-US/locale/en-US/"], > ]); (In reply to Zibi Braniecki [:gandalf][:zibi] from comment #8) > I'm not sure why the file is not in the xpi? Perhaps a repack bug? Is that the real underlying issue then? I'm not sure how this is supposed to work?

Maybe you'd know if anything related to packaging may interfere? If I understand correctly (It would be helpful to have a description of what the extension is trying to do localization wise), the extension is meant to be en-US only but is packaged into repack builds? And that somehow makes the en-US strings be axed? I'm not sure.

The extension is supposed to work in all locales… just only enabled for en-US outside of Nightly.

What do you mean by "work for all locales, but only enabled for en-US outside of Nightly"?

As Andrew said, this works as designed. The chrome registry is a footgun. Ever since bug 1407528, we repackaged formautofill with localized files. There is a chrome.manifest inside the formautofill xpi in my localized build, and the content in there looks OK, aka, it's pointing to the German resource. Andrew, could we use the `languages` key in manifest.json like we do for langpacks? { "languages": { "de": { "chrome_resources": { "formautofill": "browser/features/formautofill@mozilla.org/de/locale/de/" ... is what we have in language packs right now. If so, we'd need to ask the build folks to add creation/modification of those to the repack scripting. Or, if we want to stick to manually registering chrome, we could try to inspect one of the Service.locale.getFooLocales. Not sure how that deals with startup timing, though.

> Or, if we want to stick to manually registering chrome, we could try to inspect one of the Service.locale.getFooLocales. Not sure how that deals with startup timing, though. Startup will be good - if you just want the packaged, you can do `Services.locale.getPackagedLocales` which will be resolved *very* early and cached permanently. This will give you info on which locales are packaged (based on multilingual.txt) and you can register them for formautofill, I think?

(In reply to Axel Hecht [:Pike] from comment #13) > Andrew, could we use the `languages` key in manifest.json like we do for > langpacks? I may be misunderstanding, but right now, extensions and language packs are both packaged as xpi files with manifest.json declaring their contents. The presence of the "languages" key is exactly how we distinguish extensions from language packs, so we can't use it in an extension. > Or, if we want to stick to manually registering chrome, we could try to > inspect one of the Service.locale.getFooLocales. Not sure how that deals > with startup timing, though. From comment 14, getPackagedLocales sounds promising. If that corresponds exactly to what .properties file is present inside the formautofill extension, then that's exactly what we need.

If I want to test this patch, will following these instructions give me a proper localized build? https://firefox-source-docs.mozilla.org/build/buildsystem/locales.html

There's a risk with packaged locales: It's the list of locales in Firefox, not in the autofill extension. In a scenario where we ship an autofill extension as out-of-band in a subset of languages, it'll break on all languages that are not part of the package. AKA, we'd need to ship all languages. (In reply to Andrew Swan [:aswan] (on PTO until 8/28) from comment #17) > If I want to test this patch, will following these instructions give me a > proper localized build? > https://firefox-source-docs.mozilla.org/build/buildsystem/locales.html Yes. I didn't succeed using this on artifact builds myself, though. Neither the steps in the docs above nor what's recommended in bug 1447932.

(In reply to Axel Hecht [:Pike] from comment #18) > There's a risk with packaged locales: It's the list of locales in Firefox, > not in the autofill extension. > > In a scenario where we ship an autofill extension as out-of-band in a subset > of languages, it'll break on all languages that are not part of the package. > AKA, we'd need to ship all languages. I think this (an out-of-band update needing to include all locales) was already true before the webextension conversion? Matt, have we ever shipped a formautofill update? Is there a playbook or something that details how this is handled? (In reply to Zibi Braniecki [:gandalf][:zibi] from comment #14) > Startup will be good - if you just want the packaged, you can do > `Services.locale.getPackagedLocales` which will be resolved *very* early and > cached permanently. This will give you info on which locales are packaged > (based on multilingual.txt) and you can register them for formautofill, I > think? This appears to not do the right thing here, I did a non-artifact build for en-CA with the instructions linked in comment 17, and in the resulting browser, Services.locale.getPackagedLocales() returns ["en-CA", "en-US"], but the formautofill xpi only contains strings for en-CA (and not en-US) Zibi, is the above the intended behavior of getPackagedLocales()?

We package en-CA and en-US for the fluent files in your en-CA build, so for fluent, it's correct. For chrome://foo/locale, it's not. For chrome://foo/locale, we package a single locale, and if that's the first in getRequestedLocales, registering all packaged ones does the same thing for the chrome registry as registering the actually packaged ones. In the pre-web-extensions days, the list of packaged locales was actually per xpi, defined in chrome.manifest. Or, the chrome registry has getAvailableLocales per package (foo above), the l10n registry doesn't. Obviously, getAvailableLocales is fed by register(), fed by chrome.manifest, so we can't use that here

Now I'm confused about how localized strings in system addons work at all. First of all there's the issue of actually getting a new or updated string translated so quickly. But that is not a technical problem, assuming that gets addressed somehow, how did this work pre-webextensions? All the locales would have to be included in the updated extension, does that extension contain a massive chrome.manifest that enumerates all the locales? I'm now inclined to revisit comment 3, I don't think that even the current system with bootstrap practically allows for out-of-band updates to localized system addons. If we build the strings in to the browser, we could actually deliver an update that doesn't require any string changes. I imagine the translation thing isn't a big issue for formautofill since it is only really supported in one (?) locale. So, in the extreme case if we want to ship an update, an English string could just be embedded in the code in the update. We shouldn't make a habit of doing that of course but it exists as a safety valve. Matt, any thoughts?

(In reply to Andrew Swan [:aswan] from comment #21) > Now I'm confused about how localized strings in system addons work at all. > First of all there's the issue of actually getting a new or updated string > translated so quickly. But that is not a technical problem, assuming that > gets addressed somehow, how did this work pre-webextensions? All the > locales would have to be included in the updated extension, does that > extension contain a massive chrome.manifest that enumerates all the locales? Exactly. Some also use build tools to process .properties into web-extension native json, so they're not doing chrome:// at all.

For fluent in web-extensions, there's patches behind review-board-hardhats right now in bug 1425104. I wonder how kmag and zibi want to do locale discovery there.

Andrew, can you confirm that this only affect the nightly channel and that 63 beta and release channels are not affected?

The underlying issue is not specific to the Nightly channel, but I think we only enable formautofill for specific locales and channels, which might make this not an issue for anything other than Nightly. I forward the question to Matt, who's already needinfo'ed here.

Talked to Matt this morning, a few updates: First about who is affected: on channels other than Nightly, formautofill is only enabled for the en-US locale. Users in other locales on beta could still flip a preference and encounter this, but its not a situation that users will encounter without taking that explicit step. As for actually fixing it, moving to Fluent (bug 1446164) should address this. That effort has a few dependencies that we're not yet tracking, I'll file bugs for them and update that bug momentarily. To unbreak Nightly users in the mean time, I don't have any ideas other than going back to using chrome.manifest (ie with Cm.addBootstrappedManifestLocation) as a stopgap.

marking as fix-optional for 63 per comment #26

Comment on attachment 9011549 [details] Bug 1484325 Go back to chrome.manifest for formautofill locale registration Matthew N. [:MattN] (PM me if requests are blocking you) has approved the revision.

Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/0871a9a488b5 Go back to chrome.manifest for formautofill locale registration r=MattN

Is this something we need on Beta or can we set the status for 63 to wontfix?

formautofill is disabled by default if (channel != "nightly" && locale != "en-US") (sorry for the geeky description but writing it in words kept coming out ambiguously) Users could flip the preference manually to enable it, but I expect that a very very small number of users in locales outside en-US actually do that.

I can confirm that the whole "Forms & Autofill" section is missing from the "about:preferences#privacy" page on Nightly v64.0a1 from 2018-09-24, Italian build version and the error mentioned in comment 0 is displayed in Browser Console. The issue is fixed in Nightly v64.0a1 from 2018-10-08, Italian build version. The issue is now verified. Thank you.