Closed
Bug 1132522
Opened 9 years ago
Closed 9 years ago
Treat false return value from certain Proxy handler methods as failure
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(2 files)
13.21 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
11.46 KB,
patch
|
efaust
:
review+
|
Details | Diff | Splinter Review |
Split off from bug 1113369 which introduces these boolean "strict mode failure" return values everywhere. Implementing this for scripted proxies turns out to break a lot of code, so it may be a gut check landing it.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8564256 -
Flags: review?(efaustbmo)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8564257 -
Flags: review?(efaustbmo)
Updated•9 years ago
|
Attachment #8564256 -
Flags: review?(efaustbmo) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8564257 [details] [diff] [review] part 2 - Treat false return from proxyHandler.set() as strict mode failure Review of attachment 8564257 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: js/src/tests/ecma_6/TypedArray/from_proxy.js @@ +17,5 @@ > log.push("target", target); > var h = { > defineProperty: function (t, id) { > log.push("define", id); > + return true; shouldn't this be in the other patch?
Attachment #8564257 -
Flags: review?(efaustbmo) → review+
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Eric Faust [:efaust] from comment #3) > part 2 - Treat false return from proxyHandler.set() as strict mode failure > > defineProperty: function (t, id) { > > log.push("define", id); > > + return true; > > shouldn't this be in the other patch? Yeah, but it's never called, so it doesn't matter. :)
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/95fa879e6855 https://hg.mozilla.org/integration/mozilla-inbound/rev/0f393119201c
Comment 6•9 years ago
|
||
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=306ea1a7975d for Jetpack Test failures like : https://treeherder.mozilla.org/logviewer.html#?job_id=7371703&repo=mozilla-inbound
Comment 7•9 years ago
|
||
Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/46de54aebe951b0c8335b0f7c5894fc5baf1b669 Update proxy handler for ES6 See [bug 1132522](https://bugzilla.mozilla.org/show_bug.cgi?id=1132522). We definitely need this particular change. I'm about 95% sure this is the only one, since we won't be changing the behavior of legacy Proxy.create() proxies. https://github.com/mozilla/addon-sdk/commit/4694b75478a49edc9362f25fcfa2dac07c44ec84 Merge pull request #1888 from jorendorff/patch-1 Bug 1132522 - Update proxy handlers in the Addon SDK for ES6 error handling. r=Mossop
Updated•9 years ago
|
Assignee | ||
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2a7b760ad5c https://hg.mozilla.org/integration/mozilla-inbound/rev/911612d952e6
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f2a7b760ad5c https://hg.mozilla.org/mozilla-central/rev/911612d952e6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 10•9 years ago
|
||
https://developer.mozilla.org/en-US/Firefox/Releases/39#JavaScript https://developer.mozilla.org/en-US/Firefox/Releases/39/Site_Compatibility#Proxy_handers_may_throw_under_specific_conditions https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/handler/defineProperty https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/handler/set
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•