632391 - ES5 accessor properties created with Object.defineProperty can't be watched

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

This bug prevents me from landing the test in bug 631305 comment 5.

  var s = '';
  var obj = {};
  Object.defineProperty(obj, "p", {configurable: true, enumerable: true,
                                   get: function () { return 0; },
                                   set: function (v) { s += v; }});
  obj.watch("p", function (id, oldv, newv) { s += 'w'; return 'x'; });
  obj.p = 'v';
  assertEq(s, 'wx');  // FAILS

Note that if the object is set up like this:
  var obj = {get p() { return 0; }, set p(v) { s += v; }};
which is ostensibly the same thing, it passes.

The bug is: (a) Object.defineProperty makes a JSPROP_READONLY property; and (b) Object.prototype.watch for whatever reason silently ignores any request to watch a READONLY property (as determined by js::CheckAccess).

Waldo can check me on this: I think in our brave new ES5-inspired world, the READONLY bit of an accessor property should never be checked at all. But if that's the case, it should never be set on such properties, just for consistency.

Comment 1

[Jeff Walden [:Waldo]]

(In reply to comment #0)
> I think in our brave new ES5-inspired world, the READONLY bit of an accessor
> property should never be checked at all.

Word to your mother.

> But if that's the case, it should never be set on such properties, just for
> consistency.

Why not the other direction?

...I kid, I kid.  Mostly.  Somewhat.  Kind of.  :-\

Comment 2

[Jason Orendorff [:jorendorff]]

(In reply to comment #1)
> (In reply to comment #0)
> > I think in our brave new ES5-inspired world, the READONLY bit of an accessor
> > property should never be checked at all.
> > But if that's the case, it should never be set on such properties, just for
> > consistency.
> 
> Why not the other direction?

Clearing the bit is the conservative thing since that's what we've done historically. Thus it's less likely to trigger bugs like the one in obj_watch.

Or we could ruthlessly eradicate code that could possibly have such bugs. That's some busywork that might or might not pay for itself.

Comment 3

[André Bargull [:anba]]

No longer reproducible, resolving as WFM.