-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove boundary events and use capture node as target #138
Conversation
<li>Set the <code>relatedTarget</code> attribute of the event to <code>null</code>.</li> | ||
<li>Set the target to the <a>pointer capture target override</a> object.</li> | ||
</ul> | ||
<li>If the <a>pointer capture target override</a> has been set for the pointer, set the target to <a>pointer capture target override</a> object. This may cause some boundary events as this is the same as pointer leaving its previous target and entering this new target. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This may fire some boundary events...the same as the pointer leaving...
<li>Otherwise, set the target to the object returned by normal hit test mechanisms (out of scope for this specification).</li> | ||
</ul> | ||
|
||
<p>If this is a <code>pointerdown</code> event, the associated device is a direct manipulation device and the target is an <code>Element</code>, then <a href="setting-pointer-capture">set pointer capture</a> for this <code>pointerId</code> to the target element as described in <a>implicit pointer capture</a>. | ||
|
||
<p>Fire the event to the determined target. | ||
|
||
<div class="note">Using the <a>pointer capture target override</a> as the target instead of the normal hit-test result may fire some boundary events. This is the same as the pointer leaving its previous target and entering this new capturing target and if they are different targets boundary events should be dispatched first. When the capture is released the same scenario may happen as the pointer is leaving the capturing node and entering the hit-test target.</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little hand-wavy (it's non-normative text but it describes behavior that isn't captured by the normative spec text outside of notes). As you know I'm OK with being a little imprecise (as least for now) as long as we have tests that validate the intended precise behavior.
One simple way to improve this may be to change the sections that describe the boundary events. They currently say "when a pointing device is moved into the hit test boundaries of ..." etc. If instead they said something like "when the target for the events for a given pointerId changes to ...". I.e. boundary events are generated as a result of changes in event targets within a per-pointer event stream, not as a result of movement around "hit-test boundaries". Alternately in the section on pointer capture you could just say that capture modifies the (unspecified) hit-test algorithm so that all other references to hit-testing in the spec use the pointer capture target override.
Even better, this could be encoded precisely in the firing algorithm (eg. by defining a "last target per pointer" map). But that's probably a fair bit more work. Maybe file a spec bug for that to explore in detail later?
Seems like we're just focused on details. Since this is on a branch already, should we merge this PR as-is and then continue to iterate on details in a subsequent (less urgent) PR? |
I agree that the only remaining piece here is defining the modified triggering of boundary events. I think Rick's idea that boundary events triggers "when the target ... for a given pointerId changes..." is the cleanest solution. IMO all we need after that is to clarify the non-normative note added in this patch with an example capture/release from outside the hittest element. LGTM on hashing out the details through future PRs. |
Ok, merged. Let's keep iterating though. |
This fix addresses issue #8