Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upWindowProxy and violations of internal method invariants #672
Comments
bterlson
added
the
web reality
label
Aug 17, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bzbarsky
Aug 22, 2016
I think the other relevant links showing why this could not be implemented are https://bugzilla.mozilla.org/show_bug.cgi?id=1178638#c1 and https://bugzilla.mozilla.org/show_bug.cgi?id=1178638#c2 with the latter being a third-party library.
bzbarsky
commented
Aug 22, 2016
|
I think the other relevant links showing why this could not be implemented are https://bugzilla.mozilla.org/show_bug.cgi?id=1178638#c1 and https://bugzilla.mozilla.org/show_bug.cgi?id=1178638#c2 with the latter being a third-party library. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
erights
Sep 12, 2016
Regarding the proposal in the first bullet at https://bugzilla.mozilla.org/show_bug.cgi?id=1197958#c4 comment 4:
- Redefine the contract of Object.defineProperty so that it can signal failure either by throwing (as now) or by returning false. Returning false rather than throwing is hazardous, because existing clients that rely only on normal control flow to determine success can be misled. Thus, we would allow false only on the narrowest case we can viably specify, such as non-writable, non-configurable on the WindowProxy when the underlying property on the Window (the real but unreifiable global object) has indeed been successfully made non-writable, non-configurable. Object.getOwnPropertydescriptor would still do what the draft spec says it should do -- report the property on the WindowProxy to be configurable. (Crucially, we do not alter the contract of Reflect.defineProperty or the contract of the [[DefineOwnProperty]] trap.)
Do we agree that this would fix all reported breakages? On rereading this, I would be prepared to go even a bit further on weakening how Object.defineProperty reports failure, given that the Reflect.defineProperty and [[DefineOwnProperty]] contracts were not weakened, nor the invariants upheld by these contracts. From the reported breakages, I see no reason to weaken any of those. We need only weaken how Object.defineProperty reports failure.
erights
commented
Sep 12, 2016
|
Regarding the proposal in the first bullet at https://bugzilla.mozilla.org/show_bug.cgi?id=1197958#c4 comment 4:
Do we agree that this would fix all reported breakages? On rereading this, I would be prepared to go even a bit further on weakening how Object.defineProperty reports failure, given that the Reflect.defineProperty and [[DefineOwnProperty]] contracts were not weakened, nor the invariants upheld by these contracts. From the reported breakages, I see no reason to weaken any of those. We need only weaken how Object.defineProperty reports failure. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bzbarsky
Sep 12, 2016
Do we agree that this would fix all reported breakages?
I believe it would, yes. So, importantly, the magic would live in Object.defineProperty, and hence in the ES spec, not in the [[DefineOwnProperty]] of WindowProxy objects?
bzbarsky
commented
Sep 12, 2016
I believe it would, yes. So, importantly, the magic would live in Object.defineProperty, and hence in the ES spec, not in the [[DefineOwnProperty]] of |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
erights
commented
Sep 12, 2016
|
yes, I think that's correct. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
allenwb
Sep 12, 2016
Member
I think a cleaner way to allow for this behavior is to add a new paragraph to https://tc39.github.io/ecma262/#sec-error-handling-and-language-extensions that says something like:
"An implementation my define failure behavior other than throwing TypeError for Object.defineProperty when it's first argument is an implementation provided exotic object."
The HTML spec. would then state that Object.defineObject returns false rather than throwing when its first argument is a Windows Proxy object.
This would keep all of the details out of the ES spec. and put them back into the HTML spec, which is where they probably belong.
|
I think a cleaner way to allow for this behavior is to add a new paragraph to https://tc39.github.io/ecma262/#sec-error-handling-and-language-extensions that says something like: "An implementation my define failure behavior other than throwing TypeError for Object.defineProperty when it's first argument is an implementation provided exotic object." The HTML spec. would then state that Object.defineObject returns false rather than throwing when its first argument is a Windows Proxy object. This would keep all of the details out of the ES spec. and put them back into the HTML spec, which is where they probably belong. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
domenic
Sep 12, 2016
Member
@allenwb's version seems OK but I'd prefer to formalize it with something like a HostObjectDefinePropertyFailureBehavior abstract op which is called as part of the algorithm.
|
@allenwb's version seems OK but I'd prefer to formalize it with something like a HostObjectDefinePropertyFailureBehavior abstract op which is called as part of the algorithm. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
allenwb
Sep 12, 2016
Member
@domenic I think we should avoid adding attractive nuisance hooks the to the ES spec. that could be taken as a general invitation for hosts to do unanticipated bizarre things. In that light, I would further refine my proposed addition to clause 16 to says:
"An implementation my define failure behavior other than throwing TypeError for Object.defineProperty when it's first argument is an implementation provided exotic object and the throwing behavior would cause legacy code to fail."
The HTML spec. would not need to include any algorithmic specifications for this behavior. All it needs to say is: When a WindowProxy is passed as the first argument to Object.defineProperty failure is reported by returning false rather than throwing a TypeError exception.
|
@domenic I think we should avoid adding attractive nuisance hooks the to the ES spec. that could be taken as a general invitation for hosts to do unanticipated bizarre things. In that light, I would further refine my proposed addition to clause 16 to says: "An implementation my define failure behavior other than throwing TypeError for Object.defineProperty when it's first argument is an implementation provided exotic object and the throwing behavior would cause legacy code to fail." The HTML spec. would not need to include any algorithmic specifications for this behavior. All it needs to say is: When a WindowProxy is passed as the first argument to Object.defineProperty failure is reported by returning false rather than throwing a TypeError exception. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
domenic
Sep 12, 2016
Member
Sure. I think our proposals are equivalent except stylistically. I think it's much nicer to have the host-defined hooks called out by specific names and as explicit modifications to the algorithm steps, instead of hidden in paragraphs afterward.
|
Sure. I think our proposals are equivalent except stylistically. I think it's much nicer to have the host-defined hooks called out by specific names and as explicit modifications to the algorithm steps, instead of hidden in paragraphs afterward. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
domenic
Sep 12, 2016
Member
I'll put up a pull request that we can discuss more concretely, using much of your phrasing.
|
I'll put up a pull request that we can discuss more concretely, using much of your phrasing. |
added a commit
to domenic/ecma262
that referenced
this issue
Sep 12, 2016
domenic
referenced a pull request that will
close
this issue
Sep 12, 2016
Open
Layering: add HostObjectDefinePropertyReturnFalse #688
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bzbarsky
Sep 12, 2016
The HTML spec. would not need to include any algorithmic specifications for this behavior.
It totally would, because of the following two constraints:
- We want this code:
Object.defineProperty(window, "gotcha", { configurable: false, value: 5 });
to indicate failure (to make the property non-configurable) by returning false.
2. We want this code:
Object.defineProperty(window, "gotcha", { configurable: false, writable: false, value: 5 });
Object.defineProperty(window, "gotcha", { configurable: false, writable: false, value: 6 });
to throw.
bzbarsky
commented
Sep 12, 2016
•
It totally would, because of the following two constraints:
to indicate failure (to make the property non-configurable) by returning false.
to throw. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
@bzbarsky what's the property name you're defining in those examples? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
bzbarsky
Sep 12, 2016
Er, right. Doesn't matter, as long as it's consistent. I'll edit the examples.
bzbarsky
commented
Sep 12, 2016
|
Er, right. Doesn't matter, as long as it's consistent. I'll edit the examples. |
annevk commentedAug 17, 2016
Basically, the problem raised by @bzbarsky in https://esdiscuss.org/topic/figuring-out-the-behavior-of-windowproxy-in-the-face-of-non-configurable-properties two years ago now was resolved, but the resolution could not be implemented per https://bugzilla.mozilla.org/show_bug.cgi?id=1197958#c4 and therefore the HTML Standard's
WindowProxyobject definition contradicts ECMAScript here after I discussed the situation with bz and @ajklein.See https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-getownproperty (step 3) and https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-defineownproperty (also step 3) for the violations.
(I missed this when reporting the document.all violation as this one was not properly tagged in HTML.)