Closed
Bug 1142279
Opened 9 years ago
Closed 9 years ago
DataView should require `new`
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: jgilbert, Assigned: jgilbert)
References
()
Details
(Keywords: dev-doc-complete, site-compat, Whiteboard: [DocArea=JS])
Attachments
(3 files)
5.06 KB,
patch
|
Details | Diff | Splinter Review | |
3.54 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
1.22 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
The test requires that `DataView()` throw an exception: var expr = "DataView(new ArrayBuffer)"; // Use try/catch instead of calling shouldThrow to avoid different exception message being reported from different platform. try { TestEval(expr); testFailed(expr + " does not throw exception"); } catch (e) { testPassed(expr + " threw exception"); } I'm told this is an easy fix with a longish tail of tests to fixup, since this non-spec syntax has infected our tests. If I could get a JS person to write the engine fix, I am willing to deal with the test fixups.
Comment 1•9 years ago
|
||
Updated•9 years ago
|
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Assignee | ||
Comment 2•9 years ago
|
||
Assignee: nobody → jgilbert
Attachment #8602296 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8602298 -
Flags: review?(efaustbmo)
Assignee | ||
Comment 4•9 years ago
|
||
Try looks good?: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8781fe15317
Comment 5•9 years ago
|
||
Comment on attachment 8602296 [details] [diff] [review] 0001-Forbid-non-new-constructors.-from-waldo.patch Review of attachment 8602296 [details] [diff] [review]: ----------------------------------------------------------------- ThorwIfNotConstructing makes a whole lot more sense than ReportIsNotFunction... r=me ::: js/src/vm/NativeObject-inl.h @@ +612,5 @@ > JSMSG_BUILTIN_CTOR_NO_NEW, builtinName); > } > > +inline bool > +ThrowIfNotConstructing(JSContext *cx, const CallArgs &args, const char *builtinName) Up till now, this has just been written out inline. Can we file a followup to do the trudging task of replacing the other places where this would be useful? The Proxy and TypedObject constructors come to mind, offhand. also this normally uses JSMSG_NOT_FUNCTION, which I always thought sucked, but there we are. We should at least endeavor to have a unified approach here.
Attachment #8602296 -
Flags: review?(efaustbmo) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8602298 [details] [diff] [review] 0002-Fix-JS-tests.patch Review of attachment 8602298 [details] [diff] [review]: ----------------------------------------------------------------- APPROVED.
Attachment #8602298 -
Flags: review?(efaustbmo) → review+
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Eric Faust [:efaust] from comment #5) > Comment on attachment 8602296 [details] [diff] [review] > 0001-Forbid-non-new-constructors.-from-waldo.patch > > Review of attachment 8602296 [details] [diff] [review]: > ----------------------------------------------------------------- > > ThorwIfNotConstructing makes a whole lot more sense than > ReportIsNotFunction... r=me > > ::: js/src/vm/NativeObject-inl.h > @@ +612,5 @@ > > JSMSG_BUILTIN_CTOR_NO_NEW, builtinName); > > } > > > > +inline bool > > +ThrowIfNotConstructing(JSContext *cx, const CallArgs &args, const char *builtinName) > > Up till now, this has just been written out inline. Can we file a followup > to do the trudging task of replacing the other places where this would be > useful? The Proxy and TypedObject constructors come to mind, offhand. also > this normally uses JSMSG_NOT_FUNCTION, which I always thought sucked, but > there we are. We should at least endeavor to have a unified approach here. Can you file a follow-up bug for this?
Flags: needinfo?(efaustbmo)
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ded1da5b0d7a
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•9 years ago
|
Keywords: site-compat
Comment 10•9 years ago
|
||
Release notes: https://developer.mozilla.org/en-US/Firefox/Releases/40#JavaScript Site compat: https://developer.mozilla.org/en-US/Firefox/Releases/40/Site_Compatibility#TypeError_when_DataView_called_without_new Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView#Browser_compatibility
Keywords: dev-doc-needed → dev-doc-complete
Comment 11•9 years ago
|
||
(In reply to Jeff Gilbert [:jgilbert] from comment #7) > (In reply to Eric Faust [:efaust] from comment #5) > > Comment on attachment 8602296 [details] [diff] [review] > > 0001-Forbid-non-new-constructors.-from-waldo.patch > > > > Review of attachment 8602296 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ThorwIfNotConstructing makes a whole lot more sense than > > ReportIsNotFunction... r=me > > > > ::: js/src/vm/NativeObject-inl.h > > @@ +612,5 @@ > > > JSMSG_BUILTIN_CTOR_NO_NEW, builtinName); > > > } > > > > > > +inline bool > > > +ThrowIfNotConstructing(JSContext *cx, const CallArgs &args, const char *builtinName) > > > > Up till now, this has just been written out inline. Can we file a followup > > to do the trudging task of replacing the other places where this would be > > useful? The Proxy and TypedObject constructors come to mind, offhand. also > > this normally uses JSMSG_NOT_FUNCTION, which I always thought sucked, but > > there we are. We should at least endeavor to have a unified approach here. > > Can you file a follow-up bug for this? evilpie just did this in bug 1215814 :)
Flags: needinfo?(efaustbmo)
You need to log in
before you can comment on or make changes to this bug.
Description
•