Summary: | date setUTCFullYear to setUTCMinutes adding 1 hour in time zone 0 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | toby lewis <tobylewis> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, ddkilzer, eric, ian, kmccullough | ||||||||
Priority: | P2 | ||||||||||
Version: | 417.x | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Attachments: |
|
Description
toby lewis
2006-06-21 09:25:52 PDT
Created attachment 8955 [details]
test case
Same test, as an attachment.
Confirmed. TOT behavior seems to be even more broken, needs investigation. Created attachment 8988 [details] First attempt To me it seems related to BST handling in 1970(which is default for t3): http://en.wikipedia.org/wiki/British_Summer_Time Now this is not a problem in itself, but the tm structure seems to indicate summer time, even at 1-1-1970! I guess that is correct given the wikipedia page, but it is not helping our calculation. One way I found to fix it is to just use the current time, assuming that from now on we stick to winter and summertime as we know it since years. I am welcome to input on how to test and suggestions for better, or likely more efficient fixes. One way could be to single out the BST case? Cheers, Rob. Comment on attachment 8988 [details]
First attempt
I don't like the fact that the call to localtime_r is now dependent on the current time. Perhaps that's a clever elegant solution, but I'm worried that makes this function "less deterministic". And the local variable clearly should not be named zero if it's going to be used this way.
We really want the utcOffset at time t, not the utcOffset at the current time.
I'm not absolutely certain of this, but I think I'm certain enough to say review-.
Feel free to research further and put this up for review again if I'm wrong.
I don't know anything about the source code, but just from a logical point of view, UTC is UTC and should have nothing to do with time zones or daylight savings. The setUTC functions should be far simpler than the locale set functions. The way I have worked around the problem in javascript was simply to do the millisecond calculations myself and use mydate.setTime() which only needed about twenty lines of code. Created attachment 9058 [details]
Improved patch
This patch seems to work better, it uses the utcOffset info of the new date, instead of 1/1/1970.
It may need more testing though.
Cheers,
Rob.
Comment on attachment 9058 [details]
Improved patch
Makes no sense for this variable to be named "zero" any more if it's not zero. So you should fix that. I also don't understand quite how this fixes the bug. A comment explaining that would be welcome.
This may be related to Bug 12057 and r18514. http://trac.webkit.org/projects/webkit/changeset/18514 (In reply to comment #8) > This may be related to Bug 12057 and r18514. > http://trac.webkit.org/projects/webkit/changeset/18514 It may already be fixed as well. (In reply to comment #9) > It may already be fixed as well. Then again, I doubt this is fixed, although the fix would be to use Jan 1, 2000 as the basis for UTC timezone offset instead of Jan 1, 1970 for the same reasons as bug 12057 used that date. (In reply to comment #10) > (In reply to comment #9) > > It may already be fixed as well. > > Then again, I doubt this is fixed, although the fix would be to use Jan 1, 2000 > as the basis for UTC timezone offset instead of Jan 1, 1970 for the same > reasons as bug 12057 used that date. No, it's fixed. :) The original failure was probably fixed by r16780, and then the UTC time zone issue was fixed by r18514. http://trac.webkit.org/projects/webkit/changeset/16780 |