1384007 - Expose intl.regional_prefs.use_os_locales in the UI

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

By popular demand, M-C have implemented bug 1379420, bug 1380916 and bug 1379905 so that chrome dates/times done via JS mozIntl and C++ (DateTimeFormat.cpp) will again display dates as chosen in the OS.

Whether the app locale of the OS locale will be followed depends on the value of a preference, intl.regional_prefs.use_os_locales.

We should expose this preference in the UI, something like:
(*) Use application language (English United States)
( ) Use custom regional settings defined in the OS (German Germany).

Let's do it quickly, so this can land on TB 56. Even a version without the languages in parenthesis to be refined later would be acceptable.

Richard, do you have some cycles for this?

Ben, this one is for you!!

Comment 1

[Jorg K (CEST = GMT+2)]

Zibi, can you please lend us a hand here: What is the official way to retrieve the app language and the OS language? I'm sure you have magic in the box for that.

Comment 2

[Jorg K (CEST = GMT+2)]

BTW, FF will eventually do this in bug 1379910.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch - WIP (v1).

Now we can discuss what the strings should look like:

Languages
(*) Use application language (...)
( ) Use language from regional settings in the operating system (...)

Or maybe: "Regional Preferences" or "Date and Time formatting"?

So maybe:

Date and Time formatting
(*) Application locale (...)
( ) Locale from regional settings (...).

Suggestions please!

In bug 1379910 Zibi suggested a simple checkbox:
[ ] Use System Locale (ab-CD) for Regional Preferences.
I don't think that's crash hot since at least on Windows, there's a system locale and a regional formatting locale.

P.S.: I'm not assigning the bug to me (yet) in case someone has a great idea and wants to take over.

Comment 4

[Richard Marti (:Paenglab)]

I think "Date and Time formatting" would be the best as the user sees a wrong time formatting and searches the prefs for this problem. With "languages" the user could associates it more with the UI localization.

Comment 5

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

(In reply to Jorg K (GMT+2) from comment #1)
> Zibi, can you please lend us a hand here: What is the official way to
> retrieve the app language and the OS language? I'm sure you have magic in
> the box for that.

The AppLocale - LocaleService.GetAppLocalesAsBCP47, OSLocale - OSPreferences::GetSystemLocale, OS Regional Preferences Locale - OSPreferences::GetRegionalPrefsLocales.

If you want to get the name out of the locale ID, you'll need to use ICU API for that.

Comment 6

[Jorg K (CEST = GMT+2)]

Thanks you, I see it now.

dom/tests/mochitest/chrome/test_intlUtils_getLocaleInfo.html:
const localeService =
  Cc["@mozilla.org/intl/localeservice;1"].getService(Ci.mozILocaleService);
let appLocale = localeService.getAppLocalesAsBCP47()[0];

intl/locale/tests/unit/test_osPreferences.js:
const osprefs =
  Components.classes["@mozilla.org/intl/ospreferences;1"]
  .getService(Components.interfaces.mozIOSPreferences);
const systemLocale = osprefs.systemLocale;
const systemLocales = osprefs.getSystemLocales();
const rgLocales = osprefs.getRegionalPrefsLocales();

As for turning the locale ID into a name, InlineSpellChecker.jsm has some code for that in getDictionaryDisplayName(dictionaryName). Would it be bad to use that?

Comment 7

[Richard Marti (:Paenglab)]

Attachment: prefs.png

This is how it would look with my wording.

This makes the prefs window very tall (591px). But it's hard to move it to a other tab that fits to the format theme.

Comment 8

[Frank-Rainer Grahl (:frg)]

Why not under Display?

Formatting seems to have plenty of space left and Display->Advanced can be used too.

Comment 9

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

Yeah, makes sense. I'm likely going to try to get rid of it eventually and move to ICU's getDisplayName [0]

So, if you prefer to do less work now, go for it :)

If you have capacity - maybe you'd like to take on the task to add it to mozIntl.getDisplayInfo?
The API would be like:

mozIntl.getDisplayNames("en-US", [
  "locales/en-US",
  "locales/de",
], "long");

should return:

{
  locale: 'en-US',
  style: 'long',
  values: {
    'locales/en-US': 'English (American)',
    'locales/de': 'German'
  }
}

[0] http://icu-project.org/apiref/icu4c/classicu_1_1Locale.html#a9255df735dfbc5af6b2883edc4ca4f15

