Closed Bug 1621674 Opened 6 months ago Closed 5 months ago

Unify Localization.jsm, mozILocalization and Localization IDL

Categories

(Core :: Internationalization, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

(Blocks 1 open bug)

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: nobody → gandalf
Blocks: 1613705
Status: NEW → ASSIGNED
Priority: -- → P2
Summary: Unify Localization.jsm and Localization IDL → Unify Localization.jsm, mozILocalization and Localization IDL
Attachment #9132786 - Attachment description: Bug 1621674 - [WIP] Unify Localization.jsm, mozILocalization and Localization WebIDL. → Bug 1621674 - Unify Localization.jsm, mozILocalization and Localization WebIDL.

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.

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

Blocks: 1627809
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

We need to update RemoteL10n.test.js.

Flags: needinfo?(gandalf)
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
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

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

Flags: needinfo?(gandalf)

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.

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
See Also: → 1629275
You need to log in before you can comment on or make changes to this bug.