913417 - The defineProperty method applied to the window object (or any other direct proxy or DOM proxy) sets all property descriptor values to their defaults.

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[martin.slota]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130803215302

Steps to reproduce:

I tried the following global code in Firefox:

    this.n = 42;
    console.log(Object.getOwnPropertyDescriptor(this, "n"));
    Object.defineProperty(this, "n", {});
    console.log(Object.getOwnPropertyDescriptor(this, "n"));


Actual results:

The first log entry has the value attribute set to 42 and all flags (writable, enumerable, configurable) set to true. The subsequent log has all the attributes set to their defaults, i.e. value is undefined and writable, enumerable, configurable are false.


Expected results:

I believe that the second log should be identical to the first log, just as it would be if a regular object was used instead of window (unless there is a good reason for this behaviour, of course).

Comment 1

[Boris Zbarsky [:bzbarsky]]

So what happens here is that js::DefineProperty calls Proxy::defineProperty with the empty property descriptor.  That lands us in nsOuterWindowProxy::defineProperty, which forwards to js::Wrapper::defineProperty, which is actually js::DirectProxyHandler::defineProperty.

And DirectProxyHandler does:

  return CheckDefineProperty(cx, target, id, v, desc.getter(), desc.setter(),
                             desc.attributes()) &&
         JS_DefinePropertyById(cx, target, id, v, desc.getter(), desc.setter(),
                               desc.attributes());

and now we've clobbered the property that used to be there.

Jeff, is this an issue in DirectProxyHandler (in that it should be doing the "[[GetOwnProperty]] and merge that with the incoming desc" dance), or in js::DefineProperty, or in obj_defineProperty, or in nsOuterWindowProxy::defineProperty?  For normal objects, js::DefineProperty calls DefinePropertyOnObject which does all the 8.12.9 bookkeeping, but it's not clear to me whether the proxy is supposed to be trapping [[DefineOwnProperty]] or some other part of the MOP, and what exactly is supposed to be the right way to get "normal" behavior if [[DefineOwnProperty]] is what's trapped...

Comment 2

[Boris Zbarsky [:bzbarsky]]

You get the same issue with "this" replaced by document.getElementsByTagName("*"), for similar reasons (though there DOMProxyHandler::defineProperty is explicitly calling js_DefineOwnProperty on the expando object).

For the direct proxy handler case, you even get it with JS Proxy objects.  A shell testcase:

  var obj = {};
  var proxy = new Proxy(obj, {});
  proxy.n = 42;
  print(JSON.stringify(Object.getOwnPropertyDescriptor(proxy, "n")));
  Object.defineProperty(proxy, "n", {});
  print(JSON.stringify(Object.getOwnPropertyDescriptor(proxy, "n")));

So _if_ the proxy trap is supposed to represent [[DefineOwnProperty]], what I think we need here is a way of invoking the default [[DefineOwnProperty]], to be used by both DirectProxyHandler and DOMProxyHandler....

Comment 3

[Jeff Walden [:Waldo]]

It's sort of an issue with nsOuterWindowProxy::defineProperty.  But it's not an issue that could be fixed by changing that method, right now, because the ObjectOps::defineProperty signature is insufficiently powerful to implement descriptor-merging.  This comes back to ObjectOps's MOP being not the one we need, as usual.  Fixing this (I'm pretty sure there's a bug on file for it already) is roughly on my list as the next thing after bug 826587.