Comment 10

[Richard Marti (:Paenglab)]

(In reply to Frank-Rainer Grahl (:frg) from comment #8)
> Why not under Display?
> 
> Formatting seems to have plenty of space left and Display->Advanced can be
> used too.

I thought first about it too. But I think it's more for the messages display and not the global display.

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch - WIP (v2) - not working

Aceman, can dynamic labels be done in XUL? This isn't working.

BTW, I'm trying to make it:

Date and Time formatting
(*) Application locale (...)
( ) Locale from regional settings (...)

Comment 12

[Jorg K (CEST = GMT+2)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #9)
> If you have capacity - maybe you'd like to take on the task to add it to
> mozIntl.getDisplayInfo?
Sorry, my task is to keep the show on the road for Thunderbird, I haven't done anything significant in M-C for a while.

Comment 13

[Frank-Rainer Grahl (:frg)]

> I thought first about it too. But I think it's more for the messages display and not the global display.

Semantics. You can always say it wasn't so :) But that is where I would probably look in the first place even before Generalas an unsuspecting user. Would be different if it would say 'Message Display' or so.

Comment 14

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

(In reply to Jorg K (GMT+2) from comment #12)
> (In reply to Zibi Braniecki [:gandalf][:zibi] from comment #9)
> > If you have capacity - maybe you'd like to take on the task to add it to
> > mozIntl.getDisplayInfo?
> Sorry, my task is to keep the show on the road for Thunderbird, I haven't
> done anything significant in M-C for a while.

Fair :)

Then feel free to use the .properties. Once we have it in mozIntl, it shouldn't be hard to switch I hope.

Comment 15

[Jorg K (CEST = GMT+2)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #14)
> Then feel free to use the .properties.
.properties? I was talking about using getDictionaryDisplayName() from InlineSpellChecker.jsm. Have I misunderstood your comment #9 "Go for it"?

mozIntl.getDisplayNames() already exists, what is mozIntl.getDisplayInfo?

Comment 16

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

> .properties? I was talking about using getDictionaryDisplayName() from InlineSpellChecker.jsm. Have I misunderstood your comment #9 "Go for it"?

The properties file is used by InlineSpellChecker.jsm.

I think I would prefer to generalize the function out of InlineSpellChecker, because it sounds architecturally wrong to put an external dependency on it as is (because the function is intended for InlineSpellChecker only and not expected to be used outside of it).

Maybe you could move it to mozIntl.js as `mozIntl.getLocaleNames(['de', 'fr', 'en-US']);` etc - and then call it from InlineSpellChecker and your code?

I'm not going to block you if you'll go the easier path, but it may create a hassle if ever someone will decide to move/remove the inlinespellchecker.

> mozIntl.getDisplayNames() already exists, what is mozIntl.getDisplayInfo?

I meant getDisplayNames. Sorry! :)

Comment 17

[Jorg K (CEST = GMT+2)]

Oops, my NI got lost. Aceman, question in comment #11.

Comment 18

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch - WIP (v3).

This works but shows the locale IDs so far. To be continued.

Comment 19

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch - WIP (v4).

OK, this is working. I'll need to localise the strings still.

No access keys since they don't seem to work with the dynamic labels.
Also I made it

Date and Time formatting
(*) Application locale: ...
( ) Locale from regional settings: ...

to avoid double parenthesis:
(*) Application locale (English (United States))

Maybe is should be "Date and Time Formatting" or "Date and time formatting".

Suggestions welcome.

Comment 20

[Jorg K (CEST = GMT+2)]

Attachment: Screenshot

Comment 21

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #19)
> Created attachment 8890305 [details] [diff] [review]
> 1384007-intl.regional_prefs.use_os_locales.patch - WIP (v4).
> 
> No access keys since they don't seem to work with the dynamic labels.

Have you tried using the first part of the label instead of "set dynamically"?

Comment 22

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch - (v5).

OK, here is the full solution with localisable strings. I made the layout horizontal, so it would take less space.

Comment 23

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #21)
> Have you tried using the first part of the label instead of "set
> dynamically"?
I don't understand the question. What's the first part?
With label="set dynamically" and accelerator key "a" I get no accelerator key since this is applied to "set dyn_a_mically" and that gets replaced later. If I use "r" for the regional settings, I get "(R)" after the string since the "set dynamically" label doesn't have an "r". I see no way to get the letter of the dynamic label underlined.

I tried:
        <hbox>
          <radio id="appLocale"
                 value="false"
                 accesskey="a"/>
          <spacer flex="1"/>
        </hbox>
        <hbox>
          <radio id="rsLocale"
                 value="true"
                 accesskey="r"/>
          <spacer flex="1"/>
        </hbox>
and got no access keys whatsoever. I could set them in code :-(

Besides, very few users will ever touch this setting, and those who do will do it once in their lives.

Feel free to play with it.

Comment 24

[Jorg K (CEST = GMT+2)]

The best I can do is do label=" ", then I get:
(*) Application locale: English (United States)(A)   (*) Regional settings locale: German (Germany)(R)
which looks funny.
The (A) and (R) are placed after the label which doesn't contain those letters, and then the labels get replaced.

Comment 25

[Richard Marti (:Paenglab)]

My proposal didn't work. Let's wait for aceman.

(In reply to Jorg K (GMT+2) from comment #22)
> Created attachment 8890336 [details] [diff] [review]
> 1384007-intl.regional_prefs.use_os_locales.patch - (v5).
> 
> OK, here is the full solution with localisable strings. I made the layout
> horizontal, so it would take less space.

Leave it vertical. With horizontal we could have issues with some locales with long labels. The pref doesn't shrink, so it's a other pane using this height.

Please, can you use "Date and Time Formatting"?

Comment 26

[Jorg K (CEST = GMT+2)]

Comment on attachment 8890336 [details] [diff] [review]
1384007-intl.regional_prefs.use_os_locales.patch - (v5).

I have a better idea, have a label with %S and only replace that dynamically. Another version later.

Comment 27

[:aceman]

(In reply to Jorg K (GMT+2) from comment #11)
> Aceman, can dynamic labels be done in XUL? This isn't working.

Yes you can do dynamic labels (set by JS code), but then the strings need to be in a .properties file that is included as a stringbundle in the xul file.

Comment 28

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch - WIP (v6).

OK, here I've changed the title and made it vertical again.

I'm doing a dynamic label by modifying the existing one. I think that's easier than making it a property like in (v5) since the localiser can see it all in context.

Whatever I do, I can't get accesses keys to work.

So Aceman, which approach do you prefer, v5 or v6 and do you have a good idea about the assess keys? If you prefer v6, we either need to get the access keys to work or pull the code out.

Comment 29

[Jorg K (CEST = GMT+2)]

There is a larger problem here. For an estimated 95% of our users, the two options will be the same. We have the largest user groups in the US, Germany, France and Japan. All these will use a localised version on an OS to match.

We developers all get very exited about this since we use an en-US version and would like it to do dates differently, but for most of our users this is really a non-issue.

So is it a good idea to place this onto the "General" pane and waste prime space?

Maybe it should be banned to "Advanced / General"?

Comment 30

[Richard Marti (:Paenglab)]

Or only show when they are different.

Comment 31

[Jorg K (CEST = GMT+2)]

That's just a support nightmare: A user interface that depends on some external parameters. So go and say:
Tools > Options, (Advanced, )General, Formatting, tell me what you see, and the persons answers: Nothing, not there :-(

Comment 32

[:aceman]

Comment on attachment 8890336 [details] [diff] [review]
1384007-intl.regional_prefs.use_os_locales.patch - (v5).

Review of attachment 8890336 [details] [diff] [review]:
-----------------------------------------------------------------

I like this version using properties better as that is the standard method.

Maybe move the option into Display ->Advanced or Advanced->General->System integration .

::: mail/components/preferences/general.js
@@ +1,5 @@
>  /* -*- Mode: JavaScript; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
>   * This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +Components.utils.import("resource://gre/modules/InlineSpellChecker.jsm");

Empty line before the import.

@@ +247,5 @@
> +                                                          [appLocale]);
> +    let rsLocaleString = this.mBundle.getFormattedString("rsLocaleString",
> +                                                         [rsLocale]);
> +    appLocaleLabel.setAttribute("label", appLocaleString);
> +    rsLocaleLabel.setAttribute("label", rsLocaleString);

Have the accesskeys in the .properties file too, get them here and then set via 'appLocaleRadio.accessKey = appLocaleAccessKey;', that worked for me (code taken from the v6 patch).

::: mail/components/preferences/general.xul
@@ +155,5 @@
> +      <hbox align="center">
> +      <radiogroup id="formatLocale"
> +                  class="indent"
> +                  preference="intl.regional_prefs.use_os_locales"
> +                  orient="horizontal">

The stacking should be vertical as the 2 option do not fit inside the Preferences dialog when horizontal.

::: mail/locales/en-US/chrome/messenger/preferences/general.dtd
@@ +38,5 @@
>  <!ENTITY browse.accesskey                 "B">
>  
>  <!ENTITY defaultSearchEngine.label        "Default Search Engine">
> +
> +<!ENTITY dateTimeFormatting.label         "Date and time formatting">
\ No newline at end of file

Date and Time Formatting, that seems to be the style for headers in the dialog.

Comment 33

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch (v7).

So please confirm that this is how you like it (accesskey working!) before I move it to "Advanced / General".

Comment 34

[:aceman]

Comment on attachment 8890542 [details] [diff] [review]
1384007-intl.regional_prefs.use_os_locales.patch (v7).

Review of attachment 8890542 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, I like this, thanks.

::: mail/components/preferences/general.xul
@@ +160,5 @@
> +        <hbox>
> +          <radio id="appLocale"
> +                 value="false"/>
> +                 <!-- label and accesskey will be set dynamically -->
> +          <spacer flex="1"/>

Are the spacers needed?

Comment 35

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #34)
> Are the spacers needed?
Copied from elsewhere on the page. I'll move it now.

Comment 36

[Jorg K (CEST = GMT+2)]

Attachment: 1384007-intl.regional_prefs.use_os_locales.patch (v8).

That should be it then. If you want different access keys, let me know. These appear to work in the "in content" view.

Comment 37

[Richard Marti (:Paenglab)]

Comment on attachment 8890559 [details] [diff] [review]
1384007-intl.regional_prefs.use_os_locales.patch (v8).

This makes the prefs now really tall. Before 533px and now 603px on Win10, not tested on Linux or macOS. We can add it now to Advanced/General to have it in TB but we need to decide what we want to do to make it less tall in a followup bug (maybe enable the incontent prefs ;-) ).

I'm not so happy with the sentence "Regional settings locale:" because it doesn't say from where ths setting comes. Maybe a "Systems regional settings locale:" would be better.

Comment 38

[Jorg K (CEST = GMT+2)]

I'm using in-content prefs ever since I've discovered them ;-) In fact, I have options open all the time.

In Windows 10 the control panel is called "Regional", in Windows 10 it's "Regional and Language Options". I don't have Linux with me right now, but looking on the web I see "Regional Formats" (https://i.stack.imgur.com/rHCLE.png).

Even on Mac this seems to be associated with "region" (http://cdn.osxdaily.com/wp-content/uploads/2012/06/change-regional-settings-mac.jpg) so I don't think adding "System" won't clarify anything. Besides, it's on the Advanced panel ;-)

Comment 39

[:aceman]

Comment on attachment 8890559 [details] [diff] [review]
1384007-intl.regional_prefs.use_os_locales.patch (v8).

Review of attachment 8890559 [details] [diff] [review]:
-----------------------------------------------------------------

But please fix the expanding of the preferences dialog when the Advanced pane is selected due to these new options in a followup.

Comment 40

[Jorg K (CEST = GMT+2)]

On my Windows 10 the panel grows from 585px to 637px. There are 37px of empty space above the [OK] [Cancel] buttons, surely we can lose some of that.

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/db038d905a4e
Expose intl.regional_prefs.use_os_locales in the UI. r=aceman,ui-r=Paenglab

Comment 42

[Edoardo Tirtarahardja]

What is the different between 'intl.locale.matchOS' and 'intl.regional_prefs.use_os_locales'???

Comment 43

[baptx]

Does someone have an answer to Edoardo question? By the way, where is the UI option to enable intl.regional_prefs.use_os_locales? I cannot see the option in Preferences -> General -> Language when using latest Firefox 74.0.1 on Linux 64-bit (deb package on Lubuntu).

Comment 44

[baptx]

I noticed this bug is about Thunderbird and it is indeed displayed in Thunderbird under Preferences -> Advanced -> General -> Date and Time Formatting. But is it possible to add the UI option to Firefox also?