Skip to content
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

got/lostpointercapture events: attributes and immediate firing on implicit release #122

Merged
merged 9 commits into from Aug 29, 2016

Conversation

@NavidZ
Copy link
Member

commented Jul 28, 2016

Closes #113 and #32.

@NavidZ NavidZ force-pushed the NavidZ:got-lost-pointer-capture-attributes branch from 3cbe5b9 to 8585da3 Jul 28, 2016

index.html Outdated
@@ -423,6 +423,8 @@
<li><code>pointerout</code></li>
</ul>

<p>Run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> steps for this <code>PointerEvent</code> if it is not <code>gotpointercapture</code> or <code>lostpointercapture</code>.

This comment has been minimized.

Copy link
@RByers

RByers Jul 29, 2016

Contributor

nit: reverse the sentence "If the event is not ..., run ..."

index.html Outdated
@@ -546,7 +548,7 @@
</tbody>
</table>
<p>In the case of the <a>primary pointer</a>, these events (with the exception of <code>gotpointercapture</code>, and <code>lostpointercapture</code>) may also fire <a>compatibility mouse events</a>.</p>
<p>For all PointerEvents in the table above, <a href="https://www.w3.org/TR/uievents/#widl-UIEvent-detail">detail</a> attribute defined in [[!DOM-LEVEL-3-EVENTS]] SHOULD be 0.</p>
<p>For all PointerEvents in the table above, <a href="https://www.w3.org/TR/uievents/#widl-UIEvent-detail"><code>detail</code></a> attribute defined in [[!DOM-LEVEL-3-EVENTS]] SHOULD be 0. For <code>gotpointercapture</code> and <code>lostpointercapture</code> all the attributes except the ones defined in the table above should be the same as the Pointer Event that causes the user agent to run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> and fire <code>gotpointercapture</code> and <code>lostpointercapture</code> events.</p>

This comment has been minimized.

Copy link
@RByers

RByers Jul 29, 2016

Contributor

I realize now that this bit about detail really fits better in the "firing events" section (since that's what's describing how events get fired, including setting other properties like bubbles and cancelable. Mind moving it?

Then I think copying the properties for got/lostpointercapture make the most sense in the "process pending pointer capture" section. In fact I think the property copying also applies to all the other events fired there (over,enter,leave,out) too, right? So maybe just one sentence at the top of 5.1.3.1 indicating that this is done only in response to some other pointer event, and (unless indicated otherwise) all events fired in that section get their property values from the original event. WDYT?

This comment has been minimized.

Copy link
@patrickhlauke

patrickhlauke Jul 29, 2016

Member

nit: "that causes the user agent to run " > "that caused the user agent to run"; "and fire <code>gotpointercapture</code>" > "and fire the <code>gotpointercapture</code>"

index.html Outdated
@@ -854,7 +856,7 @@
<div class="note">Some user agents implement their own implicit pointer capture behavior - for instance, for touch interactions, a user agent could automatically capture the pointer as part of an interaction on a form control (such as a button) to improve user interaction (allowing some finger movement to stray outside of the form control itself during the interaction). As part of this behavior, user agents typically fire <code>gotpointercapture</code> and <code>lostpointercapture</code> events, even though no explicit pointer capture functions (<code>setPointerCapture</code> and <code>releasePointerCapture</code>) were called.</div>
<section>
<h3>Implicit Release of Pointer Capture</h3>
<p>Immediately after firing the <code>pointerup</code> or <code>pointercancel</code> events, a user agent MUST run the steps as if the <code>releasePointerCapture()</code> method has been called with an argument equal to the <code>pointerId</code> property of the <code>pointerup</code> or <code>pointercancel</code> event just dispatched.</p>
<p>Immediately after firing the <code>pointerup</code> or <code>pointercancel</code> events, a user agent MUST run step 4 in <a href="#releasing-pointer-capture">Releasing Pointer Capture</a> with the <code>pointerId</code> property of the <code>pointerup</code> or <code>pointercancel</code> event just dispatched and then run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> steps to fire <code>lostpointercapture</code> if necessary.</p>

This comment has been minimized.

Copy link
@RByers

RByers Jul 29, 2016

Contributor

Rather than say "step 4" I think you can continue to say "run the steps", right? It looks like they're written to handle this case (it's kind of weird to jump into the middle of the steps, and the step numbers may change). Alternately you can just say "clear the pending pointer capture override for the specified pointerId" if you think that's simpler.

This comment has been minimized.

Copy link
@patrickhlauke

patrickhlauke Jul 29, 2016

Member

nit: "event just dispatched and then run" > "event that was just dispatched, and then run" (note comma)

This comment has been minimized.

Copy link
@NavidZ

NavidZ Jul 29, 2016

Author Member

The problem with those steps is that they expect an element to be passed to release pointer capture api. But previously it was not specified on which element release pointer capture steps should be run. Since the rest of the steps have no effect in this case I just mentioned step 4. I can certainly change it to the text if you think that is okay too.

This comment has been minimized.

Copy link
@RByers

RByers Jul 29, 2016

Contributor

You could just address that by augmenting "as if the releasePointerCapture() method has been called with an argument equal to the pointerId property of the pointerup or pointercancel event just dispatched." to include "on the pending pointer capture override element". Either way is OK with me.

@RByers

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2016

Big picture looks good to me, thanks! Just a couple minor style / editorial suggestions.

@NavidZ

This comment has been minimized.

Copy link
Member Author

commented Jul 29, 2016

I moved the table to the other section and removed the "Composed" column from it as we do mention setting composed attribute to true for all the pointer events at first.
Regarding the attributes of got/lostpointercapture, I'm not sure if we split the attributes values into two sections makes things clearer. Right now they are all in this "Firing event.." section. Regarding that boundary events I believe we would be able to remove them all from that section in future PRs. Right?

@RByers

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2016

Personally I think the table makes more sense in 5.2 as a summary of that section (listing all the events). Having a line in the firing algorithm saying "initialize the properties according to this table" (link) seems adequate to me. But I don't have a strong opinion, this is all just editorial. @patrickhlauke WDYT?

@patrickhlauke

This comment has been minimized.

Copy link
Member

commented Jul 29, 2016

Personally I think the table makes more sense in 5.2 as a summary of that section (listing all the events). Having a line in the firing algorithm saying "initialize the properties according to this table" (link) seems adequate to me.

yeah, i'd agree with that

@NavidZ NavidZ force-pushed the NavidZ:got-lost-pointer-capture-attributes branch from 0315d08 to 578e45e Jul 29, 2016

index.html Outdated
<td>None</td>
</tr>
<tr>
<td><code>lostpointercapture</code></td>
<td>Sync/Async</td>
<td>Yes</td>
<td>No</td>
<td>Yes</td>
<td>None</td>
</tr>
</tbody>
</table>
<p>In the case of the <a>primary pointer</a>, these events (with the exception of <code>gotpointercapture</code>, and <code>lostpointercapture</code>) may also fire <a>compatibility mouse events</a>.</p>

This comment has been minimized.

Copy link
@mustaqahmed

mustaqahmed Aug 2, 2016

Contributor

Please get rid of the comma before "and lostpointercamture...".

index.html Outdated
<p>Initialize the <code>bubbles</code> and <code>cancelable</code> attributes for the event according to the <a href="#pointer-event-type-table">Pointer Events table</a>.</p>
<p>For <code>gotpointercapture</code> and <code>lostpointercapture</code> all the attributes except the ones defined in the table above should be the same as the Pointer Event that caused the user agent to run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> and fire the <code>gotpointercapture</code> and <code>lostpointercapture</code> events.</p>

<p>If the event is not <code>gotpointercapture</code> or <code>lostpointercapture</code>, run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> steps for this <code>PointerEvent</code>.

This comment has been minimized.

Copy link
@mustaqahmed

mustaqahmed Aug 2, 2016

Contributor

This para seems odd given that this section talks mostly about setting attributes. I suggest getting rid of this para, instead grouping all paras except the first one (the definition of "firing") in this block into a new section "5.1.3.1 Initializing attributes". The "Process Pending Pointer Capture" section will then become 5.1.3.2---the obvious next step.

index.html Outdated
<p>Run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> steps for this <code>PointerEvent</code> if it is not <code>gotpointercapture</code> or <code>lostpointercapture</code>.
<p>For any pointer event initialize the <code>composed</code> ([[!WHATWG-DOM]]) attribute to <code>true</code> and <a href="https://www.w3.org/TR/uievents/#widl-UIEvent-detail"><code>detail</code></a> [[!DOM-LEVEL-3-EVENTS]] attribute to 0.</p>
<p>Initialize the <code>bubbles</code> and <code>cancelable</code> attributes for the event according to the <a href="#pointer-event-type-table">Pointer Events table</a>.</p>
<p>For <code>gotpointercapture</code> and <code>lostpointercapture</code> all the attributes except the ones defined in the table above should be the same as the Pointer Event that caused the user agent to run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> and fire the <code>gotpointercapture</code> and <code>lostpointercapture</code> events.</p>

