Closed Bug 632391 Opened 9 years ago Closed 3 years ago

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

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jorendorff, Unassigned)

Details

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.
(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.  :-\
(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.
Assignee: general → nobody
No longer reproducible, resolving as WFM.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.