1331608 - [DateTimePicker] months appear twice or not at all in month picker due to daylight savings time (e.g. March, October)

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Steps to reproduce:
0. Set your computer to a timezone which observes daylight savings time for a part of the year.
1. Open about:config and confirm the warning.
2. Search for dom.forms.datetime and set it to true.
3. Close and restart Nightly.
4. Open http://slides.com/jessicajong/datetime#/15
5. Click on the form.
6. Click onto the header "<month> <year>"
Actual result here:
"Mar" (March) appears twice, "Oct" (October) is missing.
Expected:
All months appear once

This is caused by creating a Date in locale timezone after daylight savings changes and then using it with Intl.DateTimeFormat.

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Bug 1331608 - show every month exactly once in monthpicker.

Review commit: https://reviewboard.mozilla.org/r/105108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105108/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8827424 [details]
Bug 1331608 - show every month exactly once in monthpicker.

https://reviewboard.mozilla.org/r/105108/#review106020

Thanks for catching this!

There are a few other places that I think need updating too:

http://searchfox.org/mozilla-central/search?q=new+Date%280&path=toolkit%2Fcontent%2Fwidgets

scottwu, care to weigh in? Can we skip any of these?

Comment 3

[Mike Conley (:mconley) (:⚙️)]

ni'ing scottwu for https://reviewboard.mozilla.org/r/105108/#review106020

Comment 4

[Scott Wu [:scottwu]]

Thanks for catching the bug Sebastian!

I found that the actual problem is that I didn't define a time zone for DateTimeFormat:
http://searchfox.org/mozilla-central/rev/790b2cb423ea1ecb5746722d51633caec9bab95a/toolkit/content/widgets/datepicker.js#288

There should be a `timeZone: "UTC"` option. Without it, it defaults to runtime's time zone. Whether we use `new Date(0, month)` or `new Date(Date.UTC(0, month))` is irrelevant in this case, but it might help for consistency & readability.

As for other uses of `new Date(0)`, I think they are okay because they don't have time zone problems, and are constructed to later be set with the actual time/dates.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Is that enough information to proceed on this bug, aryx? I think it's just a matter of adding the timeZone properties here: http://searchfox.org/mozilla-central/rev/790b2cb423ea1ecb5746722d51633caec9bab95a/toolkit/content/widgets/datepicker.js#288

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Scott Wu [:scottwu] from comment #4)
> There should be a `timeZone: "UTC"` option. Without it, it defaults to
> runtime's time zone. Whether we use `new Date(0, month)` or `new
> Date(Date.UTC(0, month))` is irrelevant in this case, but it might help for
> consistency & readability.
It's relevant as |new Date(0, month)| with |timeZone: "UTC"| in the formatter will always create wrong month names for machines with timezones UTC+X, e.g. check this output:

var monthFormat = new Intl.DateTimeFormat("en-US", { month: "short", timeZone: "UTC" }).format;
var dateTimeFormat = new Intl.DateTimeFormat("en-US", { year: "numeric", month: "short", day: "numeric", hour: "numeric", timeZone: "UTC" }).format;
var output = "";
for (var month = 0; month < 12; month++)
{
  var date = new Date(0, month);
  output += date + " - " + dateTimeFormat(date) + " - " + monthFormat(date) + "\n";
}
output

This bug should only affect Windows because Linux and Mac always use the same timezone even if it is one which will switch to DST, see bug 1330307 comment 2.

I didn't find any other code in toolkit/content/widgets/ which requires changes.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #6)
> I didn't find any other code in toolkit/content/widgets/ which requires
> changes.
Correction: There was code in datetimepicker.xml which also needed changes.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Comment on attachment 8827424 [details]
Bug 1331608 - show every month exactly once in monthpicker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105108/diff/1-2/

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8827424 [details]
Bug 1331608 - show every month exactly once in monthpicker.

https://reviewboard.mozilla.org/r/105108/#review107668

Looks okay - though I didn't test it locally.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Thanks for the patch!

Comment 11

[Scott Wu [:scottwu]]

You're right :aryx, it is necessary to use Date.UTC combined with timeZone: UTC.

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #7)
> Correction: There was code in datetimepicker.xml which also needed changes.

This is actually an older date picker, and not the one currently being worked on. However, I can't find where it's being used in Firefox. The tests appear to pass fine on a Mac though.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9271902315d
show every month exactly once in monthpicker. r=mconley

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b9271902315d

Comment 14

[roxleitan]

Verified as fixed using the latest Nightly 56.0a1 (2017-07-09) on Ubuntu 16.04, Mac OS X 10.12 and Windows 10 x64.