1440235 - Port |Bug 1431260 - Migrate LocaleService::AvailableLocales from ChromeRegistry to multilocale.json|

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement, P3)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1431260 +++

As per bug 1431260 comment # we should add
const { L10nRegistry, FileSource } = ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm", {});

and

+    let locales = Services.locale.getPackagedLocales();
+    const appSource = new FileSource("app", locales, "resource://app/localization/{locale}/");
+    L10nRegistry.registerSource(appSource);

to `final-ui-startup` - https://dxr.mozilla.org/comm-central/rev/91311b1d6aa3bddd427858cb7f8c149a81c71a92/mail/components/mailGlue.js#58

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1440235-L10nRegistry.patch (v1)

This should do it. Note that final-ui-startup calls _onProfileStartup(), so I put the new code there.

So this should work after bug 1431260 lands?

Comment 2

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

Comment on attachment 8952986 [details] [diff] [review]
1440235-L10nRegistry.patch (v1)

Review of attachment 8952986 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, lgtm! I left a comment about the path but it's up to your discretion.

Thank you for a quick reaction! :)

::: mail/components/mailGlue.js
@@ +88,5 @@
>    _onProfileStartup: function MailGlue__onProfileStartup() {
>      TBDistCustomizer.applyPrefDefaults();
>  
> +    let locales = Services.locale.getPackagedLocales();
> +    const appSource = new FileSource("app", locales, "resource://app/localization/{locale}/");

My knowledge of comm-central is limited, but I don't see there any `resource://app/` used.

I know it's just a mock for now, but since we're landing it, maybe we should land it to already get ready for future `/localization/` directory being available?

What I found scanning comm-central is `resource:///` paths, which in Firefox is an equivalent of `resource://app/` and I imagine in Thunderbird it'll be a Tb specific omni.ja?

I found this: https://searchfox.org/comm-central/search?q=resource%3A%2F%2F%2F&case=false&regexp=false&path=

So, I think the path here should be `resource:///localization/{locale}/`

Comment 3

[Jorg K (CEST = GMT+2)]

Looks like nothing broke so for by not landing this, so this slipped a bit off the radar.

Tom, Philipp, any opinion about what the path should be?

Comment 4

[Tom Prince [:tomprince]]

I haven't wrapped my head around `resouces://` paths, so I don't have an informed opinion.

Comment 5

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Calendar l10nregistry - v1

I'd suggest using resource:///chrome/{locale}/locale/{locale}/ for mail, or just the chrome:// uri. I've successfully been able to use ftl in calendar with an url like this. Please also see attached patch for calendar.

Comment 6

[[:MakeMyDay]]

Comment on attachment 8965948 [details] [diff] [review]
Calendar l10nregistry - v1

Review of attachment 8965948 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=me

Comment 7

[Jorg K (CEST = GMT+2)]

Hmm, for calendar you're using
let fs = new FileSource("calendar", locales , "resource://calendar/chrome/calendar-{locale}/locale/{locale}/");
and for mail you suggest:
let fs = new FileSource("app", locales , "resource:///chrome/{locale}/locale/{locale}/");

That doesn't quite correspond. What am I missing?

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

Each product is slightly different, you can openDialog("resource:///") to browse the app resource uri namespace. Obviously we need to find the folder with the localization files in it, not just use the exact same url as in calendar.

Comment 9

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

The basic idea is to have one FileSource per omni.ja package (in Firefox we have two omni.ja, one for gre, one for browser).

Comment 10

[Jorg K (CEST = GMT+2)]

Right, openDialog("resource:///chrome/en-US/locale/en-US/"); works, thanks!

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1440235-L10nRegistry.patch (v2)

Here we go. Should we use "app" or something else?

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8965995 [details] [diff] [review]
1440235-L10nRegistry.patch (v2)

Review of attachment 8965995 [details] [diff] [review]:
-----------------------------------------------------------------

"app" sounds good to me, this matches what Firefox uses.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/31c55b1e1d59
Port bug 1431260 to TB [Migrate LocaleService::AvailableLocales from ChromeRegistry to multilocale.json]. r=philipp
https://hg.mozilla.org/comm-central/rev/7a0e2dff1db7
Port bug 1431260 to TB [Migrate LocaleService::AvailableLocales from ChromeRegistry to multilocale.json], Calendar part. r=MakeMyDay

Comment 14

[Jorg K (CEST = GMT+2)]

Care to fix the linting errors?
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/src/calStartupService.js:76:21 | 'locales' is already declared in the upper scope. (no-shadow)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/src/calStartupService.js:77:21 | Identifier name 'fs' is too short (< 3). (id-length)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/src/calStartupService.js:77:61 | There should be no space before ','. (comma-spacing)

Comment 15

[[:MakeMyDay]]

Attachment: FixLintingErrorsIncalStartupService-V1.diff

This should take care.

Comment 16

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8966051 [details] [diff] [review]
FixLintingErrorsIncalStartupService-V1.diff

Review of attachment 8966051 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm. Bonus points for making it in under 100 characters per line, I didn't count though.

Comment 17

[[:MakeMyDay]]

Attachment: FixLintingErrorsIncalStartupService-V2.diff

Thanks, here you go.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/74d0ddb667fd
Fix linting errors in calStartupService.js. r=philipp