Bug 4142 - Date object does not always adjust daylight savings correctly
Summary: Date object does not always adjust daylight savings correctly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-07-26 11:06 PDT by Carsten Guenther
Modified: 2005-10-24 13:45 PDT (History)
2 users (show)

See Also:


Attachments
Patch (957 bytes, patch)
2005-10-09 14:58 PDT, Geoffrey Garen
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carsten Guenther 2005-07-26 11:06:56 PDT
This will fix

    ecma/Date/15.9.4.3.js
    ecma/Date/15.9.5.28-1.js

There seems to be a problem with daylight savings when a time change crosses the daylight savings start/
end dates.
Comment 1 Geoffrey Garen 2005-07-26 11:15:11 PDT
(In reply to comment #0)

FYI, Mac IE has the same problem. Win IE and Mac FF don't.
Comment 2 George Staikos 2005-10-03 12:35:01 PDT
Adding this before the return in KJS::makeTime() fixes it for me:  
  
    // Determine if we passed over a DST change boundary  
    if (!utc) { 
      time_t tval = mktime(t) + utcOffset + int((ms + yearOffset)/1000);  
      struct tm t3;  
      localtime_r(&tval, &t3);  
      t->tm_isdst = t3.tm_isdst;  
    } 
  
Does not seem to introduce any regressions.  I am a bit worried about overflow  
and rounding though.  It might break down very close to the changeover period.  
  
Comment 3 Geoffrey Garen 2005-10-09 14:58:00 PDT
Created attachment 4271 [details]
Patch

Implemented George's suggestion.
Comment 4 Darin Adler 2005-10-09 16:00:35 PDT
Comment on attachment 4271 [details]
Patch

What's the rationale for calling the local variable "t3"?

The FIXME says "on some systems", but this will happen on all systems; there's
no system where time_t has enough range to include all the years we want our
function to handle. So I suggest re-wording it. Otherwise, looks fine.
Comment 5 George Staikos 2005-10-10 15:47:43 PDT
There is already a t and a t2, and although t3 wasn't perhaps the best sequel, 
I didn't see fit to leave it out. :-) 
 
And yes, "all systems" is correct.  I don't know if it's a realistic problem 
to put more effort into at this time though. 
Comment 6 John Sullivan 2005-10-24 13:45:37 PDT
I checked this in. It did fix ecma/Date/15.9.5.28-1.js, but ecma/Date/15.9.4.3.js was apparently fixed by 
some earlier checkin.