1384172 - Expose intl.regional_prefs.use_os_locales in the UI in SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, enhancement)

Description

[Frank-Rainer Grahl (:frg)]

+++ This bug was initially created as a clone of Bug #1384007 +++

From bug 1384007:

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

Comment 1

[Frank-Rainer Grahl (:frg)]

This affects all parts so it should probably go under Appearance or Content.

Appearance and Contents both are crowded at least on Windows but there is a little space left over the buttons at least on Windows. Or maybe a new item named Region settings? User Interface Language could go in there too.

rsx11m, a job for you?

Comment 2

[rsx11m]

Looks more like we have the language-related settings in Browser > Languages. How does this affect MailNews given that we have language settings in MailNews > Text Encoding and the Folder Properties?

Comment 3

[rsx11m]

So, as suggested by frg, I had a look at bug 1384007 attachment 8890559 [details] [diff] [review] which implemented this for Thunderbird. After a few modifications, I've got the generation of the locale strings for the radiobutton labels working. Now, the only question is where to put it...

(1) The Appearance > Content prefpane: Would be consistent with Thunderbird's placement in their Advanced tab, but it's very crowded in there already and probably would trigger the window to expand vertically in certain OS/font environments.

(2) Browser > Languages seems to be a different context, this pref is about the webpage language rather than chrome languages (though I'd assume that Date/Time converted from JavaScript on such pages would be subject to the setting as well).

(3) I'd like it in the Appearance main prefpane best, it should just fit there, and at the bottom we have "User Interface Language" already, which would go along well with "Regional Formatting" of date and time.

Thus, I'll work on option (3) and see how it fits. I'm not sure if the "Date and Time Formatting" groupbox caption is entirely correct (for now, maybe, if that's the only thing modified by the locale settings, but there may be more to come). But anyway, I'll keep it consistent with Thunderbird's labels and we can discuss as we go.

Comment 4

[rsx11m]

Attachment: Proposed patch for Appearance prefpane

Here we go - this is basically the Thunderbird code, but with a couple of simplifications. For example, I got rid of the apparently redundant <hbox> and <spacer> items in the XUL code. It seems to work fine without them, and I didn't see any good reason in the TB bug to retain them.

As a semi-drive-by fix, both to make the visual appearance more pleasing and to ensure that sufficient vertical space is available, I've moved the startup- and tooltip-related groupboxes next to each other rather than underneath. The former had a lot of wasted space, and after increasing the overall size of the window considering the Calendar preferences, there is sufficient width to do that.

If someone feels strongly about the thin spacer around the Tooltips checkbox, I'd rather re-add it between Tooltips and User Interface Language, since the latter forms a nice block with the Date and Time Formatting groupbox.

I've kept the wording of the labels consistent with their TB counterparts, but changed the accesskeys ('p' was taken, and 'g' should be avoided anyway).

Help contents was updated per best knowledge. I don't have a localized build to really look at the differences between the two settings. Do we need to cover the case that both languages are equal? In theory, you could still customize your operating-system settings in a way that they differ from the general locale.

Putting up for review, but any feedback for improvement is certainly welcome.

Comment 5

[rsx11m]

Attachment: Screenshot on Windows 7

For reference, here the screenshot with the Windows 7 Default desktop theme.

The fonts are screwed up in the Preferences on trunk for some reason right now, but I've double-checked on a comm-esr52 build (where it doesn't work, but anyway) that the spacing works out fine.

Comment 6

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8948110 [details] [diff] [review]
Proposed patch for Appearance prefpane

Works fine and I also think "I've moved the startup- and tooltip-related groupboxes next to each other rather than underneath." is a good idea. Dialog would be too high otherwise to fit for x600 or maybe even x768 screens properly.

I backported to 2.53 and tested with a German region setting and the pref is only effective after a restart. It either needs its own "<description>&restartOnLangChange.label;</description>" or this one needs to be moved underneath the two groupboxes.
Maybe "Language and formatting preferences will take effect when you restart SeaMonkey".

r+ with this changed.

Comment 7

[rsx11m]

(In reply to Frank-Rainer Grahl (:frg) from comment #6)
> Dialog would be too high otherwise to fit for x600 or maybe even x768
> screens properly.

Yeah, though scrollbars should show up now if the window is getting too large for the screen. Nevertheless, it's always better to keep the layout within the visible window.

> I backported to 2.53 and tested with a German region setting and the pref is
> only effective after a restart.

Since my choices were either English or English, I didn't notice...  ;-)

> It either needs its own "<description>&restartOnLangChange.label;</description>"
> or this one needs to be moved underneath the two groupboxes.

I'll go for the latter rather than introducing yet another line.

> Maybe "Language and formatting preferences will take effect when you restart SeaMonkey".

Sounds good, I'll post an updated patch later today.

Comment 8

[rsx11m]

Attachment: Updated patch with description moved (v2)

Changes made per comment #6.

Note that you'll have to apply bug 1435034 attachment 8947580 [details] [diff] [review] first as backing it out bitrotted prefutilities.properties (the difference upon relanding hopefully only being a change in the labelDefaultFont property name).

Comment 9

[rsx11m]

Attachment: Updated patch with description moved (v2b)

Sigh, I forgot to move the localization note in pref-appearance.dtd as well...

Also, I had moved and extended the "Note:" in help/cs_nav_prefs_appearance.xhtml in v2 already to avoid confusion that the languages are shown with the same names in all three locations for most default installations, unless a differently localized SM version or different OS settings are used.

Comment 10

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8948275 [details] [diff] [review]
Updated patch with description moved (v2b)

Just an f+ for the new patch. Builds and displays fine.

Comment 11

[rsx11m]

Another change due to bug 1434737 landing:

> +++ b/suite/common/pref/pref-appearance.js
> @@ -5,5 +5,5 @@
> 
>  // Load spell-checker module to properly determine language strings
> -Components.utils.import("resource://gre/modules/InlineSpellChecker.jsm");
> +ChromeUtils.import("resource://gre/modules/InlineSpellChecker.jsm");
> 
>  function Startup()

While Cu.import() still seems to work, it apparently goes away in the (near) future.

Comment 12

[rsx11m]

Attachment: Rebased patch (v3)

Rebased to be applied on top of bug 1435034 attachment 8948518 [details] [diff] [review] and with comment #11 included.

Comment 13

[Ian Neal]

Comment on attachment 8948538 [details] [diff] [review]
Rebased patch (v3)

>+function NumberLocales_Load()
>+{
>+  const localeService =
>+    Components.classes["@mozilla.org/intl/localeservice;1"]
>+              .getService(Components.interfaces.mozILocaleService);
>+  let appLocale = localeService.getAppLocalesAsBCP47()[0];
Services should be available so could just use Services.locale instead.

r/ui-r=me with that answered.

Comment 14

[rsx11m]

(In reply to Ian Neal from comment #13)
> Services should be available so could just use Services.locale instead.

Thanks, I'll test that and will push the version that works.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e90d747e8f57
Expose intl.regional_prefs.use_os_locales in the UI in SeaMonkey. r=frg,IanN; ui-r=IanN

Comment 16

[rsx11m]

Attachment: Patch as landed (v4)

Comment 17

[rsx11m]

Attachment: Followup: Correct localization notes

I'll need to better watch out for those l10n requirements...  :-(

Too much literal copy-pasting from the TB patch, the localization notes refer to *String rather than *.label.

Trivial fix amending attachment 8948891 [details] [diff] [review] which already landed. TB has the same issue, I'll file a bug for them.

Comment 18

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8948975 [details] [diff] [review]
Followup: Correct localization notes

Didn't notice it either. doh!

r+ and thanks. If it is the only patch please push with DONTBUILD.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9b016aa17e7a
Followup: Correct references in localization notes. r=frg

Comment 20

[Frank-Rainer Grahl (:frg)]

Target 2.53.1
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/08829fd56184d1d96b4664ecbf9c2e97e759aa44