-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
timeMinute, timeHour are sometimes wrong (Chrome 67). #29
Comments
Per https://tc39.github.io/ecma262/#sec-time-values-and-time-range:
So, I think this is probably a bug in Chrome. |
I did a little bisection to discover when it gets weird: noon, November 18, 1883. new Date(-2717640000000).toString()
But date = new Date(-2717640000001)
|
The day time zones were created in the US. https://www.history.com/this-day-in-history/railroads-create-the-first-time-zones |
In case it helps, the issue may arise from Chrome incorporating an updated table of historical timezone offsets. date = new Date(-7506057601234)
// Fri Feb 22 1732 00:07:00 GMT-0752 (Pacific Standard Time) <-- Note GMT-0752 date.getTimezoneOffset() // 472 Compare to: date = new Date(2018, 1, 22)
// Fri Feb 22 2018 00:00:00 GMT-0800 (Pacific Standard Time) date.getTimezoneOffset() // 480 |
For the purposes of discussion, I’ve created a “naive” implementation of time intervals that uses the native Date methods as much as possible. For example, d3.timeHour is implemented as: date.setMinutes(0, 0, 0); https://github.com/d3/d3-time/compare/naive-time Unfortunately, this does not work reliably across browsers. For example, Chrome and Node do not perform the correct result on the boundary with daylight saving: date = new Date(Date.UTC(2011, 10, 06, 09, 00)); // 2011-11-06T09:00:00.000Z
date.setMinutes(0, 0, 0); // Should be a no-op!
date; // 2011-11-06T08:00:00.000Z In other words, Sun Nov 06 2011 01:00:00 GMT-0800 is already on the hour boundary because it has 0 minutes, 0 seconds and 0 milliseconds. If you set the minutes, seconds and milliseconds to zero, it should do nothing, but it changes the timezone offset and the resulting date is Sun Nov 06 2011 01:00:00 GMT-0700. I’ll note that other libraries such as Moment.js do nothing to workaround this browser bug, however. So in Chrome, you’ll see this: moment('2011-11-06T09:00:00.000Z').format() // 2011-11-06T01:00:00-08:00
moment('2011-11-06T09:00:00.000Z').startOf('hour').format() // 2011-11-06T01:00:00-07:00 Whereas in Safari you’ll see the expected result: moment('2011-11-06T09:00:00.000Z').format() // 2011-11-06T01:00:00-08:00
moment('2011-11-06T09:00:00.000Z').startOf('hour').format() // 2011-11-06T01:00:00-08:00 But I’m not sure it’s the right tradeoff to break the behavior around daylight saving boundaries (which occur frequently) in favor of a “more pure” implementation that also happens to (maybe?) work for less common historical times. |
Here is a four-year-old Chrome bug describing the issue: https://bugs.chromium.org/p/chromium/issues/detail?id=428321 |
Hey Mike, I've been running into this and finally landed on that rabbit hole issue in chrome, which led me back here. What does one do at this point? 😂 |
I fiddled around a bit with the DST example above; I think I may have stumbled onto a workaround (I don't know if you are looking for one or not). For me, the trick was to set using the UTC method, then take into account the offset for 1/2-hour time zones. // reproduce problem setting system to "America/Los_Angeles"
date = new Date(Date.UTC(2011, 10, 06, 09, 00)); // 2011-11-06T09:00:00.000Z
date.setMinutes(0, 0, 0); // Should be a no-op!
date; // 2011-11-06T08:00:00.000Z
// first-pass at workaround
date = new Date(Date.UTC(2011, 10, 06, 09, 00)); // 2011-11-06T09:00:00.000Z
date.setUTCMinutes(0, 0, 0); // Should be a no-op!
date; // 2011-11-06T09:00:00.000Z (avoids problem)
// second-pass at workaround to take-into-account Newfoundland (1/2 hour zone)
date = new Date(Date.UTC(2011, 10, 06, 09, 00)); // 2011-11-06T09:00:00.000Z
offset = date.getTimezoneOffset();
date.setUTCMinutes(offset % 60, 0, 0); // Should be a no-op!
date; // 2011-11-06T09:00:00.000Z (still works)
// repeat, having set my system time to "America/St_Johns"
date = new Date(Date.UTC(2011, 10, 06, 04, 30)); // 2011-11-06T04:30:00.000Z
date.setMinutes(0, 0, 0); // Should be a no-op!
date; // 2011-11-06T03:30:00.000Z (repeats the problem)
// repeat, having set my system time to "America/St_Johns"
date = new Date(Date.UTC(2011, 10, 06, 04, 30)); // 2011-11-06T04:30:00.000Z
offset = date.getTimezoneOffset();
date.setUTCMinutes(offset % 60, 0, 0); // Should be a no-op!
date; // 2011-11-06T04:30:00.000Z (still works) |
Thank you for looking into this issue. According to this StackOverflow topic the behaviour of Chrome 67 is not a bug, but desired behaviour (which other browsers might adopt). The current behaviour means that historical dates cannot be rendered well anymore in time scales. I have the same issue as the one referenced above: working with dates in Western European Standard Time, which pre-1900 will have an offset of +19:32, and this means the test in d3-scale.time for whether timeMinutes(date) < date is evaluated true. As I'm not interested to look at time scales lower than a day, can you think of a workaround? |
I have a new idea to solve this in #36 that seems safe and simple. |
For example, given the date 1732-02-22T00:07:00.766:
d3.timeSecond works as expected:
d3.timeMinute should return 1732-02-22T00:07, but doesn’t:
d3.timeHour should return 1732-02-22T00:00, but doesn’t:
https://beta.observablehq.com/d/3a8558e7a5a27d6e
The implementation of timeHour and timeMinute assume that minutes are 60-second and 3,600-second multiples from UNIX epoch, but apparently that’s no longer the case in the latest version of Chrome (leap seconds maybe?), so we may require a slower strategy.
The text was updated successfully, but these errors were encountered: