420585 - Defining setters for properties of the global object doesn't work

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P2)

Description

[Adam Kowalczyk]

Attachment: testcase

The following works as expected in the shell:

< var y;
< __defineSetter__('x', function(val) { y = val });
< x = 3;
< y
> 3

However, it fails in an apparently identical testcase, the setter never gets called.

Regressed between 2008-03-01 00:16 and 2008-03-01 03:10:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1204359360&maxdate=1204369859

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

This is *so* mine.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

We need to propagate the getters and setters onto the inner object. There is now some code duplication between XPCWrapper::AddProperty and XPCWrapper::NewResolve, which I can fix if you think it's worth it.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Brendan: blocking1.9 with a target milestone of beta4 means that this blocks shipment of beta4. If you don't think that's necessary, please change the TM to "mozilla1.9".

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Oops, that's my fault.

Comment 5

[Adam Kowalczyk]

Comment on attachment 306951 [details] [diff] [review]
Fix

This bug is blocking1.9+, approval is not needed.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 7

[Brian Crowder]

The if (!isXOW) sequence in XPCWrapper::AddProperty() seems to allow a situation where |v| is used in OBJ_DEFINE_PROPERTY uninitialized.  I am hitting this case while using firebug 1.2 with a recent nightly build.

Comment 8

[Brendan Eich [:brendan]]

(In reply to comment #7)
> The if (!isXOW) sequence in XPCWrapper::AddProperty() seems to allow a
> situation where |v| is used in OBJ_DEFINE_PROPERTY uninitialized.

Obvious bug, sorry I missed it -- please file new and cite it here?

/be

Comment 9

[Brendan Eich [:brendan]]

Crowder: I missed your setting the new bug as blocked by this one, but that is backwards. The reason is that if one were cherry-picking fixes onto a release branch, one would not want just this bug -- one would want to see that it was blocked by a newer bug, and consider taking that bug's patch too.

/be

Comment 10

[Brian Crowder]

Sorry, assumed "Clone This Bug" would do the right thing, and did not examine dependencies closely enough.