1478930 - negotiateLanguages doesn't prefer ja-JP-mac over ja

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P2)

Description

[Ed Lee :Mardak]

Attachment: ja-JP-mac build showing ja import button text

I was testing out bug 1478183 where with l10n-central, we now have ja-JP-mac strings instead of just ja. But trying out the mac ja build shows it uses the wrong strings. See attachment.

ja:        manual_migration_import_button=今すぐインポート
ja-JP-mac: manual_migration_import_button=今すぐ読み込む

https://hg.mozilla.org/l10n-central/ja/file/tip/browser/chrome/browser/activity-stream/newtab.properties#l161
https://hg.mozilla.org/l10n-central/ja-JP-mac/file/tip/browser/chrome/browser/activity-stream/newtab.properties#l161

We use negotiateLanguages(getAppLocalesAsLangTags(), […, "ja", "ja-JP-mac"])
https://searchfox.org/mozilla-central/rev/d47c829065767b6f36d29303d650bffb7c4f94a3/browser/components/newtab/aboutNewTabService.js#312-315

Here's what I get from using a nightly ja-JP-mac https://download.mozilla.org/?product=firefox-nightly-latest-l10n-ssl&os=osx&lang=ja-JP-mac

Services.locale.getAppLocalesAsLangTags()
Array [ "ja-JP-mac", "en-US" ]

Services.locale.negotiateLanguages(Services.locale.getAppLocalesAsLangTags(), ACTIVITY_STREAM_LOCALES, "en-US")
Array(5) [ "ja", "ja-JP-mac", "en-US", "en-CA", "en-GB" ]

flod, are we passing in the available locales correctly?

In particular, if I flip the available ordering:

Services.locale.negotiateLanguages(Services.locale.getAppLocalesAsLangTags(), ACTIVITY_STREAM_LOCALES.reverse(), "en-US")
Array(5) [ "ja-JP-mac", "ja", "en-US", "en-GB", "en-CA" ]

Also, not sure if the "macos" is expected?

Services.locale.getAvailableLocales()
Array [ "ja-JP-macos", "en-US" ]

Services.locale.getRequestedLocales()
Array [ "ja-JP-macos", "en-US" ]

Comment 1

[Francesco Lodolo [:flod]]

ja-JP-macos is expected and comes out of bug 1450656 (also CCing folks who might be able to better answer questions on the subject).

Comment 2

[Axel Hecht [:Pike]]

So, I think the code should use "Lookup".

That said,

Services.locale.negotiateLanguages(['ja-JP-mac'], ['ja','ja-JP-mac'], 'en-US', Services.locale.langNegStrategyLookup)

returns ['ja']

Services.locale.negotiateLanguages(['ja-JP-macos'], ['ja','ja-JP-macos'], 'en-US', Services.locale.langNegStrategyLookup)

return ['ja-JP-macos']

Zibi? Bug or feature?

Comment 3

[Ed Lee :Mardak]

Ah, I guess Lookup causes it to short circuit early instead of trying to find more, so it makes some sense that Lookup returns "ja" as that's the first thing in the list from my example.

Similarly the reverse trick works too with Lookup:

Services.locale.negotiateLanguages(['ja-JP-mac'], ['ja','ja-JP-mac'].reverse(), 'en-US', Services.locale.langNegStrategyLookup)
Array [ "ja-JP-mac" ]

Comment 4

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

It's a bug. 

The `ja-JP-mac` is an improper BCP47 tag and we handle it slightly less well than a proper equivalent `ja-JP-macos`.

Ed: how much this bothers you? I can try to dig into it if needed and the fix should take less than debugging process will take.
Alternatively, you could switch to use `ja-JP-macos` and use `Services.locale.getAppLocalesAsBCP47();` which will return the proper BCP47 version for you.

Comment 5

[Axel Hecht [:Pike]]

They would need to change their pre-rendering code to serialize ja-JP-mac to a ja-JP-macos path.

How did we fix this for fluent? Or would we end up in a similar problem if we woud ship ja and ja-JP-mac in a single binary?

Comment 6

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

In Fluent we use `getAppLocales` because our negotiated list is *the* list and we have logic in LocaleService to handle the `ja-JP-mac` edge case.

In result if you set your available locales to ["ja", "ja-JP-mac"], and your requested to ["ja-JP-mac"] your `getAppLocalesAsLangTags` will return `["ja-JP-mac", "ja"]` properly.

Comment 7

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

To play the same "one-off" logic here, you'd do:

