Closed
Bug 1347798
Opened 8 years ago
Closed 7 years ago
Create a small per-document bindings for Localization API
Categories
(Core :: Internationalization, enhancement)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(1 file)
Most of the new localization logic will live in a JS module, but the minimal live bindings will be per-document.
Comment hidden (mozreview-request) |
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8847968 [details]
Bug 1347798 - Create a small per-document bindings for DOMLocalization.
https://reviewboard.mozilla.org/r/120916/#review122932
::: intl/l10n/l20n.js:18
(Diff revision 1)
> + )
> + );
> +}
> +
> +function getResourceLinks(elem) {
> + return Array.prototype.map.call(
I like Array.from with a map function, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from.
Assignee | ||
Updated•8 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
This is the final of the six main patches for the new L10n API! :)
The role of this file is to be loaded into every document that is to be localized using DOMLocalization. This code will polyfill the future WebL10n API by:
- collecting links with rel="localization"
- creating the initial DOMLocalization attached on `document.l10n`
- performing the initial localization of the DOM
- registering observers for the initial DOMLocalization
The one thing it does on top of the list is performing the initial registration of L10nRegistry sources. I don't know where this code should live. In a perfect world I'd imagine that each of the two - browser and toolkit would at some point register its own FileSource and the toolkit's one would be registered first (so that the browser's is newer).
Any other Gecko app like Fennec, Thunderbird, Seamonkey etc. would then register the "toolkit" FileSource and potentially other FileSources per component.
I'm open to suggestions as to where this code should live.
Also, currently, it just takes the locale from AppConstants, but once we land bug 1362617 and start being able to produce multilocale builds, we'll want to switch to this code fetching via async I/O multilocale.json.
Bug 1394977 also would allow me to move that into L10nRegistry because then L10nRegistry.generateContexts would be an async function that could wait for L10nRegistry to be ready.
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8847968 [details]
Bug 1347798 - Create a small per-document bindings for DOMLocalization.
https://reviewboard.mozilla.org/r/120916/#review183454
::: intl/l10n/l10n.js:42
(Diff revision 6)
> + );
> + }
> +
> + const resIds = getResourceLinks(document.head || document);
> +
> + document.l10n = new DOMLocalization(MutationObserver, resIds);
This needs updating for the chnges we made to DOMLocalization.jsm
::: intl/l10n/l10n.js:44
(Diff revision 6)
> + // This is a one-time bootstrapping code for L10nRegistry.
> + if (L10nRegistry.ready === undefined) {
> + const {AppConstants} =
> + Components.utils.import("resource://gre/modules/AppConstants.jsm", {});
> +
> + L10nRegistry.ready = async function() {
> + let locales = [AppConstants.INSTALL_LOCALE];
> +
> + const toolkitSource = new FileSource('toolkit', locales, 'resource://gre/localization/{locale}/');
> + L10nRegistry.registerSource(toolkitSource);
> + const appSource = new FileSource('app', locales, 'resource://app/localization/{locale}/');
> + L10nRegistry.registerSource(appSource);
> + }();
> + }
nsBrowserGlue.js is where I'd put most or all of this.
Attachment #8847968 -
Flags: review?(dtownsend)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
> nsBrowserGlue.js is where I'd put most or all of this.
Excellent! Thank you!
I can see us in the future making it more declarative and store some JSON with build time info about which sources we have in the package, because the `toolkit` one should be initialized by toolkit, not browser, but for now I think it's a good start.
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8847968 [details]
Bug 1347798 - Create a small per-document bindings for DOMLocalization.
https://reviewboard.mozilla.org/r/120916/#review185110
Still not convinced about touchNext but I'm not sure the alternative is any better so lets go with it.
Attachment #8847968 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 13•7 years ago
|
||
Thanks Dave!
I'm going to wait with this patch until 58. It adds a piece of code on the startup path (in nsBrowserGlue) that we're not using in 57, so I think it makes sense to wait :)
Comment 14•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1ae02c8d697
Create a small per-document bindings for DOMLocalization. r=mossop
Backed out for eslint failures like https://treeherder.mozilla.org/logviewer.html#?job_id=132538822&repo=autoland
https://hg.mozilla.org/integration/autoland/rev/c065007b752b62c5af0e82176f4c8183cfb0fa56
Flags: needinfo?(gandalf)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•7 years ago
|
||
Ugh, thank you and apologies for the hassle!
Flags: needinfo?(gandalf)
Comment 18•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e46cfa3b45ff
Create a small per-document bindings for DOMLocalization. r=mossop
Comment 19•7 years ago
|
||
Backed out because it will fail browser-chrome's browser_all_files_referenced.js:
https://hg.mozilla.org/integration/autoland/rev/45a8457d012e1a51531981e88adae9b1cdc7cba1
Previous push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f1ae02c8d697c6ca0c1a24cdadcca304b8a30638&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=132549644&repo=autoland
> TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unreferenced file: chrome://global/content/l10n.js -
> TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unused whitelist entry: resource://gre/modules/DOMLocalization.jsm -
Flags: needinfo?(gandalf)
Comment hidden (mozreview-request) |
Comment 21•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/587c58121d45
Create a small per-document bindings for DOMLocalization. r=mossop
Comment 22•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(gandalf)
You need to log in
before you can comment on or make changes to this bug.
Description
•