-
Notifications
You must be signed in to change notification settings - Fork 58
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
interval.range should protect against infinite loops. #25
Comments
A reduction of this issue (requires switching to Brasilia Standard Time): date = new Date("2017-10-08T03:00:00.000Z") // Date 2017-10-08T03:00:00.000Z
d3.timeWeek(date) // Date 2017-10-08T03:00:00.000Z
d3.timeWeek.offset(date, 1) // Date 2017-10-15T02:00:00.000Z
d3.timeWeek(d3.timeWeek.offset(date, 1)) // Date 2017-10-08T03:00:00.000Z |
Actually, it’s even worse in that d3.timeWeek is not idempotent: date = new Date("2017-10-15T03:00:00.000Z") // Date 2017-10-15T03:00:00.000Z
d3.timeWeek(date) // Date 2017-10-15T02:00:00.000Z
d3.timeWeek(d3.timeWeek(date)) // Date 2017-10-08T03:00:00.000Z |
In vanilla JavaScript (Firefox): date = new Date("2017-10-15T03:00:00.000Z") // Date 2017-10-15T03:00:00.000Z
date.setHours(0, 0, 0, 0) // Date 2017-10-15T02:00:00.000Z
date.setHours(0, 0, 0, 0) // Date 2017-10-14T03:00:00.000Z Whereas in Chrome: date = new Date("2017-10-15T03:00:00.000Z") // Date 2017-10-15T03:00:00.000Z
date.setHours(0, 0, 0, 0) // Date 2017-10-15T03:00:00.000Z
date.setHours(0, 0, 0, 0) // Date 2017-10-15T03:00:00.000Z So in other words, calling date.setHours in Firefox is changing the date from the 15th to the 14th. |
(d3.timeDay is also not idempotent in Firefox, for the same reasons.) |
I’m not sure it’s feasible to return the correct result in Firefox; in general, D3 is not a compatibility layer, so I rarely try to workaround a bug in date.setHours (and I think this counts as a bug… although perhaps I am misreading the spec). That said, if there’s a reasonably simple workaround, I’d be amenable to using it—I just can’t think of one immediately. I’d also like to guarantee that interval.range can’t go into an infinite loop, but I think that would still mean that Firefox would have incorrect results. I’ll change this issue title accordingly. |
Any plans to make a new release soon with this patch? |
Thanks for the reminder. Will try to do this soon… |
Is there any update on this? We are blocked on releasing a project as this is still a problem with Firefox 57. Thank you! |
Will try today. Thanks for the reminder. |
The code above works fine on chrome, but creates an infinite loop on firefox (-v 52.2.0, tested on two different linux machines).
Doing some debugging I ended up here, the do-while never halts. Placing some console.log calls in the code I got the following:
firefox:
so
offseti
increases2017-10-08T03:00:00.000Z
to2017-10-15T02:00:00.000Z
and thenfloori
decreases it back to2017-10-08T03:00:00.000Z
and this keeps going on forever.for chrome we have:
The text was updated successfully, but these errors were encountered: