Bug 49989

Summary: REGRESSION: Date.parse("Tue Nov 23 20:40:05 2010 GMT") returns NaN
Product: WebKit Reporter: ravager
Component: JavaScriptCoreAssignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, aestes, ap, christopher.reiss, commit-queue, edwardjsabol, ggaren, kling, oliver, webkit.review.bot
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
Bug Depends on:    
Bug Blocks: 55228    
Attachments:
Description Flags
simple fix
none
simple fix
darin: commit-queue-
comment fix and a few extra tests added
none
Updated version of Chris's patch, allow year in either position. oliver: review+

ravager
Reported 2010-11-23 13:14:27 PST
Date.parse("Tue Nov 23 20:40:05 2010 GMT") returns NaN in Safari 5, but not in Safari 4 (or other browsers).
Attachments
simple fix (3.71 KB, patch)
2011-02-15 08:48 PST, chris reiss 		no flags
DetailsFormatted DiffDiff
simple fix (3.71 KB, patch)
2011-02-15 08:56 PST, chris reiss 		darin: commit-queue-
DetailsFormatted DiffDiff
comment fix and a few extra tests added (4.22 KB, patch)
2011-02-17 11:27 PST, chris reiss 		no flags
DetailsFormatted DiffDiff
Updated version of Chris's patch, allow year in either position. (4.64 KB, patch)
2012-03-09 12:47 PST, Gavin Barraclough 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2010-11-23 23:07:33 PST
Confirmed as a difference with Firefox 3.6.
Alexey Proskuryakov
Comment 2 2011-01-20 16:47:07 PST
<rdar://problem/8895888>
Geoffrey Garen
Comment 3 2011-01-20 17:00:36 PST
The bug here is that the parser parses the timezone as "2010 GMT".
chris reiss
Comment 4 2011-02-14 12:35:53 PST
reproducible in QtWebBrowser as well but not Chrome. Taking a look ...
chris reiss
Comment 5 2011-02-15 08:48:47 PST
Created attachment 82464 [details] simple fix
WebKit Review Bot
Comment 6 2011-02-15 08:52:42 PST
Attachment 82464 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/JavaScriptCore/wtf/DateMath.cpp:927: Missing spaces around = [whitespace/operators] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
chris reiss
Comment 7 2011-02-15 08:56:30 PST
Created attachment 82467 [details] simple fix
Darin Adler
Comment 8 2011-02-15 08:59:19 PST
Comment on attachment 82467 [details] simple fix View in context: https://bugs.webkit.org/attachment.cgi?id=82467&action=review I’m not going to say review- just for a comment, but I don’t feel comfortable doing commit-queue+ on this so for now I am doing commit-queue-. Someone else can reverse that if we discuss and decide to land this exactly as-is. > LayoutTests/fast/js/script-tests/date-parse-test.js:130 > +testDateParse("Wed Dec 25 1:30 1995 GMT", "819855000000"); I am concerned that we are adding only this single test case that is a well formatted date. I am more interested in more ambiguous cases that end up being treated as years. We need to make sure we cover not only legal cases but unusual ones as well. Can someone add some more test cases? > Source/JavaScriptCore/wtf/DateMath.cpp:924 > + // the year may be before the timezone WebKit project style uses a sentence style for comments. And this one is a sentence. I suggest this comment instead. // The year may be after the time but before the time zone.
chris reiss
Comment 9 2011-02-17 11:27:14 PST
Created attachment 82835 [details] comment fix and a few extra tests added
Andreas Kling
Comment 10 2011-02-18 02:58:42 PST
Comment on attachment 82835 [details] comment fix and a few extra tests added r=me, and thanks for adding more tests.
WebKit Commit Bot
Comment 11 2011-02-18 17:43:59 PST
Comment on attachment 82835 [details] comment fix and a few extra tests added Clearing flags on attachment: 82835 Committed r79060: <http://trac.webkit.org/changeset/79060>
Adele Peterson
Comment 12 2011-06-20 11:28:58 PDT
This broke searching for flights at United Airlines. I recommend rolling this out while we investigate further.
Adele Peterson
Comment 13 2011-06-20 11:35:13 PDT
Filed: https://bugs.webkit.org/show_bug.cgi?id=63003
Adele Peterson
Comment 14 2011-06-20 11:41:29 PDT
Rollout patch posted: https://bugs.webkit.org/attachment.cgi?id=97829&action=review
Oliver Hunt
Comment 15 2011-06-20 12:49:03 PDT
This broke united airlines and has been rolled out in r89281
Andy Estes
Comment 16 2011-06-20 12:54:39 PDT
Here is the string that united.com attempts to parse: Mon Jun 20 11:01:37 CDT 2011 Parsing this string via Date.parse() fails after r79060 but should not (works in Safari 5 and Firefox). If someone tries to re-land a patch for this bug, please make sure strings such as the one above and the one in <https://bugs.webkit.org/show_bug.cgi?id=55228> can be parsed.
Oliver Hunt
Comment 17 2011-06-20 13:00:22 PDT
(In reply to comment #16) > Here is the string that united.com attempts to parse: > > Mon Jun 20 11:01:37 CDT 2011 > > Parsing this string via Date.parse() fails after r79060 but should not (works in Safari 5 and Firefox). > > If someone tries to re-land a patch for this bug, please make sure strings such as the one above and the one in <https://bugs.webkit.org/show_bug.cgi?id=55228> can be parsed. I added this string to the date parsing tests.
Darin Adler
Comment 18 2011-07-16 09:16:33 PDT
Comment on attachment 82467 [details] simple fix Clear review flag since this was rolled out.
Gavin Barraclough
Comment 19 2012-03-09 12:47:27 PST
Created attachment 131077 [details] Updated version of Chris's patch, allow year in either position.
Gavin Barraclough
Comment 20 2012-03-09 14:32:12 PST
Fixed in r110331
Note You need to log in before you can comment on or make changes to this bug.