Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date#toISOString/Native Date object bugs #111

Closed
Yaffle opened this issue Apr 9, 2012 · 26 comments
Closed

Date#toISOString/Native Date object bugs #111

Yaffle opened this issue Apr 9, 2012 · 26 comments

Comments

@Yaffle
Copy link
Contributor

Yaffle commented Apr 9, 2012

Safari 5.1.5

console.log(new Date(-1).toISOString()); // Safari 5.1.5 "1969-12-31T23:59:59.-01Z" - es5-shim not applyied, because of
// "if (!Date.prototype.toISOString || (new Date(-62198755200000).toISOString().indexOf('-000001') === -1)) {"

// console.log(new Date(-1).toISOString()); should be "1969-12-31T23:59:59.999Z"

Opera 11.61, Opera 12
(new Date(-3509827334573292)).getUTCMonth() === 12 // - 13 monthes in a year !

console.log(new Date(-3509827334573292).toISOString());
// Opera 11.61/Opera 12 (with es5-shim): -109253-13-01T10:37:06.708Z
// should be -109252-01-01T10:37:06.708Z

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 9, 2012

fix for Date#toISOString also added to #110

Date#getUTCMonth still buggy in Opera

@ghost
Copy link

ghost commented Apr 11, 2012

Hmm, in Opera 10.62 on OS X Lion, new Date(-3509827334573292).getUTCMonth() == 27. getMonth yields the same result.

@ghost
Copy link

ghost commented Apr 12, 2012

Okay, it appears that getUTCFullYear and getUTCDate are broken, too (tested in 10.62)...

var value = new Date(-109252, 0, 1, 3, 37, 6, 708);
value.getTime() == -3509827334573292; // Sanity check.

value.getUTCFullYear() == -109253;
value.getUTCMonth() == 27;
value.getUTCDate() == 31;
value.getUTCHours() == 10;

// All the following are correct.
value.getUTCMinutes() == 37;
value.getUTCSeconds() == 6;
value.getUTCMilliseconds() == 708;

Edit: getUTCHours is buggy, too.

@ghost
Copy link

ghost commented Apr 12, 2012

FWIW, here's my fix for getUTCMonth:

Date.prototype.getUTCMonth = function () {
  var year = this.getUTCFullYear(), day = Math.floor(+this / 8.64e7) - (365 * (year - 1970) + Math.floor((year - 1969) / 4) - Math.floor((year - 1901) / 100) + Math.floor((year - 1601) / 400));
  return (0 <= day && day < 31) ? 0 :
    // Subtract a day for leap years.
    (31 <= day && (day -= (!(year % 4) && year % 100 || !(year % 400)) ? 1 : 0) < 59) ? 1 :
    (59 <= day && day < 90) ? 2 :
    (90 <= day && day < 120) ? 3 :
    (120 <= day && day < 151) ? 4 :
    (151 <= day && day < 181) ? 5 :
    (181 <= day && day < 212) ? 6 :
    (212 <= day && day < 243) ? 7 :
    (243 <= day && day < 273) ? 8 :
    (273 <= day && day < 304) ? 9 :
    (304 <= day && day < 334) ? 10 :
    (334 <= day && day < 365) ? 11 : NaN;
};

Unfortunately, it's not very useful—it relies on getUTCFullYear, which, as I noted above, is also broken.

Re-implementing getUTCFullYear is much more difficult: according to the spec (see the YearFromTime operation), computing a year from a raw millisecond time value essentially involves looping over all possible millisecond date values, from -8.64e15 to 8.64e15, and returning the matching year:

function getDay(year) {
  return 365 * (year - 1970) + Math.floor((year - 1969) / 4) - Math.floor((year - 1901) / 100) + Math.floor((year - 1601) / 400);
}

function YearFromTime(value) {
  for (var index = 0; index <= 8.64e15; index += 1) {
    if (8.64e7 * getDay(index) >= value) {
      return index - 1;
    }
  }
}

