Closed Bug 411968 Opened 12 years ago Closed 12 years ago

Finishing JS_AlreadyHasOwnProperty

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: jorendorff, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The new JS_AlreadyHasOwnProperty API (bug 400793) isn't finished: it isn't documented.  I'm working on that, and I have a few questions.

* Why not call it JS_HasOwnProperty?  (I think we should.  We can still rename it; it hasn't been in a release yet.)

* The implementation is quite different from the implementation of obj_hasOwnProperty (in jsobj.c).  Can some code be shared here?

* Object.prototype.hasOwnProperty() handles inner/outer objects specially.  When searching for a property on an object x, if the property is found on x's outer object, the JSAPI says false; the JS method says true.  Is that intentional?

* Object.prototype.hasOwnProperty() also handles JSPROP_SHARED, JSPROP_PERMANENT properties specially.   JS_AlreadyHasOwnProperty doesn't, so it exposes an implementation detail about Function.length/arity.  Is that intentional?
* It doesn't CHECK_REQUEST(cx).  Should it?
(In reply to comment #0)
> * Why not call it JS_HasOwnProperty?  (I think we should.  We can still rename
> it; it hasn't been in a release yet.)

Because ECMA specifies hasOwnProperty on Object.prototype, and that does something different from what the underpinning of this API does. We may want to expose that standard method as JS_HasOwnProperty. This API does not resolve lazily-created properties -- hence the "Already".

> * The implementation is quite different from the implementation of
> obj_hasOwnProperty (in jsobj.c).  Can some code be shared here?

No, see above (and see the other bug, I thought we discussed all this there, but it might have been more on IRC; IRC is a problem :-/).

> * Object.prototype.hasOwnProperty() handles inner/outer objects specially. 
> When searching for a property on an object x, if the property is found on x's
> outer object, the JSAPI says false; the JS method says true.  Is that
> intentional?

I think so. Again, this is *not* ECMA hasOwnProperty, reflected as a JS API.

> * Object.prototype.hasOwnProperty() also handles JSPROP_SHARED,
> JSPROP_PERMANENT properties specially.   JS_AlreadyHasOwnProperty doesn't, so
> it exposes an implementation detail about Function.length/arity.  Is that
> intentional?

Yes, see above.

I'm pretty sure you are barking up the wrong tree here, except for the doc writing goodness -- for which, thanks! :-).

/be
Cc'ing bz, since he is sole user of this new API. Jason, you are right we can change this still, but since it can't do what Object.prototype.hasOwnProperty, we may end up doc'ing it as is. But don't let me stop you from proposing better API impl and/or name, taking into account the intentionality of the original change.

/be
Thanks, Brendan.

I think this API should CHECK_REQUEST(cx), because it calls JS_LOCK_OBJ.  Patch attached.  I searched mxr for uses and only found <http://mxr.mozilla.org/seamonkey/source/content/xbl/src/nsXBLProtoImpl.cpp#258>, which is only called from within auto-requests.

Also please see http://developer.mozilla.org/en/docs/JS_AlreadyHasOwnProperty .
Assignee: general → jorendorff
Status: NEW → ASSIGNED
Attachment #297063 - Flags: review?(brendan)
Attachment #297063 - Flags: review?(brendan)
Attachment #297063 - Flags: review+
Attachment #297063 - Flags: approval1.9+
Keywords: checkin-needed
Checking in js/src/jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.385; previous revision: 3.384
done

Not sure if this bug is fixed completely, so leaving open.
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9 M11
Blocks: 412368
Thanks, Reed.  It's fixed.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.