Bug 5514 - Date conversion to local time gets the DST flag wrong for some dates
Summary: Date conversion to local time gets the DST flag wrong for some dates
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-26 09:58 PDT by mitz
Modified: 2005-11-30 05:26 PST (History)
0 users

See Also:


Attachments
Testcase (1.07 KB, text/html)
2005-10-26 10:00 PDT, mitz
no flags Details
Apply the realYearOffset hack only when absolutely necessary (1.35 KB, patch)
2005-10-28 03:08 PDT, mitz
darin: review-
Details | Formatted Diff | Diff
Apply the realYearOffset hack only when absolutely necessary (1.50 KB, patch)
2005-10-28 13:49 PDT, mitz
no flags Details | Formatted Diff | Diff
Better patch (1.47 KB, patch)
2005-10-29 01:46 PDT, mitz
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2005-10-26 09:58:55 PDT
For dates before 1970-01-01 00:00 UTC, conversion to local time takes the DST flag from the same day in 
a different year. This leads to incorrect results in many cases, since DST cutoff dates are not the same 
every year.

The test case demonstrates this for several time zones: now that bug 4142 is fixed, "new Date(y,m,d)" 
works as expected, but the conversion back to local time is wrong due to using DST status from the same 
date in 2002.

Since the fix for bug 4142 involves passing negative and large positive values to localtime_r(), I'm not sure 
why the same isn't done in DateProtoFuncImp::callAsFunction().
Comment 1 mitz 2005-10-26 10:00:03 PDT
Created attachment 4484 [details]
Testcase
Comment 2 Geoffrey Garen 2005-10-26 14:17:11 PDT
Mitz mentioned on IRC that eliminating the yearOffset hack fixes this bug. But that would make the code 
slightly less portable.
Comment 3 mitz 2005-10-28 03:08:25 PDT
Created attachment 4506 [details]
Apply the realYearOffset hack only when absolutely necessary
Comment 4 mitz 2005-10-28 03:11:03 PDT
Comment on attachment 4506 [details]
Apply the realYearOffset hack only when absolutely necessary

Haven't tested it on any platform other than PPC OS X.
Comment 5 Darin Adler 2005-10-28 10:42:59 PDT
Comment on attachment 4506 [details]
Apply the realYearOffset hack only when absolutely necessary

This line of code:

+const bool time_tIsSigned = isTime_tSigned();

will result in code run at load time. We don't allow that in JavaScriptCore and
WebCore. We'll need an alternate approach that does the same thing without a
global that requires executing code at library load time.
Comment 6 mitz 2005-10-28 13:49:32 PDT
Created attachment 4510 [details]
Apply the realYearOffset hack only when absolutely necessary

Initialize time_t limits only the first time they're actually needed.
Comment 7 Geoffrey Garen 2005-10-28 17:25:12 PDT
Here's an idea:

static bool isTime_tSigned()
{
  time_t minusOne == -1;
  return minusOne < 0;
}
Comment 8 Geoffrey Garen 2005-10-28 17:26:37 PDT
(In reply to comment #7)

s/==/=/
Comment 9 Darin Adler 2005-10-28 18:25:05 PDT
Comment on attachment 4510 [details]
Apply the realYearOffset hack only when absolutely necessary

Looks good. r=me
Comment 10 mitz 2005-10-29 01:37:00 PDT
(In reply to comment #7)
> Here's an idea:
>   time_t minusOne = -1;

Clever! But the above will give a compiler warning if time_t is unsigned. This should work:

    time_t minusOne = (time_t)(-1);
Comment 11 mitz 2005-10-29 01:46:45 PDT
Created attachment 4517 [details]
Better patch
Comment 12 mitz 2005-10-29 01:48:32 PDT
Comment on attachment 4517 [details]
Better patch

Did as Geoffrey suggested