That creates an infinite loop (I'm sure the correct year will be discovered eventually, but not in a reasonable amount of time, and not without locking up the browser).

I might be missing something, though...

@ghost
Copy link

ghost commented Apr 12, 2012

Also, this bug is not present in Opera 9.64 and earlier. I'm not sure about early versions of 10.x, but it is present in 10.53 onward. Sounds like a regression.

I don't currently have access to a Windows VM, so I was only able to test 9.64, 10.53, 10.63, 11.52, and 11.62 on OS X Lion (10.7.3) only.

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 12, 2012

but

  1. value.getUTCFullYear() == -109253; - this is correct (Date constuctor is not UTC, so result depends on your local timezone)

  2. in Opera 12 i can reproduce only bug with getUTCMonth() == 12
    So, i don't think we should replace all native methods (as Opera 12 is not buggy)

  3. YearFromTime could be implemented:

function YearFromTime(t) {
    t = Math.floor(t / 86400000);
    var year = Math.floor(t / 365.2425) + 1970 - 1;
    // this circle will iterate no more than 2 times
    while (getDay(year + 1, 0) <= t) {
      year += 1;
    }
    return year;
}

(see https://gist.github.com/2312309 )

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 12, 2012

var value = new Date(Date.UTC(-109252, 0, 1, 3, 37, 6, 708));
                                                     // correct     Opera 11.6x/Opera 12
console.log(value.getUTCFullYear());// -109252    -109253
console.log(value.getUTCMonth());   // 0               12
console.log(value.getUTCDate());     //  1               1
console.log(value.getUTCHours());   //    3             3

console.log(value.getUTCMinutes());// 37              37
console.log(value.getUTCSeconds());// 6             6
console.log(value.getUTCMilliseconds());//708    708

you are right, getUTCFullYear is also buggy in Opera 11.6x/Opera 12, but this
is easy to workaround, because getUTCMonth returns 12

@ghost
Copy link

ghost commented Apr 12, 2012

  1. value.getUTCFullYear() == -109253; - this is correct (Date constuctor is not UTC, so result depends on your local timezone)

Ah, I'd forgotten about that. I don't think UTC is the issue here, though...value.getUTCFullYear() is -109252 in Safari >= 2.0, Firefox >= 1.0, Chrome >= 1.0.154, and Opera <= 9.64...it's only -109253 in Opera >= 10.53.

Edit: Mid-air collision; just saw your latest comment.

  1. in Opera 12 i can reproduce only bug with getUTCMonth() == 12

Glad to hear that's been fixed. Opera 10.53 returns 16; 10.62 returns 27. I have no idea what to make of that.

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 12, 2012

#110

this lines was added to workaround getUTCMonth bug in Opera

+        // see https://github.com/kriskowal/es5-shim/issues/111
+        year += Math.floor(month / 12);
+       month = (month % 12 + 12) % 12;

@ghost
Copy link

ghost commented Apr 12, 2012

In Opera 10.53, with es5-shim loaded, new Date(-3509827334573292).toISOString() is "-109252-04-366T10:37:06.708Z". In 11.62 (again, with the shim loaded), the result is "-109251-04-31T10:37:06.708Z".

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 12, 2012

@kitcambridge can't reproduce, could you provide full source code for this test?
i have this with current es5-shim and Opera 11.62 (build 1347)
new Date(-3509827334573292).toISOString() === "-109253-13-01T10:37:06.708Z"

@ghost
Copy link

ghost commented Apr 12, 2012

new Date(-3509827334573292).toISOString() === "-109253-13-01T10:37:06.708Z"

Yep, that's what I'm using, too. But the result is definitely "-109251-04-31T10:37:06.708Z".

Opera 11.62 Test

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 12, 2012

0_O
may be result is OS-dependent ?

@ghost
Copy link

ghost commented Apr 12, 2012

Update: The above screenshot was made with your fork of es5-shim loaded (which includes #110). This is the result with the current shim:

Opera 11.62; Unpatched

Bizarre! I pinged @miketaylr about this via Twitter yesterday; hopefully, he'll be able to provide some answers at his convenience. I'll also test this on my Windows VM later today.

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 12, 2012

// -109253-28-31T10:37:06.708Z - Opera/9.80 (X11; Linux i686; U; de) Presto/2.10.229 Version/11.62

// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; ru) Presto/2.10.229 Version/11.62
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; de) Presto/2.10.229 Version/11.61
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.10.229 Version/11.60
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.9.168 Version/11.51
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.8.131 Version/11.11
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.8.131 Version/11.10
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.7.62 Version/11.01
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.7.62 Version/11.00

// -109253-20-32965478T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; de) Presto/2.6.30 Version/10.63
// -109253-20-32965478T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.6.30 Version/10.62
// -109253-20-32965478T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.6.30 Version/10.61
// -109253-20-32965478T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.6.30 Version/10.60

// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.5.24 Version/10.54
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.5.24 Version/10.53
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.5.24 Version/10.52
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.5.22 Version/10.51
// -109253-13-01T10:37:06.708Z - Opera/9.80 (Windows NT 5.1; U; en) Presto/2.5.22 Version/10.50

@miketaylr
Copy link

Whoa, impressive collection of Operas there.

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 13, 2012

what should be fixed for es5-shim ?

@miketaylr
Copy link

Still trying to wrap my head around this issue--there's a lot of numbers. :) Can someone summarize it in 2 lines?

@ghost
Copy link

ghost commented Apr 13, 2012

@miketaylr I'll try :bowtie:: Given A = new Date(-3509827334573292), A.getUTCFullYear(), A.getUTCMonth(), and A.getUTCDate() return incorrect and/or nonsensical results. This bug is not present in Opera <= 9.64.

Edit: @Yaffle's table above also shows that the nonsensical results are not consistent across versions 10.x and 11.x.

@miketaylr
Copy link

thx @kitcambridge. I just checked in one of the most recent internal builds:

ci319 vs chrome

Assuming that Chrome is correct here, this suggests that getUTCFullYear, getUTCMonth and getUTCDate are still buggy, right?

@ghost
Copy link

ghost commented Apr 13, 2012

@miketaylr Yep! Chrome, Firefox, and Safari all return identical results.

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 13, 2012

http://yaffle.github.com/date-shim/
https://github.com/Yaffle/date-shim
according to spec, Date.parse(dt.toString());
Date.parse(dt.toUTCString());
should be equal to dt.getTime(), if milliseconds === 0 & dt is valid date,
but 1) GMT(UTC) format not supports extended years(4 digit years only);
2) some browsers not support this format for small years (Chrome)
Date.parse(new Date(-62135596800000).toUTCString()) // Mon, 01 Jan 0001 00:00:00 GMT

