Last Comment Bug 505587 - Implement ES5 Object.getOwnPropertyDescriptor
: Implement ES5 Object.getOwnPropertyDescriptor
Status: RESOLVED FIXED
fixed-in-tracemonkey
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 516351
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-21 16:30 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2010-08-18 10:27 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (10.61 KB, patch)
2009-08-28 17:43 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
mrbkap: review+
Details | Diff | Splinter Review
A decent set of tests for a start (6.13 KB, patch)
2009-09-04 12:27 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2009-07-21 16:30:16 PDT
Needed for verifying correctness of Object.defineProperty...
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2009-08-28 17:43:26 PDT
Created attachment 397398 [details] [diff] [review]
Patch

Still needs tests, will start formally writing them shortly...
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-04 12:27:55 PDT
Created attachment 398735 [details] [diff] [review]
A decent set of tests for a start

I also ran the patch against the ES5 tests at <http://es5conform.codeplex.com/>, and things seem to work perfectly fine.

This is currently limited to what's exposed by ES5 itself; bug 430133 and Object.defineProperty will allow access to the real corner cases that can't be examined in the current specified APIs.
Comment 3 Blake Kaplan (:mrbkap) 2009-09-09 15:05:23 PDT
Comment on attachment 397398 [details] [diff] [review]
Patch

>+static JSBool
>+obj_getOwnPropertyDescriptor(JSContext *cx, uintN argc, jsval *vp)
>+{
>+    if (argc == 0 || JSVAL_IS_PRIMITIVE(vp[2])) {
>+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
>+                             JSMSG_NOT_NONNULL_OBJECT,
>+                             js_getter_str);
>+        return JS_FALSE;
>+    }
>+    JSObject *obj = JSVAL_TO_OBJECT(vp[2]);

Nit: blank line after the }.

>+        JSScopeProperty *sprop = reinterpret_cast<JSScopeProperty *>(prop);
>+        if (!desc->defineProperty(cx, ATOM_TO_JSID(atomState.getAtom),
>+                                  ((attrs & JSPROP_GETTER) && OBJ_IS_NATIVE(obj))
>+                                  ? js_CastAsObjectJSVal(sprop->getter)
>+                                  : JSVAL_VOID,
>+                                  JS_PropertyStub, JS_PropertyStub,
>+                                  JSPROP_ENUMERATE, NULL) ||
>+            !desc->defineProperty(cx, ATOM_TO_JSID(atomState.setAtom),
>+                                  ((attrs & JSPROP_SETTER) && OBJ_IS_NATIVE(obj))
>+                                  ? js_CastAsObjectJSVal(sprop->setter)
>+                                  : JSVAL_VOID,
>+                                  JS_PropertyStub, JS_PropertyStub,
>+                                  JSPROP_ENUMERATE, NULL)) {

I'd prefer to move the OBJ_IS_NATIVE checks (and the cast to JSScopeProperty) into a common place.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-09 16:40:31 PDT
http://hg.mozilla.org/tracemonkey/rev/b7ee9263cf96

with the requested changes, also with the attached tests in the new ecma_5/Object/ directory.
Comment 5 Blake Kaplan (:mrbkap) 2009-09-16 17:32:15 PDT
http://hg.mozilla.org/mozilla-central/rev/b7ee9263cf96
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-28 13:12:58 PDT
http://hg.mozilla.org/tracemonkey/rev/87fcc6cb16af to update tests to use reportCompare once
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2010-01-11 11:36:11 PST
I wrote up a little bit of documentation, but it needs to be made more visible than simply exposing it deep within the JS reference:

https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Global_Objects/Object/getOwnPropertyDescriptor

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