Closed Bug 1106119 Opened 7 years ago Closed 6 years ago

Bad detection of the selected locale for hyphenation rules of labels in Panel UI

Categories

(Firefox :: Menus, defect)

x86
All
defect
Not set
normal
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 41
Iteration:
41.1 - May 25
Tracking Status
firefox41 --- fixed

People

(Reporter: nohamelin, Assigned: Gijs)

References

Details

Attachments

(1 file, 1 obsolete file)

In:
http://lxr.mozilla.org/mozilla-central/source/browser/components/customizableui/content/panelUI.js?rev=6071fbf83d63#513

the getLocale() function doesn't take in account neither the intl.locale.matchOS preference nor the -UILocale command-line parameter to determine the current locale. With it the wrong hyphenation rules can be applied to the labels of the toolbar buttons inside the menu panel.

Steps to reproduce:
* Ensure the general.useragent.locale pref is set to "en-US"
* Install the es-ES langpack
* Close Firefox, and start it with the -UILocale flag
    ./firefox -UILocale es-ES
* You get the UI in Spanish, but the label of the Sidebars button is "Barras lat- erales" (wrong) instead of "Barras latera- les".
Add support for the intl.locale.matchOS preference is a direct thing, but, as far as I understand the implementation code of the nsChromeRegistryChrome interface, it's not possible to know directly the  locale set initially with -UILocale. Maybe is better to have a new more general bug about the latter.
Blocks: 983583
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Carlos [:nohamelin] from comment #1)
> Add support for the intl.locale.matchOS preference is a direct thing, but,
> as far as I understand the implementation code of the nsChromeRegistryChrome
> interface, it's not possible to know directly the  locale set initially with
> -UILocale. Maybe is better to have a new more general bug about the latter.

Are you saying that the chrome registry's locale for package thing doesn't reflect the -UILocale switch? So if I start Firefox with a profile where the general.useragent.locale pref is fr-FR, but I pass -UILocale es-ES and I ask:

chromeRegistry.getSelectedLocale("browser");

it will return fr-FR instead of es-ES ?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(nohamelin)
No, getSelectedLocale() from the chrome registry works as expected, but here the direct value of the preferred locale is set for the menu panel, not caring about its availability, causing then a mismatch. 

Actually, there are at least two diferent cases where the current situation can go wrong:

 - When the application locale happens to be determined from *general.useragent.locale*, but it corresponds to an unavailable locale: chromeRegistry returns whatever locale is available, locale that is finally used for the browser, but the menu panel have set the original unavailable locale --> wrong hyphenation rules.
 - When the locale is determined from -UILocale (or intl.locale.matchOS): chromeRegistry returns that locale, as expected, that is used for the browser, but the menu panel have set the value of *general.useragent.locale* --> wrong hyphenation rules again.

It seems that using directly the value returned by the chrome registry could help here; my comment 1 was about the impossibility of knowing the exact value specified with -UILocale (when it turned out to correspond to an unavailable locale); something that I like to see available but probably not required here.
Flags: needinfo?(nohamelin)
Right, so both issues could be fixed by asking the chrome registry instead of relying directly on the pref. Great!
Assignee: nobody → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Iteration: --- → 40.3 - 11 May
Points: --- → 2
Ever confirmed: true
Flags: qe-verify-
Flags: in-testsuite-
Flags: firefox-backlog+
Attached file MozReview Request: bz://1106119/Gijs (obsolete) —
/r/8595 - Bug 1106119 - fix locale detection for menu panel, r?mconley

Pull down this commit:

hg pull -r 07d836086c83e599bee35dcde9a81cc3e031333d https://reviewboard-hg.mozilla.org/gecko/
Attachment #8604339 - Flags: review?(mconley)
Iteration: 40.3 - 11 May → 41.1 - May 25
Comment on attachment 8604339 [details]
MozReview Request: bz://1106119/Gijs

https://reviewboard.mozilla.org/r/8593/#review7255

LGTM - just one suggestion. Thanks Gijs!

::: browser/components/customizableui/content/panelUI.js:520
(Diff revision 1)
> -    let locale = Services.prefs.getComplexValue(PREF_SELECTED_LOCALE,
> +  return chromeRegistry.getSelectedLocale("browser");

Looks like nsChromeRegistryChrome::GetSelectedLocale can return failure nsresults, which will throw here.

Can we keep the try/catch and return en-US in that case?
Attachment #8604339 - Flags: review?(mconley) → review+
https://hg.mozilla.org/mozilla-central/rev/d2b74b96a090
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Attachment #8604339 - Attachment is obsolete: true
Attachment #8618755 - Flags: review+
See Also: → 1175132
You need to log in before you can comment on or make changes to this bug.