@miketaylr
Copy link

Filed DSK-361368. Thanks guys!

@Yaffle
Copy link
Contributor Author

Yaffle commented Apr 15, 2012

what about this fix?

(this code tries to get equivalent year in range [1970; 1970+400), than applies native method)

// getXXX fix for Opera 10-12

function fixDateMethod(nativeMethod, y) {
  return function () {
    if (Object.prototype.toString.call(this) === '[object Date]') {
      var tmp = Number(this),
          year = y ? Math.floor(tmp / 12622780800000) * 400 : 0;
      tmp = new Date((tmp % 12622780800000 + 12622780800000) % 12622780800000);// tmp is equivalent year to original
      return year + nativeMethod.call(tmp);
    }
    return nativeMethod.call(this);
  };
}

Date.prototype.getUTCFullYear = fixDateMethod(Date.prototype.getUTCFullYear, true);
Date.prototype.getFullYear = fixDateMethod(Date.prototype.getFullYear, true);
Date.prototype.getUTCMonth = fixDateMethod(Date.prototype.getUTCMonth, false);
Date.prototype.getMonth = fixDateMethod(Date.prototype.getMonth, false);
Date.prototype.getUTCDate = fixDateMethod(Date.prototype.getUTCDate, false);
Date.prototype.getDate = fixDateMethod(Date.prototype.getDate, false);

@Yaffle
Copy link
Contributor Author

Yaffle commented Aug 3, 2012

Opera bug with getUTCMonth() == 12 is fixed in Opera 12.50

@Yaffle
Copy link
Contributor Author

Yaffle commented May 11, 2013

the bug is not reproducible since Opera 12.1x

@Yaffle Yaffle closed this as completed May 11, 2013
ljharb pushed a commit to ljharb/es5-shim that referenced this issue Dec 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants