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

VERIFIED FIXED in Firefox 54

Status

()

Toolkit
XUL Widgets
VERIFIED FIXED
11 months ago
5 months ago

People

(Reporter: aryx, Assigned: aryx)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed, firefox56 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

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 hidden (mozreview-request)
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?
Attachment #8827424 - Flags: review?(mconley) → review-
ni'ing scottwu for https://reviewboard.mozilla.org/r/105108/#review106020
Flags: needinfo?(scwwu)

Comment 4

11 months ago
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.
Flags: needinfo?(scwwu)
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
Flags: needinfo?(aryx.bugmail)
(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.
Flags: needinfo?(aryx.bugmail)
(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 hidden (mozreview-request)
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.
Attachment #8827424 - Flags: review?(mconley) → review+
Thanks for the patch!

Comment 11

11 months ago
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

11 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/b9271902315d
show every month exactly once in monthpicker. r=mconley

Comment 13

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9271902315d
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
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.
Status: RESOLVED → VERIFIED
status-firefox56: --- → verified
You need to log in before you can comment on or make changes to this bug.