Date.parse() should ignore comments in parentheses anywhere white space is allowed, following RFC 2822. For example, Date.parse("Oct(ober)4 (today)1995 (10 years ago)") should be the same as Date.parse ("Oct 4 1995"), and not NaN as it currently is.
Actually, "following RFC 2822" would not be "anywhere white space is allowed", but I think Firefox allows is more permissive than the RFC.
Created attachment 4209 [details] allow comments where spaces are allowed This is more permissive than the RFC, to match Firefox
Created attachment 4211 [details] testcase for this
Comment on attachment 4209 [details] allow comments where spaces are allowed Nice catch on this bug. I have two concerns: 1) I think your logic is wrong -- it doesn't seem to support dates of the form "Wed (comment 1) (comment 2) Sep 28 2005", which FF does support. (Correct me if I'm wrong - I haven't tested.) 2) The nesting of the &&'s and ||'s at the end is pretty confusing at first glance. I think this might work better: while ((ch = *s++)) { if (ch == '(') nesting++; else if (ch == ')') nesting--; else if (!isspace(ch) && nesting == 0) break; }
PS I think you should add some "Wed (comment 1) (comment 2) Sep 28 2005" cases to the test.
Actually, while (ch = *s++) won't work if there are no spaces or comments. Instead: while ((ch = *s)) { if (ch == '(') nesting++; else if (ch == ')') nesting--; else if (!isspace(ch) && nesting == 0) break; s++; }
(In reply to comment #4) > Nice catch on this bug. Can't take credit for that, actually. > 1) I think your logic is wrong -- it doesn't seem to support dates of the form > "Wed (comment 1) (comment 2) Sep 28 2005", which FF does support. (Correct me > if I'm wrong - I haven't tested.) Tested. It is supported. I'll add it to the test. > if (ch == '(') > nesting++; > else if (ch == ')') > nesting--; > else if (!isspace(ch) && nesting == 0) > break; Is not good because it doesn't break if we encounter a ')' when nesting is 0.
Actually, while the patch allows "Wed (comment 1) (comment 2) Dec 25 1995 1:30 GMT" it doesn't allow "Wed(comment 1) (comment 2) Dec 25 1995 1:30 GMT" which Firefox does. On the other hand, WebKit allows and Firefox rejects "Junk Dec 25 1995 1:30 GMT" while WebKit rejects and Firefox allows "We Dec 25 1995 1:30 GMT" The latter couple of cases seem to be outside the scope of this bug, though. I'll see what I can do about the Wed(comment) case.
Created attachment 4215 [details] revised patch
Created attachment 4216 [details] testcase Added Wed(comment 1)... and We(comment 1)... cases
Comment on attachment 4215 [details] revised patch Updated to allow comments between "day of week" and month. I didn't change skipSpacesAndComments (see my previous comments).
Comment on attachment 4215 [details] revised patch I'm surprised to learn that dates can include parenthesized comments, but this is a very nice way to fix the bug. Brace should be on the line after the function, but otherwise, looks good. r=me
Comment on attachment 4215 [details] revised patch I'm marking this R- so that nobody lands it yet -- Mitz and I discussed some changes on IRC, and he'll submit a new patch.
Created attachment 4217 [details] revised patch Corrected style and also made skipSpacesAndComments more readable, as suggested by ggaren.
I landed the fix, with one last tweak I couldn't resist (check isspace first before checking for '(' and ')').