Closed Bug 1381433 Opened 3 years ago Closed 3 years ago

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


(Core :: JavaScript: Standard Library, enhancement)

Not set



Tracking Status
firefox57 --- fixed


(Reporter: anba, Assigned: anba)



(2 files, 1 obsolete file)

Per, 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]

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

Nice, thank you
Attachment #8886998 - Flags: review?(till) → review+
Pushed by
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:

Push with failure:
Failure log:

[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]

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]

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
Throw when Date.prototype.toString is called with non-Date object per ES2018 spec draft. r=till, r=bgrins
Keywords: checkin-needed
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.