1374552 - i18n.getUILanguage API doesn't respect i18n.LanguageCode.

Status: RESOLVED FIXED

(WebExtensions :: Compatibility, defect, P3)

Description

[abel lin]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.96 Safari/537.36

Steps to reproduce:

browser.i18n.getUILanguage()  called  in web extension. 


Actual results:

The language code with tag return '_' rather than '-' like en_US, zh_TW, zh_CN.


Expected results:

 It's supposed to be en-US, zh-TW, zh-CN

Comment 1

[Loic]

Bob, you worked on the implementation of chrome.i18n.getUILanguage in bug 1246748, WDYT?

Comment 2

[Bob Silverberg [:bsilverberg]]

The code for the i18n API intentionally converts the dash ("-") to an underscore ("_") as can be seen at [1]. I checked and this is not the same output that one gets from Chrome, which returns the codes with dashes. I cannot remember why we wrote the code that way, but I'm sure there must have been a reason. It does contradict the docs at [2].

Kris, can you remember why we chose to intentionally convert dashes to underscores?

[1] http://searchfox.org/mozilla-central/rev/714606a8145636d93b116943d3a65a6a49d2acf8/toolkit/components/extensions/ExtensionCommon.jsm#1348-1352
[2] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/i18n/LanguageCode

Comment 3

[Bob Silverberg [:bsilverberg]]

It's odd because we did initially implement it that way to match Chrome, but now it seems that Chrome is returning language codes with dashes instead of underscores. Some of the Chrome docs [1] still show language codes with underscores, but getAcceptLanguages and getUILanguage both return codes with dashes, so we should change our methods to match.

[1] https://developer.chrome.com/extensions/i18n

Comment 4

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

When we implemented this API we converted dashes in the language code to underscores because that is what Chrome did.
Chrome no longer does this, so we need to remove the code that does the replacing.

Review commit: https://reviewboard.mozilla.org/r/167554/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/167554/

Comment 5

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8896277 [details]
Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

https://reviewboard.mozilla.org/r/167554/#review172842

I'm surprised Chrome ever did that.  The standard uses dash, Mozilla has a history of using underscore in paths (source code, web sites, etc), so it does lead to confusion.

Comment 6

[Shane Caraveo (:mixedpuppy)]

We should uplift this as far as we can, possibly mark it for ESR as well in case there is an update to it.

Comment 7

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8896277 [details]
Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167554/diff/1-2/

Comment 8

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8896277 [details]
Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167554/diff/2-3/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e371fc35ab59
Fix i18n.getUILanguage so it does not replace dashes with underscores, r=mixedpuppy

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e371fc35ab59

Comment 11

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8896277 [details]
Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

Approval Request Comment
[Feature/Bug causing the regression]: 1246748
[User impact if declined]: The getUILanguage API call will return data in a format that is inconsistent with Google Chrome.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No 
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is a simple change that only affects the i18n WebExtensions API and it is covered by tests.
[String changes made/needed]: None

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8896277 [details]
Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

Fix i18n.getUILanguage API. Beta56+.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cb905f9be690