Closed Bug 1374552 Opened 2 years ago Closed 2 years ago

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

Categories

(WebExtensions :: Compatibility, defect, P3)

55 Branch
defect

Tracking

(firefox56 fixed, firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: j-linjinan1, Assigned: bsilverberg)

Details

Attachments

(1 file)

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
Bob, you worked on the implementation of chrome.i18n.getUILanguage in bug 1246748, WDYT?
Component: General → WebExtensions: Untriaged
Flags: needinfo?(bob.silverberg)
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
Flags: needinfo?(kmaglione+bmo)
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
Assignee: nobody → bob.silverberg
Status: UNCONFIRMED → NEW
Component: WebExtensions: Untriaged → WebExtensions: Compatibility
Ever confirmed: true
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(bob.silverberg)
Priority: -- → P3
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.
Attachment #8896277 - Flags: review?(mixedpuppy) → review+
We should uplift this as far as we can, possibly mark it for ESR as well in case there is an update to it.
Pushed by bsilverberg@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e371fc35ab59
Fix i18n.getUILanguage so it does not replace dashes with underscores, r=mixedpuppy
https://hg.mozilla.org/mozilla-central/rev/e371fc35ab59
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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
Attachment #8896277 - Flags: approval-mozilla-beta?
Comment on attachment 8896277 [details]
Bug 1374552 - Fix i18n.getUILanguage so it does not replace dashes with underscores,

Fix i18n.getUILanguage API. Beta56+.
Attachment #8896277 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.