This comment has been minimized.

Copy link
@mustaqahmed

mustaqahmed Aug 2, 2016

Contributor

Replace "table above" with the link too. On my first reading, I scrolled up to find the buttons table!

@mustaqahmed mustaqahmed changed the title Attributes of got/lostpointercapture events got/lostpointercapture events: attributes and immediate firing on implicit release Aug 2, 2016

@NavidZ

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2016

Anyone else has any comment about this PR?

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2016

Am I the only one to think that the mix of "attribute setting" and "event firing" between section headers 5.1.3 and 5.1.3.1 could be made cleaner?

@NavidZ

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2016

I personally agree with you. Do you think just moving all attribute values to 5.2 along with that table and reference that section for all attributes be better? Regarding firing events in 5.1.3 vs 5.1.3.1, they are a bit different I believe. They are 2 different routines that call into each other in some situations. WDTY?

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2016

I personally agree with you. Do you think just moving all attribute values to 5.2 along with that table and reference that section for all attributes be better?

Yes, that should be cleaner than referencing that table twice before it appears.

Regarding firing events in 5.1.3 vs 5.1.3.1, they are a bit different I believe. They are 2 different routines that call into each other in some situations. WDTY?

The mix of "setting attributes" and "firing details" looked a bit odd to me. Your suggestion above would reduce the "setting" part in 5.1.3 into a one-line reference to 5.2, which seems better. So, please don't split 5.1.3.

@NavidZ

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2016

Does this look okay?

index.html Outdated

<p>For any pointer event initialize the <code>composed</code> ([[!WHATWG-DOM]]) attribute to <code>true</code> and <a href="https://www.w3.org/TR/uievents/#widl-UIEvent-detail"><code>detail</code></a> [[!DOM-LEVEL-3-EVENTS]] attribute to 0.</p>
<p>Initialize the <code>bubbles</code> and <code>cancelable</code> attributes for the event according to the <a href="#pointer-event-type-table">Pointer Events table</a>.</p>
<p>For any pointer event initialize the attributes of the event according to the <a href="#h-pointer-event-types">Pointer Event types</a> section.</p>
<p>For <code>gotpointercapture</code> and <code>lostpointercapture</code> all the attributes except the ones defined in the table above should be the same as the Pointer Event that caused the user agent to run <a href="#process-pending-pointer-capture">Process Pending Pointer Capture</a> and fire the <code>gotpointercapture</code> and <code>lostpointercapture</code> events.</p>

This comment has been minimized.

Copy link
@mustaqahmed

mustaqahmed Aug 24, 2016

Contributor

This looks cleaner now, thanks. LGTM except for one last comment: please fix the "table above" here by moving this para to somewhere around Line 520.

This comment has been minimized.

Copy link
@NavidZ

NavidZ Aug 24, 2016

Author Member

Oops. Thanks for the catch. I missed this one. Done.

@mustaqahmed

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2016

LGTM.

index.html Outdated
@@ -400,29 +400,9 @@
<section>
<h2>Firing events using the <code>PointerEvent</code> interface</h2>
<p>To <dfn>fire a pointer event name e</dfn> means to <dfn>fire an event named e</dfn> as defined in [[!DOM4]] with an event using the <a>PointerEvent</a> interface whose attributes are set as defined in <a href="#pointerevent-interface"><code>PointerEvent</code> Interface</a>.</p>
<p>For any pointer event initialize the attributes of the event according to the <a href="#h-pointer-event-types">Pointer Event types</a> section.</p>

This comment has been minimized.

Copy link
@RByers

RByers Aug 25, 2016

Contributor

I think would be clearer if merged the previous paragraph since it's really a continuation of the theme of initializing the attributes. Maybe: "... whose attributes are set as defined in PointerEvent interface, and in Pointer Event types for the type e.".

Also while you're editing that line, can you fix the typo: "fire a pointer event name e" -> "fire a pointer event named e"?

@RByers

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2016

LGTM

@RByers

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2016

Oh one more thing I almost forgot again before merging - add a ChangeLog entry with a link to this PR.

@RByers

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2016

Great except now you have a merge conflict - need to rebase your branch

@RByers RByers merged commit e37f6d3 into w3c:gh-pages Aug 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.