1414975 - Add locale information to about:support

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

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 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1414975 - Add locale information to about:support.

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

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

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

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

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

Comment 4

[Francesco Lodolo [:flod]]

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?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

> 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?

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: screenshot

Here's how it looks like

Comment 7

[Francesco Lodolo [:flod]]

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.

Comment 8

[Jonathan Kew [:jfkthame]]

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.

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

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? :)

Comment 10

[Osmose [:osmose, :mkelly]]

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.

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

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

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: screenshot v2

Thanks! Updated the patch to your review.

Attaching a screenshot for Axel, who wanted to verify the wording.

Comment 13

[Axel Hecht [:Pike]]

Comment on attachment 8926283 [details]
screenshot v2

lgtm

Comment 14

[Francesco Lodolo [:flod]]

Comment on attachment 8926283 [details]
screenshot v2

Looks good, thanks.

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196862/diff/4-5/

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

carry over r+

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a82270f0b8a9
Add locale information to about:support. r=mkelly

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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