Migrate region/language names resources to Fluent

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
5 months ago
3 months ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

5 months ago
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.
(Assignee)

Updated

5 months ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Priority: -- → P3
(Assignee)

Comment 2

5 months ago
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.
Flags: needinfo?(francesco.lodolo)
(Assignee)

Updated

5 months ago
Depends on: 1509609
It looks good (once you fix the indentation in migration recipe).
Flags: needinfo?(francesco.lodolo)
Attachment #9027284 - Attachment description: Bug 1509583 - Migrate region/language names resources to Fluent. → Bug 1509583 - Migrate region/language names resources to Fluent. r?flod
(Assignee)

Comment 6

5 months ago
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.).
Attachment #9028187 - Attachment description: Bug 1509583 - Refactor mozIntl.getRegions to mozIntl.getAvailableLocaleDisplayNames. r?jfkthame,jaws → Bug 1509583 - Refactor mozIntl.getRegions to mozIntl.getAvailableLocaleDisplayNames. r?jfkthame,mattn
Attachment #9027284 - Attachment description: Bug 1509583 - Migrate region/language names resources to Fluent. r?flod → Bug 1509583 - Migrate region/language names resources to Fluent. r?flod,mossop
(Assignee)

Comment 7

4 months ago
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?
Flags: needinfo?(mrbkap)
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.
Flags: needinfo?(mrbkap)
(Assignee)

Comment 10

4 months ago
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 12

3 months ago
Pushed by zbraniecki@mozilla.com:
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
(Assignee)

Updated

3 months ago
Blocks: 1520043
You need to log in before you can comment on or make changes to this bug.