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

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: nohamelin, Assigned: Gijs)

Tracking

Trunk
Firefox 41
x86
All
Points:
2
Bug Flags:
firefox-backlog +
in-testsuite -
qe-verify -

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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".
(Reporter)

Comment 1

4 years ago
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.
(Reporter)

Updated

4 years ago
Blocks: 983583
(Assignee)

Updated

4 years ago
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 2

4 years ago
(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)
(Reporter)

Comment 3

4 years ago
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)
(Assignee)

Comment 4

4 years ago
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+
(Assignee)

Comment 5

4 years ago
Created attachment 8604339 [details]
MozReview Request: bz://1106119/Gijs

/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)

Updated

4 years ago
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
Last Resolved: 4 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
(Assignee)

Comment 9

3 years ago
Comment on attachment 8604339 [details]
MozReview Request: bz://1106119/Gijs
Attachment #8604339 - Attachment is obsolete: true
Attachment #8618755 - Flags: review+
(Assignee)

Comment 10

3 years ago
Created attachment 8618755 [details]
MozReview Request: Bug 1106119 - fix locale detection for menu panel, r?mconley
See Also: → bug 1175132
You need to log in before you can comment on or make changes to this bug.