301738 - Date parse compatibility with MSIE

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

This bug is forked from bug 273292 where a code change was made to make
Spidermonkey's date parsing compatible with MSIE's in a single case, e.g
11/70/2004 but broke compatibility with parsing 2005/04/25.

This bug is to fix the specific compatibility issue and to map out the overall
behavior of MSIE and improve overall compatibility with the minimum changes.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: patch v1

Comment 2

[Bob Clary [:bc] (inactive)]

http://bclary.com/2005/07/26/ contains testdate.html which was used to generate
test results for ff 1.0 (testdate-ff-1.0.out), ff trunk (testdate-ff-trunk.out),
ff trunk with the patch (testdate-ff-new.out) and msie6 (testdate-ie.out). 

The patched ff trunk agrees with MSIE for dates of the form xx/yy/zz. The
patched ff trunk is more lenient for date forms monthname monthday, year than MSIE.

I haven't run the normal js test suite yet, but will when a previous run completes.

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: patch v2

Bug 273292 caused a regression in parsing dates like "2005/04/25" as well as
dates containing monthnames such as "December 2170, 2170".

This updated patch exactly matches IE's behavior for date strings with month
names as well as the m/d/y date strings. 

<http://bclary.com/2005/07/26/> contains an updated test for dates of the form
"\d+/\d+/\d+", "monthname \d+ \d+", "\d+ monthname \d+" and "\d+ \d+
monthname".
and test results showing that with this patch Firefox and IE agree exactly. 

There is probably a much simpler fix especially starting at the code before bug
273292 was checked in.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 190683 [details] [diff] [review]
patch v2

>-            } else if (mday < 0) {
>+            }
>+            else if (mon < 0) {

Nit: else on the same line as the }, please.

>+    /*
>+      Case 1. The input string is of the form "f/m/l" where f, m and l are
>+      Case 2. The input string contains an Enlish month name.
>+    */
>+    if (seenmonthname) {

Please either switch this comment (so that case 2 is case 1) or the code around
so that the cases correspond to the code.

r=me, though I'm requesting an additional review from shaver, since he might
actually understand this code and know something I don't about it.

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: patch v3 with mrbkap's comments addressed.

This is essentially the same as v2, I am just attaching to make sure it is not
lost.

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 190683 [details] [diff] [review]
patch v2

This (excellent) comment has its cases in the opposite order to the code:

>+    /*
>+      Case 1. The input string is of the form "f/m/l" where f, m and l are
>+              integers, e.g. 7/16/45.
>+              Adjust the mon, mday and year values to achieve 100% MSIE 
>+              compatibility.
>+              a. If 0 <= f < 70, f/m/l is interpreted as month/day/year.
>+                 i.  If year < 100, it is the number of years after 1900
>+                 ii. If year >= 100, it is the number of years after 0.
>+              b. If 70 <= f < 100
>+                 i.  If m < 70, f/m/l is interpreted as
>+                     year/month/day where year is the number of years after
>+                     1900.
>+                 ii. If m >= 70, the date is invalid.
>+              c. If f >= 100
>+                 i.  If m < 70, f/m/l is interpreted as
>+                     year/month/day where year is the number of years after 0.
>+                 ii. If m >= 70, the date is invalid.
>+      Case 2. The input string contains an Enlish month name.

"English"

>+              The form of the string can be month f l, or f month l, or
>+              f l month which each evaluate to the same date. 
>+    */



>+        if (mday > year) {
>+            temp = year;
>+            year = mday;
>+            mday = temp;
>+        }

This isn't mentioned in the comment; is it really what IE does?

03 Aug 05 would then be interpreted as "5th August, 2003" instead of
(what I would expect) "3rd August, 2005".  If we need to do this for IE compat,
then r=shaver.	(With comment nits picked, natch.)

Comment 7

[Bob Clary [:bc] (inactive)]

(In reply to comment #6)
> (From update of attachment 190683 [details] [diff] [review] [edit])

> 
> >+        if (mday > year) {
> >+            temp = year;
> >+            year = mday;
> >+            mday = temp;
> >+        }
> 
> This isn't mentioned in the comment; is it really what IE does?
> 
> 03 Aug 05 would then be interpreted as "5th August, 2003" instead of
> (what I would expect) "3rd August, 2005".  If we need to do this for IE compat,

Actually it is for Firefox 1.0.x compatibility as well. It appears to use the
greater of the two numbers for the year. I will add that to the comment. 

Actually new Date('03 Aug 05') is an invalid date in both due to both values
being below 70. But new Date('03 Aug 70') and new Date('70 Aug 03') agree as
does new Date('03 Aug 2005') and new Date('2005 Aug 03').

New patch coming up.

Comment 8

[Bob Clary [:bc] (inactive)]

Attachment: patch v4 with shaver's comments addressed.

Comment 9

[Bob Clary [:bc] (inactive)]

Comment on attachment 190923 [details] [diff] [review]
patch v4 with shaver's comments addressed.

transfering the r+

Comment 10

[Bob Clary [:bc] (inactive)]

Requesting approval for patch v4 for Firefox 1.5 beta to make Firefox match
MSIE's date parsing for dates of the form mm/dd/yy, monthname dd yy, dd
monthname yy, and dd yy monthname. This fixes a regression introduced in bug
273292 where dates of the form 2005/04/25 were not parsed correctly as well as
in parsing dates with monthnames. Full testing results
<http://bclary.com/2005/07/26/>show 100% agreement with MSIE and no regressions
in the javascript test library.

Comment 11

[Bob Clary [:bc] (inactive)]

Checking in jsdate.c;
/cvsroot/mozilla/js/src/jsdate.c,v  <--  jsdate.c
new revision: 3.69; previous revision: 3.68
done

Comment 12

[Bob Clary [:bc] (inactive)]

RCS file: /cvsroot/mozilla/js/tests/js1_5/Date/regress-301738-01.js,v
done
Checking in regress-301738-01.js;
/cvsroot/mozilla/js/tests/js1_5/Date/regress-301738-01.js,v  <-- 
regress-301738-01.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/js/tests/js1_5/Date/regress-301738-02.js,v
done
Checking in regress-301738-02.js;
/cvsroot/mozilla/js/tests/js1_5/Date/regress-301738-02.js,v  <-- 
regress-301738-02.js
initial revision: 1.1
done

Comment 13

[Bob Clary [:bc] (inactive)]

v ff 1.5.0.1, 1.5, 1.6a1 20060217 win/linux/mac