Unify Localization.jsm, mozILocalization and Localization IDL
Categories
(Core :: Internationalization, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox77 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(4 files)
Currently, we have Localization WebIDL backed by C++ which uses Localization.jsm, but in several cases, the JSM is called directly.
The two use different signatures, and in the effort to prepare for migration to Rust (bug 1613705) I'd like to unify that around IDL and stop using JSM anywhere except from within of Localization.cpp.
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 1•5 years ago
|
||
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 4•5 years ago
|
||
The patch is kinda simple - I basically moved all necessary methods to WebIDL, which uses Localization.cpp, and that in turn uses mozILocalization which uses Localization.jsm.
This means that no code and no tests directly use Localization.jsm or mozILocalization, which enables me to later replace it in bug 1613705.
|
|
Assignee | |
Comment 5•5 years ago
|
||
I unified the custom generateBundles and generateBundlesSync to a dictionary which allows for the same easiness when mocking, but eventually will allow for easier plugging of L10nRegistry once its behind WebIDL [0].
[0] https://searchfox.org/mozilla-central/source/intl/l10n/L10nRegistry.jsm#136
|
|
Assignee | |
Comment 6•5 years ago
|
||
treeherder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f6000325015c077f56e5d5fa6ae13ff2599fa75
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a657cda5467e Unify Localization.jsm, mozILocalization and Localization WebIDL. r=smaug https://hg.mozilla.org/integration/autoland/rev/939beec809c1 Update callsites to use Localization WebIDL. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/f197d83fd8a1 Update tests to use Localization WebIDL. r=jfkthame
|
||
Comment 8•5 years ago
|
||
Backed out for perma failures.
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=296643293&repo=autoland&lineNumber=377
Backout: https://hg.mozilla.org/integration/autoland/rev/90e193cc5064eae2004bde7367f2ec77b780b79f
|
||
Comment 10•5 years ago
|
||
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2f7e9c807020 Unify Localization.jsm, mozILocalization and Localization WebIDL. r=smaug https://hg.mozilla.org/integration/autoland/rev/0b2fdf331bce Update callsites to use Localization WebIDL. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/f45de944f20a Update tests to use Localization WebIDL. r=jfkthame,Mardak
|
||
Comment 11•5 years ago
|
||
Backed out 3 changesets (Bug 1621674) for causing xpcshell permafailures in browser/extensions/formautofill/test/unit/test_createRecords.js CLOSED TREE
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=296688042&repo=autoland&lineNumber=4912
|
|
||
Comment 12•5 years ago
|
||
Backout by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/989838363bca Backed out 3 changesets for causing xpcshell permafailures in browser/extensions/formautofill/test/unit/test_createRecords.js CLOSED TREE
|
|
Assignee | |
Comment 13•5 years ago
|
||
|
|
Assignee | |
Comment 14•5 years ago
|
||
The reason it got backed out was because in IDL the UTF8String? means a nullabe string, not string || undefined. And we were pretty inconsistent with undefined vs null in returns from Localization.jsm, so I cleaned it up and now the test passes.
I also switched xpcshell tests for Localization to use stringEqual to make sure we catch any regressions in the null returns, and added a couple tests to capture things like formatMessageSync with a missing element. Unfortunately, one test crashes with mismatched compartment, and I can't figure out why (it uses the same code as DOMLocalization.cpp rooters which don't show this bug and I don't understand why Firefox running in debug or opt doesn't reproduce it). I NI'ed :smaug for that.
:stas - for you I'd just like to verify that for now we're ok coalescing all missing returns to null. (I know it may change once we get to specifying higher level APIs).
|
|
Assignee | |
Comment 15•5 years ago
|
||
Got help from Olli - the issue was that since we call JSAPI from C++ directly we need an autorealm. Added it and now all tests pass.
|
|
Assignee | |
Comment 16•5 years ago
|
||
new try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=700c8a41e1969c8a8a47a40c1200d54556145ed7
|
|
||
Comment 17•5 years ago
|
||
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9800998d06b2 Unify Localization.jsm, mozILocalization and Localization WebIDL. r=smaug https://hg.mozilla.org/integration/autoland/rev/df386249c4f9 Update callsites to use Localization WebIDL. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/c11b34666d70 Update tests to use Localization WebIDL. r=jfkthame,Mardak https://hg.mozilla.org/integration/autoland/rev/e2f6f75b7d26 Settle on returning null for missing values/messages out of Localization. r=smaug,stas
|
||
Comment 18•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/9800998d06b2
https://hg.mozilla.org/mozilla-central/rev/df386249c4f9
https://hg.mozilla.org/mozilla-central/rev/c11b34666d70
https://hg.mozilla.org/mozilla-central/rev/e2f6f75b7d26
Description
•