setProperty function is never called

RESOLVED INVALID

Status

()

RESOLVED INVALID
8 years ago
8 years ago

People

(Reporter: rehrlich, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0; GTB6; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30; .NET CLR 3.0.04506.648; .NET CLR 3.5.21022; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729; OfficeLiveConnector.1.3; OfficeLivePatch.0.0; .NET4.0C)
Build Identifier: 1.8.5

I am converting an application for JS 1.7 to JS 1.8.5 and have encountered a 
serious problem. The setProperty function is never called. For a particular 
class, I have the following properties defined:

static JSPropertySpec cardProperties[] = {
   {"timeout", PROP_ID_TIMEOUT,
    JSPROP_ENUMERATE | JSPROP_PERMANENT,
    cardPropertyGetTimeout, cardPropertySetTimeout},
   {NULL, 0, 0, NULL, NULL}
};

In Javascript I use:

c = new Card();
i = c.timeout;
c.timeout = 10;

The get function is called, but not the set function.

I verified that the set function is called in JS 1.7.

The JSClass setter is called, but not the setter in the properties array.

Reproducible: Always
What's the signature of your cardPropertySetTimeout function?  It's a bit odd that it's not being called at all; I'd not be surprised if it's being called incorrectly...
(Reporter)

Comment 2

8 years ago
Created attachment 531938 [details]
A very simple test file demonstrating the problem

If you run this very simple test you will see that the constructor and the getter is called once, but the setter is never called.
(Reporter)

Comment 3

8 years ago
The signature is:

static JSBool cardPropertySetTimeout(JSContext *cx, JSObject *obj, jsid id, JSBool strict, jsval *vp)

I have submitted a very simple test program that demonstrates the problem.
Hmm.  Yeah, that should all work....  Are you just running pure interp with no jit?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

8 years ago
I agree it should work, but it doesn't. Could someone please try running my simple test to verify? I am running on a WIN32 build with all the default options plus --enable-threadsafe --enable-trace-jscalls. I agree it is very strange that this feature is failing. I have run quite a few tests and the setter callback from the JSPropertySpec table is never called.
Does the problem go away if you take out the "JSOPTION_JIT | JSOPTION_METHODJIT" jscontext options?
(Reporter)

Comment 7

8 years ago
> Does the problem go away if you take out the "JSOPTION_JIT | 
> JSOPTION_METHODJIT" jscontext options?

No. Exactly the same result.
Does the setter get called if you do not run the getter before that?  So in this code:

c = new Card();
c.timeout = 10;
(Reporter)

Comment 9

8 years ago
If I run:

c = new Card();
c.timeout = 10;

the setter is still not called.
(Reporter)

Comment 10

8 years ago
I am definitely not an expert on the Javascript engine, but the bug seems to be in js_SetPropertyHelper in jsobj.cpp. This function initially gets the setter from the class (jsobj.cpp:5623). At line 5683 the setter is reset from the shape where is property setter is stored, but only (!shape->hasSlot()). I think it should always get the setter from the shape.
Hmm.  So if you add JSPROP_SHARED for that property, do things work?
(Reporter)

Comment 12

8 years ago
> Hmm.  So if you add JSPROP_SHARED for that property, do things work?
Yes, but I don't know what the side effects of adding JSPROP_SHARED are. I realize that this may be a workaround, but it seems to me to be a kludge solution.
The engine is behaving correctly. Add JSPROP_SHARED if you want the property to behave like an ES5 accessor property. (That API oddity probably predates ES5 by ten years, so there's little we can do about it now.)

The consequences of adding JSPROP_SHARED are:

  - assigning to the property on an object that inherits the property from
    a prototype calls the setter (exactly the behavior you want)

  - no field is allocated for the property

  - "the shared permanent hack", a bit of obscure legacy oddness which you
    can ignore, since this property is not also JSPROP_PERMANENT.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → INVALID
P.S. Thank you for providing a complete test case. I was able to figure out what was going on in a few minutes because of that. Otherwise this would never have been resolved.
(Reporter)

Comment 15

8 years ago
Thank you for the help in resolving this problem. In order to make the conversion from JS 1.7 to JS 1.8.5, I would like to suggest that this change be documented in the 1.8.5 release notes and that the JSPropertySpec documentation specify when the setter is called.
Jason, the weird thing here is the behavior change and the fact that presumably scripted setters on the proto _do_ get run.... or do those get defined as JSPROP_SHARED?
Scripted setters get defined with JSPROP_SHARED.
That there was a behavior change is surprising to me. I thought it had always worked this way. Jeff, did we change something in the FF4 timeframe, maybe when straightening up ES5 accessor behavior?
I'm not aware of any such change, but seemingly something did change, and that's as plausible a guess as anything.
You need to log in before you can comment on or make changes to this bug.