Improve the named property visibility algorithm for Window. #626
Conversation
| 1. While |prototype| is not null: | ||
| 1. If |prototype| is not a [=named properties object=], | ||
| and |prototype| has an own property named |P|, then return false. |
There was a problem hiding this comment.
Are we sure that removing this clause is safe for the other 3 or so call sites of "named property visiblity algorithm"?
There was a problem hiding this comment.
It'll only affect the other call sites if someone takes an object that supports named properties and sticks the named properties object into its proto chain. I don't think we need to care particularly about that scenario.
There was a problem hiding this comment.
Well, it's observable, right? If so, what do implementations do?
There was a problem hiding this comment.
IIRC, it's only observable in Gecko, and Gecko matches the new spec text.
In particular, if a string is a supported property name as well as an own property of the window or Window.prototype object, we don't need to block that property from existing on the named properties object, since it will be shadowed automatically. Fixes #607.
|
Any more thoughts? |
There was a problem hiding this comment.
Apologies for the lack of follow-up, I was hoping someone else would have picked this up.
@EdgarChen could you also take a look?
| @@ -12636,21 +12636,22 @@ internal method as follows. | |||
| is used to determine if a given named property is exposed on an object. | |||
| Some named properties are not exposed on an object depending on whether | |||
| the [{{OverrideBuiltins}}] [=extended attribute=] was used. | |||
| The algorithm operates as follows, with property name |P| and object |O|: | |||
| The algorithm operates as follows, with property name |P|, object |O|, and | |||
| optionally object |propertyHolder|: | |||
There was a problem hiding this comment.
Where is this invoked with propertyHolder?
|
@shvaikalesh this looks like it's in the same area as other stuff you've been doing. Would you be interested in picking this up (if it's still applicable)? |
|
@domenic Yes, it's still applicable and the change around https://github.com/whatwg/webidl/pull/626/files#r251604914 is a very nice simplification that matches WebKit implementation. Hopefully, I will pick up this PR in a few weeks or so. Thanks! |
|
That's great! (I didn't mean to close this by the way, it seems that somehow the branch rename didn't affect this PR and then when the master branch was removed it got automatically closed.) |
That happens when the fork repository or branch was deleted, leaving behind a dangling PR. |
In particular, if a string is a supported property name as well as an own
property of the window or Window.prototype object, we don't need to block
that property from existing on the named properties object, since it will
be shadowed automatically.
Fixes #607.
Preview | Diff