```
let requested = Services.locale.gfetAppLocalesAsBCP47();
let available = ACTIVITY_STREAM_LOCALES.map(loc => loc == "ja-JP-mac" ? "ja-JP-macos" : loc);
let resolved = Services.locale.negotiateLanguages(requested, available, "en-US").map(loc => loc == "ja-JP-macos" ? "ja-JP-mac" : loc);
```

I know it sucks, but that's kinda what we do in LocaleService - we just always operate/negotiate on `ja-JP-macos` and only convert it to `ja-JP-mac` when asked. In result the negotiation always works with correct BCP47 tag.

What happens here is you try to pass an incorrect BCP47 tag to negotiation algorithm, which parses the `ja-JP-mac` as `ja`, so it ends up with the following structure:

```
{
  "ja": Locale { language: "ja", region: null, variants: [] },
  "ja-JP-mac": Locale { language: "ja", region: null, variants: [] },
}
```

and then it eventually matches it to the requested "ja-JP-mac` (which also gets turned into the same structure), but not before `ja`.

Comment 8

[Ed Lee :Mardak]

A simple workaround is for us to flip the order of our avail list to have ja-JP-mac before ja. This happens to work.. for now…

Switching to ja-JP-macos is a bit more involved as we get the locales from https://hg.mozilla.org/l10n-central/ as well as https://searchfox.org/mozilla-central/source/browser/locales/l10n.toml and both use "ja-JP-mac" so we would need to special case converting to ja-JP-macos in various places. This is needed because we refer to the specific ja-JP-mac string to access the files at https://hg.mozilla.org/mozilla-central/file/tip/browser/components/newtab/prerendered/locales/ja-JP-mac

Comment 9

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

> A simple workaround is for us to flip the order of our avail list to have ja-JP-mac before ja. This happens to work.. for now…

That feels very fragile.

How about my suggestion from comment 7 - flip `mac`->`macos` in what you pass to negotitate and flip back in the output?

Comment 10

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4271

Comment 11

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/c2b255058e0178f2fd9d80b268ce8a9d76c1bc1f
fix(strings): Use BCP47 for negotiation and switch back to lang tags for paths (#4271)

Bug 1478930 - ja-JP-mac builds incorrectly use ja strings

Comment 12

[Ed Lee :Mardak]

So... umm... this is probably a separate bug now, but just a heads up that since 63.0a1 20180728101501, aboutNewTabService.defaultURL should be returning ja-JP-mac when appropriate:

["en", "en-CA", "ja", "ja-JP-mac", "es", "es-MX"].forEach(l => {
  ["Available", "Requested"].forEach(m => Services.locale[`set${m}Locales`]([l]));
  console.log(l, aboutNewTabService.defaultURL);
})

en        resource://activity-stream/prerendered/en-US/activity-stream-prerendered.html
en-CA     resource://activity-stream/prerendered/en-CA/activity-stream-prerendered.html
ja        resource://activity-stream/prerendered/ja/activity-stream-prerendered.html
ja-JP-mac resource://activity-stream/prerendered/ja-JP-mac/activity-stream-prerendered.html
es        resource://activity-stream/prerendered/es-ES/activity-stream-prerendered.html
es-MX     resource://activity-stream/prerendered/es-MX/activity-stream-prerendered.html

(You can try in your own nightly build from browser console setting available and requested locale then opening a new window (avoid preloaded new tab).)

However, if you check the page source of the page or in the web console if you "Persist Logs" and reload the page, the page loaded for ja-JP-mac is still ja…

Here's the logs from loading those same locales above:

Navigated to jar:file:///…/omni.ja!/chrome/browser/res/activity-stream/prerendered/en-US/activity-stream-prerendered.html
Navigated to jar:file:///…/omni.ja!/chrome/browser/res/activity-stream/prerendered/en-CA/activity-stream-prerendered.html
Navigated to jar:file:///…/omni.ja!/chrome/browser/res/activity-stream/prerendered/ja/activity-stream-prerendered.html
Navigated to jar:file:///…/omni.ja!/chrome/browser/res/activity-stream/prerendered/ja/activity-stream-prerendered.html
Navigated to jar:file:///…/omni.ja!/chrome/browser/res/activity-stream/prerendered/es-ES/activity-stream-prerendered.html
Navigated to jar:file:///…/omni.ja!/chrome/browser/res/activity-stream/prerendered/es-MX/activity-stream-prerendered.html

Comment 13

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

Services.locale.negotiateLanguages(['ja-JP-mac'], ['ja','ja-JP-mac'], 'en-US', Services.locale.langNegStrategyLookup)

now returns ja-JP-mac. Closing as fixed. Reopen if needed.