Bug 75692

Summary: [Qt] REGRESSION(r104267): fast/js/date-constructor.html fails
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, fpizlo, loki, ossy, szledan, webkit-sed, zherczeg
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 75892    
Bug Blocks: 70998    
Attachments:
Description Flags
Fixed some lines in the test none

Description Csaba Osztrogonác 2012-01-06 02:29:10 PST
After r104267 fast/js/date-constructor.html started to fail on Qt:
--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/js/date-constructor-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/fast/js/date-constructor-actual.txt 
@@ -31,7 +31,7 @@
 PASS Number(new Date(new Date(1, 1, 1, 1, Infinity, 1, 1, 1, 1))) is Number.NaN
 PASS Number(new Date(new Date(1, 1, 1, 1, 1, Infinity, 1, 1, 1))) is Number.NaN
 PASS Number(new Date(new Date(1, 1, 1, 1, 1, 1, Infinity, 1, 1))) is Number.NaN
-PASS Number(new Date(new Date(1, 1, 1, 1, 1, 1, 1, 1, Infinity))) is -2174741938999
+FAIL Number(new Date(new Date(1, 1, 1, 1, 1, 1, 1, 1, Infinity))) should be -2174741938999. Was -2174770738999.
 PASS testStr is "1234567"
 PASS testStr is "1234567"
 PASS successfullyParsed is true
Comment 1 Csaba Osztrogonác 2012-01-06 04:28:18 PST
I skipped it to paint the Qt bot green: http://trac.webkit.org/changeset/104283

Could you guys check this regression?
Comment 2 Gavin Barraclough 2012-01-07 18:53:11 PST
(In reply to comment #1)
> I skipped it to paint the Qt bot green: http://trac.webkit.org/changeset/104283
> 
> Could you guys check this regression?

Hey Ossy,

Will do.  I don't think this is a regression as such - probably an error in a new test case I added.  I think it's probably not being timezone-agnostic, I'm guessing my new test case probably only works in California time.  Should be an easy fix.

cheers,
G.
Comment 3 Csaba Osztrogonác 2012-01-09 06:27:54 PST
Our bots are in California timezone to avoid this kind of problems.
( export TZ=/usr/share/zoneinfo/America/Los_Angeles )
Comment 4 Gavin Barraclough 2012-01-09 14:43:39 PST
Hmm, this last test case that I added wasn't particularly useful from the perspective of testing the bug fixed by the patch that introduced it, so I think it can just be removed for now.

I haven't yet figured out whether this is indicating a real bug, or is an error in the test itself.  Firefox produces a different result here too, but it looks like it's getting it wrong.  It shows PST as having been -0700 in 1901, however the spec prohibits use of historic data here, and requires that DST calculations are based on the current algorithm for the geographic region.

I'm going to open a new bug to track this.

Removing the offending test in r104492.
Comment 5 Gavin Barraclough 2012-01-09 14:48:25 PST
Bugzilla tracking the possible bug being exposed is here: https://bugs.webkit.org/show_bug.cgi?id=75892
Comment 6 Szilard Ledan 2012-01-19 01:03:34 PST
The 'Number(new Date(new Date(1, 1, 1, 1, 1, 1, 1, 1, Infinity)))' line fails due to the time zone difference.  The last two arguments of the DateConstructor (8. and 9.) isn't even used, so this line is equivalent to the 'Number(new Date(new Date(1, 1, 1, 1, 1, 1, 1)))', but the timezone is always added to it.
The older version of this test uses '(new Date(1, 1, 1, 1, 1, 1, 1) - timezone)', and it doesn't fail.
Comment 7 Szilard Ledan 2012-01-20 05:56:12 PST
Created attachment 123300 [details]
Fixed some lines in the test

This test is timezone dependent, the incomplete lines have been corrected.
'Infinity' used int the first seven parameters does not have any effect.
Line 'Number(new Date(new Date(1, 1, 1, 1, 1, 1, 1, 1, Infinity)).getTime() - timeZoneOffset)' works the same way as Number(new Date(new Date(1, 1, 1, 1, 1, 1, 1)).getTime() - timeZoneOffset).
Comment 8 Csaba Osztrogonác 2012-01-20 05:59:06 PST
But why do we still pass 9 parameters to the Date candtructor when it handles only 7 parameters?
Comment 9 Eric Seidel (no email) 2012-01-30 16:01:34 PST
Comment on attachment 123300 [details]
Fixed some lines in the test

Cleared review? from attachment 123300 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).