Allow for customisation of the "get the parent" algorithm #1230
Conversation
keithamus
force-pushed
the
allow-for-customisation-of-the-get-the-parent-algorithm
branch
from
8de9564 to
652ef26
Compare
There was a problem hiding this comment.
Some initial feedback. Mainly around how to organize this. Overall I think this works, but would love to hear from @rniwa and @smaug---- especially.
dom.bs
Outdated
| <p>Each {{EventTarget}} object also has an associated <dfn export>get the parent</dfn> algorithm, | ||
| which takes an <a>event</a> <var>event</var>, and returns an {{EventTarget}} object. Unless | ||
| specified otherwise it returns null. | ||
| which takes an <a>event</a> <var>event</var>, and returns an {{EventTarget}}. Unless otherwise | ||
| specified otherwise it returns the associated <a for=EventTarget>attached internals</a><a | ||
| for=EventTargetInternals">parent</a> attribute. |
There was a problem hiding this comment.
I think instead we should make the parent setter overwrite this algorithm with an algorithm that returns the given value.
There was a problem hiding this comment.
Okay I've added some steps which I think resolve this but let me know, I'm sure it could use some refinement.
keithamus
force-pushed
the
allow-for-customisation-of-the-get-the-parent-algorithm
branch
4 times, most recently
from
2870b4a to
5eef8a5
Compare
dom.bs
Outdated
| for=EventTargetInternals>eventTarget</a> then<a>throw</a> a "{{HierarchyRequestError!!exception}}" | ||
| {{DOMException}}. | ||
|
|
||
| <li>Set <var>internal</var>'s <a for=EventTargetInternals>eventTarget</a> <a>get the parent</a> |
There was a problem hiding this comment.
I couldn't find precedent for setting an algorithm so this is just made up, hopefully it suffices?
keithamus
force-pushed
the
allow-for-customisation-of-the-get-the-parent-algorithm
branch
from
5eef8a5 to
def63ff
Compare
dom.bs
Outdated
| @@ -1322,6 +1373,9 @@ property of the event being dispatched. | |||
| <p>While <var>parent</var> is non-null:</p> | |||
|
|
|||
| <ol> | |||
| <li>If the <var>event</var>'s <a for=Event>path</a> <a for=set>contains</a> <var>parent</var> | |||
| then <a>throw</a> a "{{HierarchyRequestError!!exception}}" {{DOMException}}. | |||
|
|
|||
There was a problem hiding this comment.
This would be slow. I don't think we can do this in normal case when dispatching events to dom tree. The relevant code in implementations is so hot that extra virtual c++ calls show up, as an example.
We need to ensure that there are now loops when setting up the parents. That is already done for DOM nodes anyhow - DOM tree doesn't contain loops.
There was a problem hiding this comment.
Would you prefer us to check the chain during setting of the parent? Would that suffice?
There was a problem hiding this comment.
Okay I think I've resolved this
| interface EventTargetInternals { | ||
| attribute EventTarget parent; | ||
| }; | ||
|
|
There was a problem hiding this comment.
This feels a bit cumbersome API, one needs to pass a callback and then use its param to change a property value.
There was a problem hiding this comment.
Would you prefer an init dictionary? One downside to this, as explained in #583 (comment) is that an init dictionary cannot allow setting the parent post-construction.
There was a problem hiding this comment.
Yeah, I suppose we could also do what new Promise() does, but it doesn't seem as extensible:
new EventTarget(setParent => {
setParent(someParent);
}I think as long as the capability is only granted to whoever constructed the EventTarget object I'm okay, so alternative proposals welcome.
keithamus
force-pushed
the
allow-for-customisation-of-the-get-the-parent-algorithm
branch
2 times, most recently
from
3a8cd2b to
601544f
Compare
This modifies the EventTarget IDL to allow assignment of a parent EventTarget to an EventTarget instance, while also modifying the "get the parent" algorithm to default to returning that instance. It also modifies the Event Dispatch algorithm to ensure that custom parent chains cannot cause loops.
keithamus
force-pushed
the
allow-for-customisation-of-the-get-the-parent-algorithm
branch
from
601544f to
b5db7fa
Compare
This modifies the EventTarget IDL to allow assignment of a parent EventTarget to an EventTarget instance, while also modifying the "get the parent" algorithm to default to returning that instance.
It also modifies the Event Dispatch algorithm to ensure that custom parent chains cannot cause loops.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff