Closed Bug 1381433 Opened 3 years ago Closed 3 years ago

Throw TypeError when Date.prototype.toString is called with non-Date object

Categories

(Core :: JavaScript: Standard Library, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(2 files, 1 obsolete file)

Per https://github.com/tc39/ecma262/commit/0264be5491c2b36d592bd7a29e33a72259e5c5a8, Date.prototype.toString now throws when called with a non-Date object.
Attached patch bug1381433.patchSplinter Review
Simple update to throw a TypeError when Date.prototype.toString is called with a non-Date object. TC39 considers this change to be web-compatible, because V8 and JSC already have shipped versions which conform to the new behaviour.
Attachment #8886998 - Flags: review?(till)
Comment on attachment 8886998 [details] [diff] [review]
bug1381433.patch

Review of attachment 8886998 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thank you
Attachment #8886998 - Flags: review?(till) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07dd7b5ed0e8
Throw when Date.prototype.toString is called with non-Date object per ES2018 spec draft. r=till
Keywords: checkin-needed
Backed out for failing devtools/client/webconsole/test/browser_webconsole_output_05.js:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac2934d4263eeb0df3729f701a5ace9a98e3135d
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e4e0f3f80b80249fb841a00d3dc8226dadac09d

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=477268fa767f88f50fc99380a1bf3b303bb1b53d&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=115270109&repo=mozilla-inbound

[task 2017-07-18T15:17:57.154561Z] 15:17:57     INFO - TEST-PASS | devtools/client/webconsole/test/browser_webconsole_output_05.js | matched rule: console.log() output: /Object \{.*\}/ - 
[task 2017-07-18T15:17:57.155357Z] 15:17:57     INFO - Printing
[task 2017-07-18T15:17:57.156027Z] 15:17:57     INFO - Waiting for messages...
[task 2017-07-18T15:17:57.156712Z] 15:17:57     INFO - Buffered messages logged at 15:17:11
[task 2017-07-18T15:17:57.157415Z] 15:17:57     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
[task 2017-07-18T15:17:57.158287Z] 15:17:57     INFO - Buffered messages finished
[task 2017-07-18T15:17:57.159023Z] 15:17:57     INFO - TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_output_05.js | Test timed out - 
[task 2017-07-18T15:17:57.160539Z] 15:17:57     INFO - Removing tab.
[task 2017-07-18T15:17:57.161436Z] 15:17:57     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2017-07-18T15:17:57.164422Z] 15:17:57     INFO - Got event: 'TabClose' on [object XULElement].
[task 2017-07-18T15:17:57.165779Z] 15:17:57     INFO - Tab removed and finished closing
[task 2017-07-18T15:17:57.167259Z] 15:17:57     INFO - GECKO(1624) | console.log: dumpConsoles start
[task 2017-07-18T15:17:57.168553Z] 15:17:57     INFO - GECKO(1624) | console.log: dumpConsoles end
[task 2017-07-18T15:17:57.169807Z] 15:17:57     INFO - Not taking screenshot here: see the one that was previously logged
[task 2017-07-18T15:17:57.171035Z] 15:17:57     INFO - TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_output_05.js | failed to match rule: print() output: Invalid Date - 
[task 2017-07-18T15:17:57.172153Z] 15:17:57     INFO - Stack trace:
[task 2017-07-18T15:17:57.173281Z] 15:17:57     INFO -     chrome://mochitests/content/browser/devtools/client/webconsole/test/head.js:testCleanup:1311
[task 2017-07-18T15:17:57.174366Z] 15:17:57     INFO -     chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:390
[task 2017-07-18T15:17:57.175386Z] 15:17:57     INFO -     timeoutFn@chrome://mochikit/content/browser-test.js:858:9
[task 2017-07-18T15:17:57.176307Z] 15:17:57     INFO -     setTimeout handler*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:676:12
[task 2017-07-18T15:17:57.177279Z] 15:17:57     INFO -     timeoutFn@chrome://mochikit/content/browser-test.js:836:13
[task 2017-07-18T15:17:57.177938Z] 15:17:57     INFO -     setTimeout handler*Tester_execTest@chrome://mochikit/content/browser-test.js:820:9
[task 2017-07-18T15:17:57.178649Z] 15:17:57     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:670:7
[task 2017-07-18T15:17:57.179477Z] 15:17:57     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
Flags: needinfo?(andrebargull)
With the new semantics, |print(Date.prototype)| throws a TypeError, which leads to an issue when executing the |checkPrintOutput| test. As a simple workaround, I've added a |noPrint| option to skip the |checkPrintOutput| test, but maybe there's a better alternative. WDYT?
Flags: needinfo?(andrebargull)
Attachment #8901756 - Flags: review?(nfitzgerald)
Comment on attachment 8901756 [details] [diff] [review]
bug1381433-part2-update-devtools-test.patch

Review of attachment 8901756 [details] [diff] [review]:
-----------------------------------------------------------------

This is a better question for bgrins
Attachment #8901756 - Flags: review?(nfitzgerald) → review?(bgrinstead)
Comment on attachment 8901756 [details] [diff] [review]
bug1381433-part2-update-devtools-test.patch

Review of attachment 8901756 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/webconsole/test/browser_webconsole_output_05.js
@@ +74,2 @@
>  
>    // 7

I would just remove this check entirely and renumber the comments below appropriately. There's no need to have a webconsole test making sure Date.prototype.toString() throws - the `print` command returns exception messages in any case like this.
Attachment #8901756 - Flags: review?(bgrinstead)
New patch to simply remove the |Date.prototype| test per comment #9.
Attachment #8901756 - Attachment is obsolete: true
Attachment #8901948 - Flags: review?(bgrinstead)
Attachment #8901948 - Flags: review?(bgrinstead) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d23df6e54a90
Throw when Date.prototype.toString is called with non-Date object per ES2018 spec draft. r=till, r=bgrins
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d23df6e54a90
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.