Bug 13472 – Misparsing date in javascript leads to year value of -1

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[John McKerrell]

Simple test case given in the URL, copied here in case of problems:
   javascript:void(alert(new Date( '24 May,2007 15:35:00' ) ) )

If a space is added after the comma, the parse succeeds, without the space it gives the incorrect year.

[David Kilzer (:ddkilzer)]

Confirmed with a local debug build of WebKit r21060 on Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135).

Firefox 2.0.0.3 parses it correctly, as does Mac IE 5.2.3 and Opera 9.10.

OmniWeb 5.5.3 suffers from the same bug as WebKit.

[David Kilzer (:ddkilzer)]

Not a regression as the same misparsing occurs in shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.9 (8P135).

[Rob Buis]

Created attachment 16315 [details]
First attempt

This patch should fix the problem. However there are some things I am wondering about:

- mozilla is very liberal with ',', basically making it act as whitespace and things like this work:
javascript:void(alert(new Date( '24 May,,,,,,2007 15:35:00' )))
I wonder if we want to go that way?

- not sure where to add/change a testcase for this?

Cheers,

Rob.

[Darin Adler]

Comment on attachment 16315 [details]
First attempt

I suggest putting the test case in LayoutTests/fast/js.

[Rob Buis]

Created attachment 16316 [details]
Updated patch

Updating the patch with a fix for bug 14176, since they are pretty similar.
Cheers,

Rob.

[David Kilzer (:ddkilzer)]

You should also run WebKitTools/Scripts/run-javascript-tests to make sure there are no regressions and to update the results if there are progressions!

[Rob Buis]

Created attachment 16321 [details]
Now with testcase

I updated the patch with some added tests in the directory Darin pointed out.
Cheers,

Rob.

[Adam Roben (:aroben)]

Comment on attachment 16321 [details]
Now with testcase

+            while (*dateString && (*dateString != '-' && *dateString != ',') && !isspace(*dateString))

The parentheses around (*dateString != '-' && *dateString != ',') should not be necessary.

+            if (*dateString != '-' && *dateString != '/' && !isspace(*dateString) && *dateString != ',')

It would be a little nicer to put the comma check after the slash check, but that's a real nitpick.

[Adam Roben (:aroben)]

Comment on attachment 16321 [details]
Now with testcase

r=me even if the above comments aren't fixed.

Please land this on feature-branch.

[Rob Buis]

Hi Adam,

(In reply to comment #9)
> (From update of attachment 16321 [details] [edit])
> r=me even if the above comments aren't fixed.
> 
> Please land this on feature-branch.

I just did, including taking your comments into account. Unfortunately I had to tweak it slightly
to make it pass the test, I am not sure why, but the same fix works on trunk.
Cheers,

Rob.

[Rob Buis]

I guess this can be closed now.
Cheers,

Rob.