Bug 1122619 Comment 5 > Please file a bug, if you haven't already, blocking the es6 tracking bug: this > doesn't follow the spec. We should get rid of SetPropertyAttributes and use > JSPROP_IGNORE_ENUMERATE and JSPROP_IGNORE_VALUE to do this sort of partial > DefineProperty.

I had to add checks for TypedArray, this should probably be a different bug. Two tests: ecma_5/extensions/reviver-mutates-holder-object-nonnative.js ecma_5/extensions/reviver-mutates-holder-array-nonnative.js Fail, but they are probably flawed, because you can't define something with configurable: true. Which should happen in the CreateDataProperty call per Spec. Something weird that I noticed is that: Object.defineProperty(t, 0, {configurable: true}) doesn't throw.

Oh and this didn't work either before: x = new Int8Array(2) Object.defineProperty(x, 0, {value: 2}) x[0] == 2

This fails less tests, because I moved the TypedArray checks into StandardDefineProperty in bug 1125628.

Oh and I would much rather use the PropDesc version of StandardDefineProperty, but we first need to invent a good syntax for e.g. "enumerable" is absent/true/false.

Added tests and step annotations.

Comment on attachment 8559126 [details] [diff] [review] v2 - Remove SetPropertyAttributes Review of attachment 8559126 [details] [diff] [review]: ----------------------------------------------------------------- Stealing, as discussed on IRC. This is great. Glad to see this go. Drop-in attribute changes were unappropriately checked. r=me ::: js/src/jsobj.cpp @@ +1152,5 @@ > RootedId id(cx); > Rooted<PropertyDescriptor> desc(cx); > + > + const unsigned AllowConfigure = JSPROP_IGNORE_ENUMERATE | JSPROP_IGNORE_READONLY | > + JSPROP_IGNORE_VALUE; Man, I didn't expect people would actually *use* these when I added them. What a hack. Oh well, it's a useful stopgap.

Oh nice, getting this landed is going to be royal pain: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be37a8a5539e

Okay I think I found the problem. When invoking defineProperty on a scripted proxy we forward to DirectProxyHandler::defineProperty, which uses CheckDefineProperty. This function doesn't handle JSPROP_IGNORE_VALUE, so it complains when trying to change a non-configurable and non-writable property. e.g. QueryInterface is such a property, we invoke defineProperty(.., {configurable: false, writable: false}). But CheckDefineProperty sees a value of `undefined`, which is different from the QueryInterface function. I am not quite sure why DirectProxyHandler::defineProperty uses CheckDefineProperty, instead of StandardDefineProperty. If we can't fix that we should at least change ScriptedDirectProxyHandler to call StandardDefineProperty instead.

Comment on attachment 8562894 [details] [diff] [review] Remove CheckDefineProperty use StandardDefineProperty instead Review of attachment 8562894 [details] [diff] [review]: ----------------------------------------------------------------- Yes yes yes yes yes yes yes. So happy to see this go. It was horribly wrong, relative to the current spec.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4de5e527fbf https://hg.mozilla.org/integration/mozilla-inbound/rev/c93e99adfc7d

https://hg.mozilla.org/mozilla-central/rev/f4de5e527fbf https://hg.mozilla.org/mozilla-central/rev/c93e99adfc7d