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

When are lost/gotpointercapture delayed until the next pointer event? #32

Closed
NavidZ opened this Issue Jan 26, 2016 · 21 comments

Comments

Projects
None yet
7 participants
@NavidZ
Member

NavidZ commented Jan 26, 2016

Looking at this
https://www.w3.org/TR/pointerevents/#process-pending-pointer-capture
and
https://www.w3.org/TR/pointerevents/#setting-pointer-capture

It seems that user agent should not fire gotpointercapture right after setPointerCapture and instead fire it right before the next pointer event with the captured id. But in this example I can see Edge doesn't do that and instead fires gotpointercapture right after pointerdown for the green div.

https://jsbin.com/hivude/edit?html,output

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Jan 26, 2016

Contributor

I agree Edge (and IE 11) do not appear to be matching the spec here. Digging into the history I see this logic was added by @jacobrossi in an effort to describe IE's implementation (at least at the time - April 2014). Perhaps there's a detail of IE's implementation he missed? The Edge behavior does seem potentially (slightly) superior to what the spec implies, and I'd be worried about subtle compat issues if we didn't match Edge's behavior /cc @teddink

As you showed me in another demo, Edge does seem to mostly follow this logic (eg. if capture ping-pongs back and forth between two elements, it'll get updated exactly once per pointermove event).

Contributor

RByers commented Jan 26, 2016

I agree Edge (and IE 11) do not appear to be matching the spec here. Digging into the history I see this logic was added by @jacobrossi in an effort to describe IE's implementation (at least at the time - April 2014). Perhaps there's a detail of IE's implementation he missed? The Edge behavior does seem potentially (slightly) superior to what the spec implies, and I'd be worried about subtle compat issues if we didn't match Edge's behavior /cc @teddink

As you showed me in another demo, Edge does seem to mostly follow this logic (eg. if capture ping-pongs back and forth between two elements, it'll get updated exactly once per pointermove event).

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Apr 25, 2016

Contributor

Ping @teddink / @jacobrossi, can you guys propose a spec change to more precisely match the Edge behavior here (and/or link to a Edge bug that tracks changing behavior to match the spec)?

Contributor

RByers commented Apr 25, 2016

Ping @teddink / @jacobrossi, can you guys propose a spec change to more precisely match the Edge behavior here (and/or link to a Edge bug that tracks changing behavior to match the spec)?

@teddink

This comment has been minimized.

Show comment
Hide comment
@teddink

teddink May 17, 2016

I will propose a spec change to match the Edge behavior, which is what we intended, but perhaps did not communicate well in the spec.

teddink commented May 17, 2016

I will propose a spec change to match the Edge behavior, which is what we intended, but perhaps did not communicate well in the spec.

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers May 17, 2016

Contributor

Thanks. Even better than just a spec change would be some new tests to cover the expected behavior and help us to confirm we are interoperable here.

Contributor

RByers commented May 17, 2016

Thanks. Even better than just a spec change would be some new tests to cover the expected behavior and help us to confirm we are interoperable here.

@teddink

This comment has been minimized.

Show comment
Hide comment
@teddink

teddink Jun 6, 2016

I now understand the behavior that Edge (and IE11) currently has. Edge will immediately fire gotpointercapture ONLY when setpointercapture was called as a result of pointerdown. The code does NOT do anything special for lostpointercapture - so lostpointercapture continues to follow the spec as written. I will also comment on PR #76 by @NavidZ.

We made this change quite a while ago in response to a reported bug where with touch press and hold or a mouse press without mouse move, the gotpointercapture was delayed for an indeterminate amount of time which caused an issue for at least one site.

teddink commented Jun 6, 2016

I now understand the behavior that Edge (and IE11) currently has. Edge will immediately fire gotpointercapture ONLY when setpointercapture was called as a result of pointerdown. The code does NOT do anything special for lostpointercapture - so lostpointercapture continues to follow the spec as written. I will also comment on PR #76 by @NavidZ.

We made this change quite a while ago in response to a reported bug where with touch press and hold or a mouse press without mouse move, the gotpointercapture was delayed for an indeterminate amount of time which caused an issue for at least one site.

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Jun 7, 2016

Member

There is one more thing. Edge does send a pointer move after about 3 seconds when I press mouse button and don't move at all. That causes the lostpointercapture or gotpointercapture be dispatched by that time at least once. I don't know what the origin of that synthetic event is. But Chrome or FF doesn't send that event. Do you think that might create some other compat issues that lost/gotpointercapture is delayed for an indeterminate amount on Chrome or other browsers if js sets the capture say on pointermove? I can see now the behavior of the Edge is what you say but how can we really prevent that indeterminate delay?

Member

NavidZ commented Jun 7, 2016

There is one more thing. Edge does send a pointer move after about 3 seconds when I press mouse button and don't move at all. That causes the lostpointercapture or gotpointercapture be dispatched by that time at least once. I don't know what the origin of that synthetic event is. But Chrome or FF doesn't send that event. Do you think that might create some other compat issues that lost/gotpointercapture is delayed for an indeterminate amount on Chrome or other browsers if js sets the capture say on pointermove? I can see now the behavior of the Edge is what you say but how can we really prevent that indeterminate delay?

@teddink

This comment has been minimized.

Show comment
Hide comment
@teddink

teddink Jun 14, 2016

Regarding the erroneous pointermove with touch - the tracking bug for Edge is here: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/6866566/

teddink commented Jun 14, 2016

Regarding the erroneous pointermove with touch - the tracking bug for Edge is here: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/6866566/

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Jun 14, 2016

Member

@teddink how about the proposed solution? Did you guys agree with the way it is phrased? Do you see any edge cases that it does not cover?

Member

NavidZ commented Jun 14, 2016

@teddink how about the proposed solution? Did you guys agree with the way it is phrased? Do you see any edge cases that it does not cover?

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Jul 6, 2016

Contributor

Discussed in the call today. We'll land this PR (with any minor wording / editorial changes as needed), and Navid will file a new issue tracking the (now smaller) outstanding difference between Edge behavior and the spec. There are a bunch of subtle details here that we'll continue to iterate on, but this is a step in the right direction (perhaps the minimum change necessary for Chrome to ship a first version?).

Contributor

RByers commented Jul 6, 2016

Discussed in the call today. We'll land this PR (with any minor wording / editorial changes as needed), and Navid will file a new issue tracking the (now smaller) outstanding difference between Edge behavior and the spec. There are a bunch of subtle details here that we'll continue to iterate on, but this is a step in the right direction (perhaps the minimum change necessary for Chrome to ship a first version?).

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Jul 13, 2016

Contributor

In the call today @NavidZ raised a good question: what should the PointerEvent properties be for these events? Using the values from the previous pointer event could make sense, but would be problematic - eg. the client/page co-ordinates may be inconsistent with the current scroll offset (which may have changed since the last event was generated). Options I see:

  1. All pointer properties other than pointerType and pointerId always have their default value for gotpointercapture and lostpointercapture. This is simplest and easiest to explain, I'm not sure there's any scenario where you really want co-ordinates in one such listener. But this might be breaking or unnecessarily strict.
  2. got/lostpointercapture events generated as a direct result of another pointer event inherit the pointer properties from it. Otherwise the properties have their default values.
  3. Most pointer properties are copied directly from the last pointer event for that pointerType/id, but properties that reflect the DOM state (eg. page co-ordinates) are re-computed in the context of the current DOM state.

Next step is to better understand exactly what Edge is doing for these cases.

Contributor

RByers commented Jul 13, 2016

In the call today @NavidZ raised a good question: what should the PointerEvent properties be for these events? Using the values from the previous pointer event could make sense, but would be problematic - eg. the client/page co-ordinates may be inconsistent with the current scroll offset (which may have changed since the last event was generated). Options I see:

  1. All pointer properties other than pointerType and pointerId always have their default value for gotpointercapture and lostpointercapture. This is simplest and easiest to explain, I'm not sure there's any scenario where you really want co-ordinates in one such listener. But this might be breaking or unnecessarily strict.
  2. got/lostpointercapture events generated as a direct result of another pointer event inherit the pointer properties from it. Otherwise the properties have their default values.
  3. Most pointer properties are copied directly from the last pointer event for that pointerType/id, but properties that reflect the DOM state (eg. page co-ordinates) are re-computed in the context of the current DOM state.

Next step is to better understand exactly what Edge is doing for these cases.

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Jul 14, 2016

Member

Regarding the attributes of lost/gotpointercapture and the behavior of Edge here is what I found:
lostpointercapture attributes are always set to default except the pointerId. Even an attribute like primary is set to false for a mouse device.

gotpointercapture is always set to the pointerdown attributes if it was fired right after pointerdown.
If it is fired right before the synthetic pointermove both the synthetic poinermove and gotpointercapture attributes are set to the last pointerevent attributes except the button which is always set to -1 which makes sense as the last pointerevent might have happened as a result of a button press.

So I assume in the second scenario which Edge sends a synthetic pointermove it does indeed cache the last pointerevent.

Member

NavidZ commented Jul 14, 2016

Regarding the attributes of lost/gotpointercapture and the behavior of Edge here is what I found:
lostpointercapture attributes are always set to default except the pointerId. Even an attribute like primary is set to false for a mouse device.

gotpointercapture is always set to the pointerdown attributes if it was fired right after pointerdown.
If it is fired right before the synthetic pointermove both the synthetic poinermove and gotpointercapture attributes are set to the last pointerevent attributes except the button which is always set to -1 which makes sense as the last pointerevent might have happened as a result of a button press.

So I assume in the second scenario which Edge sends a synthetic pointermove it does indeed cache the last pointerevent.

@mustaqahmed mustaqahmed removed the question label Jul 21, 2016

@mustaqahmed mustaqahmed changed the title from Spec implies lost/gotpointercapture is delayed until the next pointer event but Edge does otherwise to Not delaying lost/gotpointercapture until the next pointer event Jul 21, 2016

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Jul 27, 2016

Contributor

Discussed this at the hackathon and Alexander (Edge dev) said that he'd prefer they just try to rip out their "bug fix" that caused immediate gotpointercapture. The principle here is that it doesn't matter who has capture until another input event arrives, so the simplest design would be to just always delay. Although we'd probably still want to say that lostpointercapture fires immediately in the implicit release scenario. The Edge team definitely considers it a bug that isPrimary is not correct.

@NavidZ what do you think?

Contributor

RByers commented Jul 27, 2016

Discussed this at the hackathon and Alexander (Edge dev) said that he'd prefer they just try to rip out their "bug fix" that caused immediate gotpointercapture. The principle here is that it doesn't matter who has capture until another input event arrives, so the simplest design would be to just always delay. Although we'd probably still want to say that lostpointercapture fires immediately in the implicit release scenario. The Edge team definitely considers it a bug that isPrimary is not correct.

@NavidZ what do you think?

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Jul 28, 2016

Member

That sounds perfect to me. As I told you before having the delay all the time is the cleanest solution. As long as Edge folks agree that developers may not expect the immediate signal for these two and particularly that special case in Edge we should be good. So I guess there are a few tests (at least their description) we need to update to reflect this.

Member

NavidZ commented Jul 28, 2016

That sounds perfect to me. As I told you before having the delay all the time is the cleanest solution. As long as Edge folks agree that developers may not expect the immediate signal for these two and particularly that special case in Edge we should be good. So I guess there are a few tests (at least their description) we need to update to reflect this.

@RByers

This comment has been minimized.

Show comment
Hide comment
@RByers

RByers Jul 28, 2016

Contributor

Thanks. Yep, we need to add the language for the pointerup/pointercancel case to fire lostpointercapture immediately, and update some tests. Sorry that I was arguing firing immediately was better, Alex convinced me there really should not be any need. They dug some more into the bug that caused them to do that for pointerdown and believe it shouldn't apply anymore (probably was some WinJS thing).

Contributor

RByers commented Jul 28, 2016

Thanks. Yep, we need to add the language for the pointerup/pointercancel case to fire lostpointercapture immediately, and update some tests. Sorry that I was arguing firing immediately was better, Alex convinced me there really should not be any need. They dug some more into the bug that caused them to do that for pointerdown and believe it shouldn't apply anymore (probably was some WinJS thing).

@RByers RByers changed the title from Not delaying lost/gotpointercapture until the next pointer event to When are lost/gotpointercapture delayed until the next pointer event? Jul 28, 2016

@teddink

This comment has been minimized.

Show comment
Hide comment
@teddink

teddink commented Jul 28, 2016

I added the following bug for tracking this in Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/8315531/

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Jul 29, 2016

Revert "Send got/lostpointercapture immediately if possible"
This reverts commit 7d314cf as a follow up to the decision about delayed capturing
w3c/pointerevents#32

This also gets rid of the need to make PointerEventManager a GC object.

BUG=627192, 629935

Review-Url: https://codereview.chromium.org/2193473002
Cr-Commit-Position: refs/heads/master@{#408714}
@mingchou

This comment has been minimized.

Show comment
Hide comment
@mingchou

mingchou Aug 5, 2016

In the case that captures pointers when pointerdown, releases pointer capture when receiving gotpointercapture, and then user click the element, the expected event sequence shall be
pointerdown
gotpointercapture
pointerup
lostpointercapture
Is my understanding correct?

mingchou commented Aug 5, 2016

In the case that captures pointers when pointerdown, releases pointer capture when receiving gotpointercapture, and then user click the element, the expected event sequence shall be
pointerdown
gotpointercapture
pointerup
lostpointercapture
Is my understanding correct?

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Aug 5, 2016

Member

Yes. We also have a test for this scenario
https://github.com/w3c/web-platform-tests/blob/master/pointerevents/pointerevent_lostpointercapture_is_first-manual.html

however, the test currently matches the old expectation and we need to update it to match what you said.

Member

NavidZ commented Aug 5, 2016

Yes. We also have a test for this scenario
https://github.com/w3c/web-platform-tests/blob/master/pointerevents/pointerevent_lostpointercapture_is_first-manual.html

however, the test currently matches the old expectation and we need to update it to match what you said.

@smaug----

This comment has been minimized.

Show comment
Hide comment
@smaug----

smaug---- Aug 9, 2016

What should happen if in a pointerdown listener one first sets pointercapture to the element foo and then removes foo from document? Should got/lostpointercapture be dispatched and where and when?
Are got/lost dispatched when there is next pointermove or pointerup for that particular pointer?

smaug---- commented Aug 9, 2016

What should happen if in a pointerdown listener one first sets pointercapture to the element foo and then removes foo from document? Should got/lostpointercapture be dispatched and where and when?
Are got/lost dispatched when there is next pointermove or pointerup for that particular pointer?

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Aug 9, 2016

Member

I have a feeling that no one should get anything (either got or lostpointercapture) based on the current spec. Do you think it is an acceptable behavior?

Member

NavidZ commented Aug 9, 2016

I have a feeling that no one should get anything (either got or lostpointercapture) based on the current spec. Do you think it is an acceptable behavior?

@teddink

This comment has been minimized.

Show comment
Hide comment
@teddink

teddink Aug 9, 2016

I agree with Navid - if the element is no longer in the DOM, then the got/lostpointercapture events should get dropped. I guess we could fire them at the next ancestor of the removed node, but that seems odd.

Is there a scenario that we would be breaking by not firing these events in that situation?

teddink commented Aug 9, 2016

I agree with Navid - if the element is no longer in the DOM, then the got/lostpointercapture events should get dropped. I guess we could fire them at the next ancestor of the removed node, but that seems odd.

Is there a scenario that we would be breaking by not firing these events in that situation?

@NavidZ

This comment has been minimized.

Show comment
Hide comment
@NavidZ

NavidZ Oct 20, 2016

Member

I updated the test in this PR. As everybody agreed on delaying got/lostpointercapture all the time I close this issue.

Member

NavidZ commented Oct 20, 2016

I updated the test in this PR. As everybody agreed on delaying got/lostpointercapture all the time I close this issue.

@NavidZ NavidZ closed this Oct 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment