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

RESOLVED FIXED

Status

()

Core
DOM
--
minor
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Yuh-Ruey Chen, 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?).
(Assignee)

Comment 1

11 years ago
(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)

Updated

11 years ago
Assignee: general → mrbkap
Flags: blocking1.9?
(Assignee)

Updated

11 years ago
Assignee: mrbkap → nobody
Component: JavaScript Engine → DOM
OS: Windows XP → All
QA Contact: general → general
Hardware: PC → All
Version: 1.8 Branch → Trunk
(Assignee)

Updated

11 years ago
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]
(Assignee)

Comment 3

11 years ago
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)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED

Updated

11 years ago
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+
(Assignee)

Comment 5

11 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
(Assignee)

Updated

10 years ago
Duplicate of this bug: 418983
You need to log in before you can comment on or make changes to this bug.