1509583 - Migrate region/language names resources to Fluent

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

While we wait for bug 1431324 and bug 1433694 we should migrate language and region names resources to Fluent.

We unified all callsites to use mozIntl, so the migration should be easier.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1509583 - Migrate region/language names resources to Fluent. r?flod,mossop

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

This is not yet a review request, since I need to create a way to load Fluent resources synchronously via L10nRegistry for this feature, but I'd like to get Flod's take on whether the migration looks reasonable.

Comment 3

[Francesco Lodolo [:flod]]

It looks good (once you fix the indentation in migration recipe).

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1509583 - Refactor mozIntl.getRegions to mozIntl.getAvailableLocaleDisplayNames. r?jfkthame,mattn

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1509583 - Introduce LocalizationSync. r?stas

Depends on D13188

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Ok, I ended up introducing both sync L10nRegistry and sync Localization because otherwise I'd have to craft something very similar to sync localization for mozIntl.

I think it looks quite ok and gives us a nice fallback and locale switching from within of the mozIntl (CachedSyncIterable, formatWithFallbackSync etc.).

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

I got all of the tests to pass except of one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=69a3ec6a89e3fe82a25fb59e99362640a5ea326a&selectedJob=219910592

`editor/spellchecker/tests/test_bug1209414.html` fails because it does a funny thing when triggering dictionary selection.

First, it tries to select dictionary based on sorting, rather than value, which is fishy, since it depends on localization (basically making the test fail in a locale where display name of en-US comes after display name de-DE).

But the other thing is that it sorts the list twice - once in a content process and once in a parent process. Since L10nRegistry has no FileSources registered in the child process this fails to sort, and in result we get the sorting `["de (DE)", "en (US)"]` which is the opposite of what the code expects. But the context menu has the labels and seems to be sorted correctly, so I'm not sure why are we sorting in the content process based on display names too...

https://searchfox.org/mozilla-central/source/toolkit/modules/InlineSpellCheckerContent.jsm#33
https://searchfox.org/mozilla-central/source/toolkit/modules/InlineSpellChecker.jsm#212

Blake - you're the last person to touch this code, can you help me understand:

a) why are we sorting it twice?
b) is there a way to make this test select en-US/de-DE based on the actual value, irrelevant of the ordering?

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

I spoke with Zibi on IRC. Currently, the code depends on the order of the dictionary list being the same in the parent and the child because it communicates with an index. It is sorted via the label because we want to display the dictionaries to the user in alphabetical order based on their locale.

That being said, I don't see a reason that we have to pass around error-prone indices between the processes.

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1509583 - Move InlineSpellChecker to sync over locale codes rather than indexes. r?felipe

Depends on D13188

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Felipe - Blake pointed at your as a good reviewer for this patch. It's actually quite simple - I just move us to use localeCode rather than index when communicating selection of the dictionary.

It simplifies multiple things and removes the need to localize locale codes in the content process (so we don't have to load FTL resources for it!)

I sprinkled a couple more nice features like locale-aware comparer just to sweeten the deal :)

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

The cleanest try run you've ever seen - https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ffde3ade80d04e02720da47dfe60f9f77697c80&selectedJob=220067759

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8e2d2010c07
Introduce LocalizationSync. r=stas
https://hg.mozilla.org/integration/autoland/rev/9d7f77b05ae7
Refactor mozIntl.getRegions to mozIntl.getAvailableLocaleDisplayNames. r=jfkthame,MattN
https://hg.mozilla.org/integration/autoland/rev/8d5e2931f526
Move InlineSpellChecker to sync over locale codes rather than indexes. r=Felipe
https://hg.mozilla.org/integration/autoland/rev/ee1c9afb353f
Migrate region/language names resources to Fluent. r=flod,mossop

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/f8e2d2010c07
https://hg.mozilla.org/mozilla-central/rev/9d7f77b05ae7
https://hg.mozilla.org/mozilla-central/rev/8d5e2931f526
https://hg.mozilla.org/mozilla-central/rev/ee1c9afb353f