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

RESOLVED FIXED

Status

()

--
minor
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: stryker330, Assigned: mrbkap)

Tracking

({regression})

Trunk
regression
Points:
---
Bug Flags:
blocking1.9 -
wanted1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
Created attachment 253938 [details]
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?).
(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.
Assignee: general → mrbkap
Flags: blocking1.9?
Assignee: mrbkap → nobody
Component: JavaScript Engine → DOM
OS: Windows XP → All
QA Contact: general → general
Hardware: PC → All
Version: 1.8 Branch → Trunk
Assignee: nobody → mrbkap
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?
Flags: blocking1.9? → blocking1.9-
Keywords: regression
Whiteboard: [wanted-1.9]
Created attachment 273176 [details] [diff] [review]
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.
Attachment #273176 - Flags: superreview?(brendan)
Attachment #273176 - Flags: review?(jst)
Status: NEW → ASSIGNED
Attachment #273176 - Flags: review?(jst) → review+
Comment on attachment 273176 [details] [diff] [review]
Proposed fix

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

/be
Attachment #273176 - Flags: superreview?(brendan) → superreview+
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Duplicate of this bug: 418983
You need to log in before you can comment on or make changes to this bug.