As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 656320 - JSClass.delProperty should allow properties to not be deleted
: JSClass.delProperty should allow properties to not be deleted
Status: RESOLVED FIXED
[mentor=jorendorff][spidernode][inbound]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Rob Arnold [:robarnold]
:
: Jason Orendorff [:jorendorff]
Mentors:
https://developer.mozilla.org/En/Spid...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-11 08:59 PDT by Rob Arnold [:robarnold]
Modified: 2011-07-27 03:28 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Naive implementation (499 bytes, patch)
2011-05-11 20:42 PDT, Rob Arnold [:robarnold]
jorendorff: review+
Details | Diff | Splinter Review

Description User image Rob Arnold [:robarnold] 2011-05-11 08:59:30 PDT
Per-docs, this should be allowed but the code doesn't check the result of the property handler. Currently this can only be done via throwing an exception from within the delProperty handler.
Comment 1 User image Rob Arnold [:robarnold] 2011-05-11 20:42:27 PDT
Created attachment 531839 [details] [diff] [review]
Naive implementation

A quick scan via mxr shows only two custom delete property hooks and neither sets *vp.
Comment 2 User image Jason Orendorff [:jorendorff] 2011-06-20 10:06:04 PDT
Comment on attachment 531839 [details] [diff] [review]
Naive implementation

I thought I plus'd this a long time ago.

I note that this leaves a delProperty call in Object.defineProperty (DefinePropertyOnObject, specifically) unchanged. I think that's OK. We're really after V8 API parity here.
Comment 3 User image Jason Orendorff [:jorendorff] 2011-07-20 19:43:41 PDT
Rob, did we not need this after all? It hasn't landed. Checkin-needed?
Comment 4 User image Rob Arnold [:robarnold] 2011-07-20 19:51:29 PDT
checkin-needed - I haven't found the time to pull a t-m tree and push (or actually figure out where it should land now).
Comment 5 User image David Mandelin [:dmandelin] 2011-07-26 11:42:09 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/cdad0b82ec90
Comment 6 User image Marco Bonardo [::mak] 2011-07-27 03:28:27 PDT
http://hg.mozilla.org/mozilla-central/rev/cdad0b82ec90

Note You need to log in before you can comment on or make changes to this bug.