Skip to content
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

startOf('hour') advances to the next hour during DST fall back #1990

Closed
leonyu opened this issue Oct 16, 2014 · 17 comments · Fixed by #4338
Closed

startOf('hour') advances to the next hour during DST fall back #1990

leonyu opened this issue Oct 16, 2014 · 17 comments · Fixed by #4338

Comments

@leonyu
Copy link

leonyu commented Oct 16, 2014

When in DST timezone, .startOf('hour') advances to the next hour during DST fall back
screen shot 2014-10-16 at 3 52 37 pm

@leonyu
Copy link
Author

leonyu commented Oct 16, 2014

further investigation it appears that this is rooted in the usage of Date.prototype.set{XXX}.

> new Date('2014-11-02T01:00:00-04:00')
Sun Nov 02 2014 01:00:00 GMT-0400 (EDT)
> new Date(new Date('2014-11-02T01:00:00-04:00').setSeconds(0))
Sun Nov 02 2014 01:00:00 GMT-0500 (EST)

@leonyu
Copy link
Author

leonyu commented Oct 16, 2014

I have a current local hack:

moment.fn.startOfThis = function(unit) {
  if (this > this.clone().startOf(unit)) {
    return this.startOf(unit);
  } else {
    return this.subtract(1, unit).startOf(unit).add(1, unit);
  }
};

@JobaDiniz
Copy link

That's not the only issue. Brazil DST is today (18/10) to Sunday(19/10), and if one add 1 day to today date (18/10 00h00m), the resulted date will not be 19/10, rather will be 18/10 23h00m.

@trevvvy
Copy link

trevvvy commented Oct 19, 2014

This seems to be related. I'm seeing wrong DST handling in Safari, but correct in Chrome and Firefox.

Chrome (Correct):

moment("2014-11-02").hour(1).format()
"2014-11-02T01:00:00-07:00"
moment("2014-11-02").hour(2).format()
"2014-11-02T02:00:00-08:00"
moment("2014-11-02").hour(3).format()
"2014-11-02T03:00:00-08:00"

Safari (Incorrect). Hour 1 and 2 both report 1am and hour 3 is an hour off.

moment("2014-11-02").hour(1).format()
"2014-11-02T01:00:00-07:00"
moment("2014-11-02").hour(2).format()
"2014-11-02T01:00:00-08:00"
moment("2014-11-02").hour(3).format()
"2014-11-02T02:00:00-08:00"

@leonyu
Copy link
Author

leonyu commented Oct 19, 2014

After more testing, the bug occurs in all major browsers. Some browsers has it during the first DST hour, while others has it during the 2nd DST hour

I have wrote mocha test to verify this:
http://jsfiddle.net/voidvector/0pqzeLnm/

@ichernev
Copy link
Contributor

Well, one of the ways to handle that is for all operations to be relative (instead of set, you get the current, compute the offset, and then add (in milliseconds)). Other than that, browsers behave very weirdly around DST, so working around just for the startOf method doesn't make sense.

I actually started a project (https://github.com/ichernev/utc_date) to implement Date without all the DST madness, and I was planning to use it inside moment at some point, but I haven't had time to work on it.

So if there is enough demand I can set up a TODO list and some documentation and we can get it going.

@trevvvy
Copy link

trevvvy commented Oct 24, 2014

This issue motivated us to adjust our app to work exclusively in UTC mode, which avoids these issues. moment.utc combined with our JSON api being 100% zoneless, is working great for our needs. Although, our app always operates in the timezone of the server-side account, and never the browser. So, this approach would not work for everyone.

@leonyu
Copy link
Author

leonyu commented Oct 24, 2014

@leonyu
Copy link
Author

leonyu commented Oct 30, 2014

I believe this can be resolved by updating rawSetter to either

  1. always use UTC
  2. create an Date object from scratch using the constructor.
    I can submit a PR with test cases if you like.

@ichernev
Copy link
Contributor

@leonyu you mean

  1. To get the token in UTC, calculate diff and then set -- this means make twice as many ops ... not that big of a deal
  2. create a date object from scratch? You mean get all the current components (year, month, day, hour, minute, second), then get the current value and replace it? This is even weirder.

Browsers are buggy even when creating new objects (in local time) -- I've seen that in safari at least. So I'd go with making a working Date object that only uses the DST information from the builtin Date (creating UTC Date always works, then you ask about the timezone offset).

@leonyu
Copy link
Author

leonyu commented Nov 5, 2014

For constructor, I meant using string constructor, because none of the other means of construction support TZ offset.

@jamesdixon
Copy link

hi, any update or workaround for this? just ran into this issue with the recent shift from MST to MDT. Using startOf causes the offset to shift forward by 1 hour.

@rntdrts
Copy link

rntdrts commented Mar 16, 2017

I'm not sure if it is related to this topic, but as you can see on the displayed image below setting the hour to one in the morning automatically increases up to one hour.

moment_bug

@darakeon
Copy link

@leonyu , test again, the endOf problem seems fixed.

@leonyu
Copy link
Author

leonyu commented Feb 15, 2018

Still bugged in Moment 2.20.1, at least on Chrome & Firefox when current local time is in EST (winter North American timezone).

Firefox attempted a fix a few months ago, which appear to have only fixed the issue when current local time is in EDT (summer North American timezone).

Here's my test case using Moment 2.20.1 (require your local timezone to observe DST, as it attempts to find the "DST fallback" programmatically):
https://jsfiddle.net/voidvector/yqsu73nq/7/
All the unit tests attempt to do is call startOf('hour') and endOf('hour') when the moment object is on the DST boundary.

@redfellow
Copy link

I'm seeing some related weirdness with startOf('day') and adding hours to it: https://vgy.me/XRPcS9.png

@leonyu
Copy link
Author

leonyu commented Mar 13, 2018

I had further back-and-forth on Mozilla's Bugzilla board since my last post a month ago.

The spec contains pseudo-code which all the browsers implement. The logic of the pseudo-code doesn't account for DST (missing hour/overlapping hour).

Mozilla developers' position is that they are following the spec. So IMO this problem is unlikely to resolve on browser vendor's end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants