Closed Bug 1381433 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript: Standard Library, enhancement)

Product:
Core
Component:
JavaScript: Standard Library
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(2 files, 1 obsolete file)

bug1381433.patch
4.53 KB, patch
till
: review+
Details | Diff | Splinter Review
bug1381433-part2-update-devtools-test.patch
3.14 KB, patch
bgrins
: review+
Details | Diff | Splinter Review
Per https://github.com/tc39/ecma262/commit/0264be5491c2b36d592bd7a29e33a72259e5c5a8, Date.prototype.toString now throws when called with a non-Date object.
Attached patch bug1381433.patch — — Splinter 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: 8 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: