All users were logged out of Bugzilla on October 13th, 2018

Change JSAddPropertyOp signature

RESOLVED FIXED in Firefox 40

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

unspecified
mozilla40
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
I want to split setProperty and addProperty, so might as well fix addProperty first.
(Assignee)

Updated

4 years ago
Assignee: nobody → evilpies
(Assignee)

Comment 1

4 years ago
Created attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp
Attachment #8582571 - Flags: review?(peterv)
Attachment #8582571 - Flags: review?(jorendorff)
(Assignee)

Updated

4 years ago
Attachment #8582571 - Attachment description: Remove mutable value parameter from JSPropertyOP → Remove mutable value parameter from JSAddPropertyOp
Comment on attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp

Review of attachment 8582571 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/idl/nsIXPCScriptable.idl
@@ +78,5 @@
>                       in JSObjectPtr globalObj, out JSObjectPtr parentObj);
>  
>      boolean addProperty(in nsIXPConnectWrappedNative wrapper,
>                         in JSContextPtr cx, in JSObjectPtr obj, in jsid id,
> +                       in jsval val);

Need to change the IID of this interface (mach update-uuids).
Attachment #8582571 - Flags: review?(peterv) → review+
Comment on attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp

Review of attachment 8582571 [details] [diff] [review]:
-----------------------------------------------------------------

Nice. Scripted WANT_ADDPROPERTY users are a wildcard; maybe Peter knows more about them? Anyway, it's worth a shot.

::: dom/base/nsDOMClassInfo.cpp
@@ +911,5 @@
>  }
>  
>  NS_IMETHODIMP
>  nsDOMClassInfo::AddProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
> +                            JSObject *obj, jsid id, JS::Handle<JS::Value> val,

JS::HandleValue is in the public API and is used everywhere, right?

::: js/xpconnect/idl/nsIXPCScriptable.idl
@@ +78,5 @@
>                       in JSObjectPtr globalObj, out JSObjectPtr parentObj);
>  
>      boolean addProperty(in nsIXPConnectWrappedNative wrapper,
>                         in JSContextPtr cx, in JSObjectPtr obj, in jsid id,
> +                       in jsval val);

When you change an IDL file, you have to change this interface's UUID and the UUIDs of all other interfaces that mention it. 'mach update-uuids' can do this for you, I think.
Attachment #8582571 - Flags: review?(peterv)
Attachment #8582571 - Flags: review?(jorendorff)
Attachment #8582571 - Flags: review+
Comment on attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp

Restoring peterv's review which I accidentally clobbered.
Attachment #8582571 - Flags: review?(peterv) → review+
Sorry, sucks to be you. Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/055c19334b2e to get a clean backout of another patch to finally get a clean backout of bustage from Friday afternoon.
https://hg.mozilla.org/mozilla-central/rev/c89c30a9b45f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox40: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.