369259 - this.__defineGetter__/__defineSetter__ in global scope doesn't work

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Yuh-Ruey Chen]

Attachment: testcase

I'm testing this on the 1.8.1 branch (Firefox 2.0) and I don't install trunk nightlies, so I have no idea if this bug still exists on the trunk.

This is an odd and really minor bug. For each of the following examples, suppose we're in the global scope with no properties defined other than the built-in ones.

1)
print(foo);

An exception is thrown as expected (compare the rest of the examples to this one).

2)
this.__defineGetter__('foo', function() { print('getting foo'); return 'foo'; });
print(foo);

Exception is NOT thrown. "undefined" outputted.

3)
__defineGetter__('foo', function() { print('getting foo'); return 'foo'; });
print(foo);

"getting foo", then "foo" outputted.

4)
this.__defineSetter__('foo', function(x) { print('setting foo: '+x); });
foo = 10;
print(foo);

"10" outputted.

5)
__defineSetter__('foo', function(x) { print('setting foo: '+x); });
foo = 10;
print(foo);

"setting foo: 10", then "undefined" outputted.



Examples 1, 3, and 5 work as expected.

In example 2, despite the getter not being called, an exception isn't thrown (so the engine does recognize the property exists).

In example 4, the setter isn't being called.

I have a feeling that this is being caused by there being different global objects (window vs. JS global?).

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #0)
> I have a feeling that this is being caused by there being different global
> objects (window vs. JS global?).

Close, but not quite. I believe the problem is here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/base/nsDOMClassInfo.cpp&rev=1.427&root=/cvsroot&mark=4564-4565#4527 . If the property has a getter or setter (or any other attribute, really), we will lose that information when forwarding the property to the inner window. I think we need to JS_GetPropertyAttrsGetterAndSetter and forward those through OBJ_DEFINE_PROPERTY.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This doesn't seem like a blocker given that this bug exists on the 1.8 branch too. However it would be great if you could have a go at this one blake?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

It might be good for someone on Windows to test to make sure that this links there, but this seems to do the trick. Note that I think that we're safe from infinite recursion, since the LOOKUP_PROPERTY is guaranteed to find the property in the scope.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 273176 [details] [diff] [review]
Proposed fix

Don't need to initialize prop, FYI -- pure out param.

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.