1505435 - Date-picker in en-US locale is suddenly doing dd/mm/yyyy instead of mm/dd/yyyy

Status: RESOLVED FIXED

(MailNews Core :: XUL Replacements, defect)

Description

[Jonathan Kamens]

I'm not sure what happened, but the datepicker XUL widget, at least in Thunderbird 60.2.1 though I don't know exactly when it started, is suddenly using DD/MM/YYYY order instead of MM/DD/YYYY, even in the en-US locale which uses MM/DD/YYYY by default.

Comment 1

[Jonathan Kamens]

Hmm.

This is not happening when I use the Thunderbird 60.2.1 release downloaded from bugzilla.mozilla.org.

It is happening when I use the Thunderbird release in Ubuntu.

Will need to investigate further.

Comment 2

[Jonathan Kamens]

OK, so the problem here appears to be that `Intl.DateTimeFormat().resolvedOptions().locale` isn't picking the right locale.

In particular, I have both en-GB and en-US locales installed, and Thunderbird is picking en-GB, even though `locale` indicates that my locale setting at the OS level is en-US, not en-GB.

I have LANG=en_US.UTF-8 set in my environment. I tried LANG=en_US, and it doesn't work either. I tried explicitly setting all of the LC_* environment variables and exporting them before running thunderbird, and that didn't help either.

I'm not certain, but I think that this is actually https://bugzilla.mozilla.org/show_bug.cgi?id=1355307?

Comment 3

[Jonathan Kamens]

I have tried all of the preference settings I could think of to try, including digging through LocaleService.cpp and DateTimeFormat.cpp, in an effort to attempt to influence Thunderbird's choice of locale. Everything I've done has been unsuccessful. I have been unable to identify any way to tell Thunderbird to use en-US as my locale instead of en-GB.

Either the code is not working properly, or whatever it is trying to do is so convoluted and poorly documented that there's no way to understand what "properly" means.

#Frustrated

Comment 4

[Jorg K (CEST = GMT+2)]

