Closed
Bug 793210
Opened 12 years ago
Closed 10 years ago
getOwnPropertyDescriptor proxy trap reverses the logic for new properties on non-extensible objects
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: simon.lindholm10, Assigned: efaust)
References
Details
(Whiteboard: [js:p2][firebug-p1])
Attachments
(1 file)
719 bytes,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18.0 Firefox/18.0 Build ID: 20120921030601 Steps to reproduce: 1. var target = {}; Object.getOwnPropertyDescriptor(new Proxy(target, { getOwnPropertyDescriptor: function () { return {value: 2, configurable: true}; } }), 'foo'); 2. var target = {}; Object.preventExtensions(target); Object.getOwnPropertyDescriptor(new Proxy(target, { getOwnPropertyDescriptor: function () { return {value: 2, configurable: true}; } }), 'foo'); Actual results: The first thing throws "TypeError: proxy can't report a new property on a non-extensible object", but shouldn't. The second thing should throw, but doesn't. TrapGetOwnProperty in jsproxy.cpp does: // step 9 if (target->isExtensible() && !isFixed) { JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_REPORT_NEW); return false; } but per spec the condition should be (!target->isExtensible() && !isFixed)
Updated•12 years ago
|
Assignee: general → ejpbruel
QA Contact: ejpbruel
Updated•12 years ago
|
Whiteboard: [js:p2]
Reporter | ||
Comment 1•12 years ago
|
||
The patch for bug 795903 should also fix this.
Updated•11 years ago
|
Whiteboard: [js:p2] → [js:p2][firebug-p1]
Reporter | ||
Comment 2•11 years ago
|
||
Any update on this (or bug 795903)? It breaking proxies pretty badly, and should be a simple fix.
Updated•10 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 3•10 years ago
|
||
Wtf. Here's a fix.
Assignee: ejpbruel → efaustbmo
Status: NEW → ASSIGNED
Attachment #8388745 -
Flags: review?(jorendorff)
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Comment 4•10 years ago
|
||
Comment on attachment 8388745 [details] [diff] [review] Fix Review of attachment 8388745 [details] [diff] [review]: ----------------------------------------------------------------- Add the test case to one of the test suites. r=me with that. Of course proxies still need work, but thanks for knocking this one out.
Attachment #8388745 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/79d8037bf2ed
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/79d8037bf2ed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•