1423532 - Port |Bug 1414390 - Add intl.locale.requested locale list to replace general.useragent.locale| to TB

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

As per bug 1414390 comment #28 and further down, we need to port the migration code:
https://hg.mozilla.org/mozilla-central/rev/8d88b85d93be#l3.13

We also need to port
-pref("general.useragent.locale", "@AB_CD@");
+pref("intl.locale.requested", "@AB_CD@");

Let's see how much is busted in
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=8cd26835235c2356404eab8643a14d0193ffed58
without it.

Comment 1

[Jorg K (CEST = GMT+2)]

Seems to cause
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_provider_language_wildcard

Comment 2

[Jorg K (CEST = GMT+2)]

INFO -  SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_provider_language_wildcard
INFO -    EXCEPTION: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [mozILocaleService.setRequestedLocales]
INFO -      at: nonesuch line 1169
INFO -         test_provider_language_wildcard test-newmailaccount.js:1169 3

Comment 3

[Jorg K (CEST = GMT+2)]

OK, I've looked into the test failure.

The test does
Services.locale.setRequestedLocales(["foo"]);
https://dxr.mozilla.org/comm-central/rev/492ec0687aad00eb74afb0eaefe6f40d368e3fa3/mail/test/mozmill/newmailaccount/test-newmailaccount.js#1130

and
Services.locale.setRequestedLocales(["foo-bar"]);
https://dxr.mozilla.org/comm-central/rev/492ec0687aad00eb74afb0eaefe6f40d368e3fa3/mail/test/mozmill/newmailaccount/test-newmailaccount.js#1169.

The latter fails.

Zibi says on the subject in bug 1410738 comment #8 that both "foo" and "foo-bar" are valid locales. So I'm not sure why the test fails now, especially why line 1169 fails and not 1130.

Zibi?

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1423532-locale-migrate-TB.patch

This is the required copy/paste job. I'm not sure how to repair the test. Help!

Comment 5

[Richard Marti (:Paenglab)]

https://dxr.mozilla.org/comm-central/source/mail/locales/en-US/all-l10n.js#7 needs also to be removed or not?

Comment 6

[Jorg K (CEST = GMT+2)]

I guess so, I'll do it in the next round.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: 1423532-locale-migrate-TB.patch (v1b)

More noise :-(

Comment 8

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

Comment on attachment 8935095 [details] [diff] [review]
1423532-locale-migrate-TB.patch (v1b)

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

::: mail/app/profile/all-thunderbird.js
@@ +15,1 @@
>  pref("general.skins.selectedSkin", "classic/1.0");

I think you can now remove the `pref("intl.locale.requested", "@AB_CD@");` from all-thunderbird.js. I'm doing this for Firefox in a follow-up in bug 1423618.

Comment 9

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

(In reply to Jorg K (GMT+1) from comment #3)
> OK, I've looked into the test failure.
> 
> The test does
> Services.locale.setRequestedLocales(["foo"]);
> https://dxr.mozilla.org/comm-central/rev/
> 492ec0687aad00eb74afb0eaefe6f40d368e3fa3/mail/test/mozmill/newmailaccount/
> test-newmailaccount.js#1130
> 
> and
> Services.locale.setRequestedLocales(["foo-bar"]);
> https://dxr.mozilla.org/comm-central/rev/
> 492ec0687aad00eb74afb0eaefe6f40d368e3fa3/mail/test/mozmill/newmailaccount/
> test-newmailaccount.js#1169.
> 
> The latter fails.
> 
> Zibi says on the subject in bug 1410738 comment #8 that both "foo" and
> "foo-bar" are valid locales. So I'm not sure why the test fails now,
> especially why line 1169 fails and not 1130.
> 
> Zibi?

It was my fault. I apologize. "foo-bar" is not a BCP47 well-formed language tag as per https://tools.ietf.org/html/bcp47#section-2.1

`foo-ba` is or `und` (commonly used for `undefined` locale) is.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: 1423532-locale-migrate-TB.patch (v1c)

This makes the test pass. It will be PLR in a few hours ;-)

Comment 11

[:aceman]

Comment on attachment 8935108 [details] [diff] [review]
1423532-locale-migrate-TB.patch (v1c)

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

Thanks.

Why doesn't the Firefox patch bump the UI_VERSION?

::: mail/test/mozmill/newmailaccount/test-newmailaccount.js
@@ +1165,5 @@
>   * is not set to "*".
>   */
>  function test_provider_language_wildcard() {
>    let originalReqLocales = Services.locale.getRequestedLocales();
> +  Services.locale.setRequestedLocales(["foo"]);

I would prefer the "foo-ba" version, as "foo" is already tested above.

Comment 12

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #11)
> Why doesn't the Firefox patch bump the UI_VERSION?
Because they already added an upgrade step for 59, so they just added the code into it.

> I would prefer the "foo-ba" version, as "foo" is already tested above.
Yep, bug foo-bar makes the test fail.

Comment 13

[:aceman]

Zibi proposed "foo-ba", not "foo-bar".

Comment 14

[Jorg K (CEST = GMT+2)]

Damn, I missed that. Coming up.

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: 1423532-locale-migrate-TB.patch (v1d)

foo-ba it is ;-)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1dd5743524e5
Port bug 1414390 to TB: Migrate general.useragent.locale to intl.locale.requested. r=aceman

Comment 17

[Jorg K (CEST = GMT+2)]

FRG, maybe it's best for you to do a separate SM bug.

Comment 18

[Frank-Rainer Grahl (:frg)]

No problemo. Needs some migration code and pref changes different from TB anyway.

Comment 19

[Frank-Rainer Grahl (:frg)]

Filed bug 1441016 for SM. 

Btw. I still see the pref listed in calendar/locales/en-US/lightning-l10n.js