Expose intl.regional_prefs.use_os_locales in the UI in SeaMonkey

RESOLVED FIXED in seamonkey2.57

Status

enhancement
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: frg, Assigned: rsx11m.pub)

Tracking

Trunk
seamonkey2.57

SeaMonkey Tracking Flags

(seamonkey2.53 affected)

Details

Attachments

(3 attachments, 4 obsolete attachments)

Reporter

Description

2 years ago
+++ 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).
Reporter

Comment 1

2 years ago
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?
Flags: needinfo?(rsx11m.pub)
Assignee

Comment 2

2 years ago
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?
Flags: needinfo?(rsx11m.pub)
Assignee

Comment 3

Last year
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.
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Assignee

Updated

Last year
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Assignee

Comment 4

Last year
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.
Attachment #8948110 - Flags: ui-review?(iann_bugzilla)
Attachment #8948110 - Flags: review?(frgrahl)
Assignee

Comment 5

Last year
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.
Reporter

Comment 6

Last year
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.
Attachment #8948110 - Flags: review?(frgrahl) → review+
Assignee

Comment 7

Last year
(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.
Assignee

Comment 8

Last year
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).
Attachment #8948110 - Attachment is obsolete: true
Attachment #8948110 - Flags: ui-review?(iann_bugzilla)
Attachment #8948273 - Flags: ui-review?(iann_bugzilla)
Attachment #8948273 - Flags: review+
Assignee

Comment 9

Last year
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.
Attachment #8948273 - Attachment is obsolete: true
Attachment #8948273 - Flags: ui-review?(iann_bugzilla)
Attachment #8948275 - Flags: ui-review?(iann_bugzilla)
Attachment #8948275 - Flags: review+
Reporter

Comment 10

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

Just an f+ for the new patch. Builds and displays fine.
Attachment #8948275 - Flags: feedback+
Assignee

Comment 11

Last year
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.
Assignee

Comment 12

Last year
Posted patch Rebased patch (v3) (obsolete) — Splinter Review
Rebased to be applied on top of bug 1435034 attachment 8948518 [details] [diff] [review] and with comment #11 included.
Attachment #8948275 - Attachment is obsolete: true
Attachment #8948275 - Flags: ui-review?(iann_bugzilla)
Attachment #8948538 - Flags: ui-review?(iann_bugzilla)
Attachment #8948538 - Flags: review+

Comment 13

Last year
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.
Attachment #8948538 - Flags: ui-review?(iann_bugzilla)
Attachment #8948538 - Flags: ui-review+
Attachment #8948538 - Flags: review+
Assignee

Comment 14

Last year
(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

Last year
Pushed by rsx11m.pub@gmail.com:
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
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Assignee

Comment 16

Last year
Attachment #8948538 - Attachment is obsolete: true
Attachment #8948891 - Flags: ui-review+
Attachment #8948891 - Flags: review+
Assignee

Comment 17

Last year
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.
Attachment #8948975 - Flags: review?(frgrahl)
Reporter

Comment 18

Last year
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.
Attachment #8948975 - Flags: review?(frgrahl) → review+

Comment 19

Last year
Pushed by rsx11m.pub@gmail.com:
https://hg.mozilla.org/comm-central/rev/9b016aa17e7a
Followup: Correct references in localization notes. r=frg
Assignee

Updated

Last year
Target Milestone: --- → Seamonkey2.57
You need to log in before you can comment on or make changes to this bug.