Closed Bug 1414975 Opened 7 years ago Closed 7 years ago

Add locale information to about:support

Categories

(Core :: Internationalization, enhancement, P3)

Product:
Core
Component:
Internationalization
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(2 files, 1 obsolete file)

Bug 1414975 - Add locale information to about:support.
59 bytes, text/x-review-board-request
Details
screenshot v2
54.56 KB, image/png
Pike
: feedback+
flod
: feedback+
Details 
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
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Priority: -- → P3
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?
> 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)
Attached 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 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)
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 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+
Attached 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)
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 on attachment 8925718 [details]
Bug 1414975 - Add locale information to about:support.

carry over r+
Attachment #8925718 - Flags: review?(jfkthame) → review+
Attachment #8925718 - Flags: review+
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
Closed: 7 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: