So say most of the tests from http://samples.msdn.microsoft.com/ietestcenter/Javascript/ES15.9.html that fail.
Created attachment 527107 [details] [diff] [review] added err def and tests nyan nyan
Comment on attachment 527107 [details] [diff] [review] added err def and tests Looks mostly reasonable, but there are enough nits that I'd like to get another patch for this, rather than make the necessary pre-landing tweaks myself. >diff --git a/js/src/js.msg b/js/src/js.msg >+MSG_DEF(JSMSG_INVALID_DATE, 270, 0, JSEXN_RANGEERR, "invalid date") >\ No newline at end of file Add one. >diff --git a/js/src/jsdate.cpp b/js/src/jsdate.cpp > if (!JSDOUBLE_IS_FINITE(utctime)) >- JS_snprintf(buf, sizeof buf, js_NaN_date_str); >+ if (printFunc == print_iso_string) { >+ JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_INVALID_DATE); >+ return false; >+ } >+ else >+ JS_snprintf(buf, sizeof buf, js_NaN_date_str); > else > (*printFunc)(buf, sizeof buf, utctime); You should put braces around the else line here. This is pretty ugh the way this handling is all done, but whatever. :-\ >diff --git a/js/src/tests/ecma_5/Date/jstests.list b/js/src/tests/ecma_5/Date/jstests.list >+script toISOString.js >\ No newline at end of file Add one. >diff --git a/js/src/tests/ecma_5/Date/toISOString.js b/js/src/tests/ecma_5/Date/toISOString.js >+function throwsRangeError(a) { >+ try { >+ var date = new Date(a); >+ date.toISOString(); >+ } catch (err) { >+ assertEq(err instanceof RangeError, true, 'wrong error class'); >+ return; >+ } >+ assertEq(0, 1, 'not good, nyan, nyan'); >+} >+ >+throwsRangeError(Infinity); >+throwsRangeError(-Infinity); >+throwsRangeError('99/70/1'); >+throwsRangeError('100/70/2'); This relies on unspecified behavior of |new Date(v)| when ToString(v) doesn't match the YYYY-MM-DDTHH:mm:ss.sssZ format. Instead use |new Date()| followed by |date.setTime(a)| for fully-specified behavior. With that you should get rid of the latter two tests, and I think those only produce NaN-backed dates, so it should be fine to replace them with |throwsRangeError(NaN)|. >+if (typeof reportCompare === "function") >+ reportCompare(true, true); >\ No newline at end of file Add one.
Created attachment 527678 [details] [diff] [review] v2 >I'd like to get another patch for this, rather than make the necessary >pre-landing tweaks myself. Nobody expects that. :) >This is pretty ugh the way this handling is all done, but whatever. :-\ Didn't like it either, i am not totally guilty, Jason proposed that, too. ;(
Comment on attachment 527678 [details] [diff] [review] v2 Looks good except for it still doing |var date = new Date(a)| rather than |var date = new Date(); date.setTime(a)|, but I don't mind fixing that on push. Thanks!
Yeah, that happens when writing patches at 2 am :(
http://hg.mozilla.org/tracemonkey/rev/f9d134c5f744 Turns out there was more than just that to fix, ecma_5/Date/15.9.4.2.js relied on toISOString returning "Invalid Date" for out-of-range dates. :-( I fixed up all the stuff in that test that relied on that and pushed with those changes. Also, a much less substantive style issue, I hadn't noticed the original patch had an if (...) return; else ..., which I converted to if (...) return; ... to avoid else-after-return.
http://hg.mozilla.org/mozilla-central/rev/f9d134c5f744
Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0 Run tests from Comment 0 on Ubuntu 11.04 x86, WinXP, Win 7 x86, Mac OS X 10.6. Total tests: 22 Passed: 22 Failed: 0 Could not load: 0 Setting status to Verified Fixed.
or