You may want to read through bug 1426907 (then you'll be really frustrated).

Comment 5

[Jonathan Kamens]

(In reply to Jorg K (GMT+1) from comment #4)
> You may want to read through bug 1426907 (then you'll be really frustrated).

Ah. It seems like perhaps this bug is a duplicate of that one? Do you agree?

Comment 6

[Jorg K (CEST = GMT+2)]

Since I'm not on Linux, I tried to avoid frustration by not reading that bug. So you can be the judge of whether it's a duplicate or not.

For the record, I'm most likely the person who has suffered most through all the date/time formatting changes, starting at bug 1325745 on Christmas Eve of 2016. If it weren't past midnight, I could easily find a dozen bugs in C-C and M-C where we, in fact, mostly myself, followed or participated in all those changes.

As far as I'm concerned the matter is mostly closed, with only Linux remaining an issue on some distros. I tried to keep away from those details.

Comment 7

[Jonathan Kamens]

Attachment: Patch to use mozIntl instead of Intl in datetimepicker

If I understand https://bugzilla.mozilla.org/show_bug.cgi?id=1426907#c73 correctly, then I think the attached patch is how to fix this.

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9023832 [details] [diff] [review]
Patch to use mozIntl instead of Intl in datetimepicker

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

As I said, we suffered through at least a dozen datetime formatting bugs and here is now more :-(

Somehow we ported this to C-C and never made the switch to mozIntl, sigh. And since we don't use this in TB, we never noticed.

::: common/bindings/datetimepicker.xml
@@ +488,4 @@
>  
>              var numberOrder = /^(\D*)\s*(\d+)(\D*)(\d+)(\D*)(\d+)\s*(\D*)$/;
>  
> +            var mozIntl = Components.classes["@mozilla.org/mozintl;1"].getService(Components.interfaces.mozIMozIntl);

Please write this as
var mozIntl = Cc["@mozilla.org/mozintl;1"].getService(Ci.mozIMozIntl);
like here:
https://searchfox.org/mozilla-central/rev/17f55aee76b7c4610a974cffd3453454e0c8de7b/toolkit/content/widgets/datetimepopup.xml#31

And twice more below. That also works for mozilla60.

@@ +1013,5 @@
> +            var mozIntl = Components.classes["@mozilla.org/mozintl;1"].getService(Components.interfaces.mozIMozIntl);
> +            var dtf = new mozIntl.DateTimeFormat();
> +            var locale = dtf.resolvedOptions().locale + "-u-ca-gregory";
> +            var dtfMonth = new mozIntl.DateTimeFormat(locale, {month: "long", timeZone: "UTC"});
> +            var dtfWeekday = new mozIntl.DateTimeFormat(locale, {weekday: "narrow"});

Also working in TB 60, see examples here:
https://dxr.mozilla.org/mozilla-esr60/search?q=mozIntl.DateTimeFormat&redirect=false

(I was thinking of the 59 change in bug 1428341.)

Comment 9

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #8)
> As I said, we suffered through at least a dozen datetime formatting bugs and
> here is now more :-(
... one more.

Comment 10

[Jonathan Kamens]

Attachment: Updated patch that uses Cc and Ci

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: datetimepicker.patch - updated

I'm looking at this again.

Should toLocaleDateString() and toLocaleTimeString() also be replaced in the picker (three call sites) using mozIntl.DateTimeFormat()?

So for example instead of
  var dt = new Date(2002, 9, 4).toLocaleDateString(locale);
it would be
  var dtf = new mozIntl.DateTimeFormat();
  var formatter = dtf.DateTimeFormat(locale, {...});
  var dt = formatter.format(Date(2002, 9, 4));

or words to that extent. See https://searchfox.org/comm-central/search?q=DateTimeFormat&case=false&regexp=false&path= for examples.

Comment 12

[Jorg K (CEST = GMT+2)]

Sorry, wires crossed, we both submitted the same patch. Anyway, see my concern in comment #11.

Comment 13

[Jonathan Kamens]

I don't know. I don't pretend to understand all of this. All I know is that it works as expected with the patch I already submitted. I would worry about the unexpected side-effects of additional changes to fix things that do not currently appear to be broken.

Comment 14

[Jorg K (CEST = GMT+2)]

The thing is this: toLocaleDateString() is basically Intl.DateTimeFormat(), so we've mostly eliminated its use from the codebase which we switched to mozIntl:
https://searchfox.org/comm-central/search?q=toLocaleDateString&case=false&regexp=false&path=

The reason it that mozIntl follows, at least on Windows, the tweaks that you can do to the date formatting. toLocaleDateString() doesn't follow that. So I'd have to experiment with this a bit.

The same goes for toLocaleTimeString().

A good example to look at is this: https://hg.mozilla.org/comm-central/rev/5298286c9521

Comment 15

[Jonathan Kamens]

I'm not denying any of that.

What I'm saying is that I find all of this stuff to be arcane, incredibly poorly documented, and seemingly in flux, and therefore I am extremely reluctant to make any changes that I do not know for certain are necessary to fix something.

If you feel like you are comfortable with making additional changes, feel free.

My goal in providing a patch was not to sign up for carrying the patch through to landing. I am, frankly, all through playing that game with Thunderbird, having been frustrated by it too many times.

I submitted the patch to point out exactly where the problem is and thereby point others who understand all this stuff far better than I at what they need to do to fix the problem.

Land my patch, land my patch with additional changes, land nothing and let it remain broken, it's up to you. I've done what I intended to do here. *shrug*

Comment 16

[[:MakeMyDay]]

Have you checked |new Service.intl.*| instead of |Intl.*| - that is the recommended way if you want OS settings being considered. See also [1] around line 66. Another good source of information about all that stuff is [2].

[1] https://searchfox.org/comm-central/source/mozilla/intl/docs/dataintl.rst
[2] https://firefox-source-docs.mozilla.org/intl/index.html

Comment 17

[Jonathan Kamens]

The comment I linked to above claims that mozIntl is the "recommended way" for privileged code to do this, so that's what I used in my patch.

Now you're telling me that something different is the "recommended way."

I frankly don't know whom to listen to.

Comment 18

[Jorg K (CEST = GMT+2)]

I'm looking into it and I'll submit a patch within a few hours. MMD and Zibi are suggesting the same thing, to use mozIntl, which is what |new Service.intl.*| returns.

The problem is the |locale + "-u-ca-gregory-nu-latn"|. mozIntl will spit out dates an Arabic/Korean/Thai etc. if you don't use it.

Comment 19

[Jorg K (CEST = GMT+2)]

Attachment: datetimepicker.patch (v2)

This works for me. Zibi, how can we improve these few lines and avoid getting a new object twice?

+  var mozIntl = Cc["@mozilla.org/mozintl;1"].getService(Ci.mozIMozIntl);
+  var dtf = new mozIntl.DateTimeFormat();
+  var locale = dtf.resolvedOptions().locale + "-u-ca-gregory-nu-latn";
+  dtf = new mozIntl.DateTimeFormat(locale, { timeStyle: "long" });
+  var pmTime = dtf.format(new Date(2000, 0, 1, 16, 7, 9));

Does something like
dtf = new Service.intl.DateTimeFormat("u-ca-gregory-nu-latn", { timeStyle: "long" }); work?
So the "default" locale but with some modifiers?

Comment 20

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

> This works for me. Zibi, how can we improve these few lines and avoid getting a new object twice?

Yes, we can.

Currently, until bug 1433303 lands, the best way would be to do:

```
let regionalPreferencesLocale = Services.locale.regionalPrefsLocales[0] + "-u-ca-gregory-nu-latn";
let dtf = new mozIntl.DateTimeFormat(locale, { timeStyle: "long" });
let pmTime = dtf.format(new Date(2000, 0, 1, 16, 7, 9));
```

once that lands, we'll want to switch to:

```
let loc = new Intl.Locale(Services.locale.regionalPrefsLocales[0], {
  calendar: "gregory",
  numberingSystem: "latn"
});
```

Comment 21

[Jorg K (CEST = GMT+2)]

Attachment: datetimepicker.patch (v3)

OK, this should do.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/38005533aabc
Switch date-picker from Intl.DateTimeFormat to mozIMozIntl. r=jorgk

Comment 23

[Jorg K (CEST = GMT+2)]

Comment on attachment 9024073 [details] [diff] [review]
datetimepicker.patch (v3)

Note that for TB 60 this
  var locale = Services.locale.regionalPrefsLocales[0] + "-u-ca-gregory-nu-latn";
needs to be changed to
  var locale = Services.locale.getRegionalPrefsLocales()[0] + "-u-ca-gregory-nu-latn";

Comment 24

[Jorg K (CEST = GMT+2)]

Attachment: datetimepicker.patch - ESR version

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6c2e0403281d
Follow-up: Import Services.jsm. rs=bustage-fix DONTBUILD

Comment 26

[Jorg K (CEST = GMT+2)]

TB 60.3.1/TB 60.4 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/c0ae895b8809f4ba4eea9a50e2f4af2fee00cfde

Comment 27

[Jorg K (CEST = GMT+2)]

Grrrr, forgot the second part:
https://hg.mozilla.org/releases/comm-esr60/rev/241764e35357d4a51cee09849f72910e72d18d7c

Comment 28

[Jorg K (CEST = GMT+2)]

As far as I can tell, this works now, tested with your add-on "Datepicker Bug Reproducer", although the 11th of November isn't a great day for testing.

Jonathan, can you please try a TB 60.3.1 version from this push:
https://treeherder.mozilla.org/#/jobs?repo=comm-esr60&selectedJob=211079578&revision=241764e35357d4a51cee09849f72910e72d18d7c
Click an "N" and under Job Details download target.tar.bz2, target.dmg or target.installer.exe:
https://queue.taskcluster.net/v1/task/UJYyVpPQTaePBZwfyft-bg/runs/0/artifacts/public/build/target.tar.bz2
https://queue.taskcluster.net/v1/task/b56j-uPgQt-bIgLgTgydqg/runs/0/artifacts/public/build/target.dmg
https://queue.taskcluster.net/v1/task/J_C7Dh-AQY6DqO3uVgOSnA/runs/0/artifacts/public/build/install/sea/target.installer.exe
https://queue.taskcluster.net/v1/task/YkzTaJsYSvaF6571aajHIA/runs/0/artifacts/public/build/install/sea/target.installer.exe

Comment 29

[Jonathan Kamens]

Yes, the behavior is as expected in the try build.

Comment 30

[Jorg K (CEST = GMT+2)]

Beta (TB 64 beta 3):
https://hg.mozilla.org/releases/comm-beta/rev/40efa95a8ab0
https://hg.mozilla.org/releases/comm-beta/rev/c40e51472fab