Add locale information to about:support

RESOLVED FIXED in Firefox 58

Status

()

P3
normal
RESOLVED FIXED
a year ago
2 months ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Very common pattern we experience when dealing with localization and language negotiation is trying to understand what locales user has available, requested and negotiated down to.

I'd like to introduce a new section in about:support with:

 - LocaleService::RequestedLocales
 - LocaleService::AvailableLocales
 - LocaleService::AppLocales
 - LocaleService::DefaultLocale
 - LocaleService::RegionalPreferencesLocales
 - OSPreferences::SystemLocales
 - OSPreferences::RegionalPreferencesLocales
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Priority: -- → P3
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8925718 [details]
Bug 1414975 - Add locale information to about:support.

https://reviewboard.mozilla.org/r/196862/#review202116

::: toolkit/locales/en-US/chrome/global/aboutSupport.dtd:166
(Diff revision 3)
> +<!ENTITY aboutSupport.intlLocalesAvailable "Available Locales">
> +<!ENTITY aboutSupport.intlLocalesSupported "App Locales">
> +<!ENTITY aboutSupport.intlLocalesRegionalPrefs "Regional Preferences Locales">
> +<!ENTITY aboutSupport.intlLocalesDefault "Default Locale">
> +<!ENTITY aboutSupport.intlOSPrefsSystemLocales "System Locales">
> +<!ENTITY aboutSupport.intlOSPrefsRegionalPrefs "Regional Preferences Locales">

I think this label should be different from aboutSupport.intlLocalesRegionalPrefs. This one is the OS regional prefs, the other one?
(Assignee)

Comment 5

a year ago
> I think this label should be different from aboutSupport.intlLocalesRegionalPrefs. This one is the OS regional prefs, the other one?

The other one is for LocaleService::GetRegionalPrefsLocales.

Basically, we use a pref and a bit of logic to select if LocaleService:GetRegionalPrefsLocales will return OSPreferences::RegionalPrefsLocales or LocaleService::AppLocales.

I know it's the same string, but in two categories, so I kept it as two strings. Do you have a preference for how it should be named?
Flags: needinfo?(francesco.lodolo)
(Assignee)

Comment 6

a year ago
Posted image screenshot (obsolete) —
Here's how it looks like
I missed that there was a header, identifying them as OS prefs. We can either:
a) Use the same string, without duplicating it, since there's the header.
b) Tweak the second string, e.g. to "System Regional Preferences Locales", to match the "System Locales" before.

I don't have a strong opinion, so I'd be fine with both of them. I don't think we gain a lot in duplicating the string for each section though.
Flags: needinfo?(francesco.lodolo)

Comment 8

a year ago
mozreview-review
Comment on attachment 8925718 [details]
Bug 1414975 - Add locale information to about:support.

https://reviewboard.mozilla.org/r/196862/#review202184

This seems like a good idea IMO, but I'm neither a JS/front-end expert nor a Toolkit peer, so I think you should probably ask someone in that world for the review.
Attachment #8925718 - Flags: review?(jfkthame)
(Assignee)

Comment 9

a year ago
Comment on attachment 8925718 [details]
Bug 1414975 - Add locale information to about:support.

Michael, Mossop pointed you out as a good reviewer.

Would you have time to review it? :)
Attachment #8925718 - Flags: review?(mkelly)

Comment 10

a year ago
mozreview-review
Comment on attachment 8925718 [details]
Bug 1414975 - Add locale information to about:support.

https://reviewboard.mozilla.org/r/196862/#review202492

Looks good to me, thanks! The schema change and semicolon should be simple enough to not need a re-review.

::: toolkit/modules/Troubleshoot.jsm:656
(Diff revision 3)
>      done({
>        exists: userJSFile.exists() && userJSFile.fileSize > 0,
>      });
> +  },
> +
> +  intl: function intl(done) {

[browser_Troubleshoot.js](http://searchfox.org/mozilla-central/source/toolkit/modules/tests/browser/browser_Troubleshoot.js) has a schema test for the snapshot, you'll have to add `intl` to the schema.

::: toolkit/modules/Troubleshoot.jsm:671
(Diff revision 3)
> +      },
> +      osPrefs: {
> +        systemLocales: osPrefs.getSystemLocales(),
> +        regionalPrefsLocales: osPrefs.getRegionalPrefsLocales()
> +      }
> +    })

We're missing a semicolon here. We could add some trailing commas in the object above too but that's a super-nitpicky thing and I dunno what our preferred style is across the browser for that.
Attachment #8925718 - Flags: review?(mkelly) → review+
Comment hidden (mozreview-request)
Posted image screenshot v2
Thanks! Updated the patch to your review.

Attaching a screenshot for Axel, who wanted to verify the wording.
Attachment #8925809 - Attachment is obsolete: true
Attachment #8926283 - Flags: feedback?(l10n)
(Assignee)

Updated

a year ago
Attachment #8926283 - Flags: feedback?(francesco.lodolo)
Comment on attachment 8926283 [details]
screenshot v2

lgtm
Attachment #8926283 - Flags: feedback?(l10n) → feedback+
Comment on attachment 8926283 [details]
screenshot v2

Looks good, thanks.
Attachment #8926283 - Flags: feedback?(francesco.lodolo) → feedback+
Comment hidden (mozreview-request)
Comment on attachment 8925718 [details]
Bug 1414975 - Add locale information to about:support.

carry over r+
Attachment #8925718 - Flags: review?(jfkthame) → review+
(Assignee)

Updated

a year ago
Attachment #8925718 - Flags: review+

Comment 17

a year ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a82270f0b8a9
Add locale information to about:support. r=mkelly
https://hg.mozilla.org/mozilla-central/rev/a82270f0b8a9
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Duplicate of this bug: 602800
You need to log in before you can comment on or make changes to this bug.