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

The behavior of getCoalescedEvent in pointerevent_constructor.html is inconsistent with the spec #229

Open
mingchou opened this issue Nov 21, 2017 · 11 comments

Comments

Projects
None yet
4 participants
@mingchou
Copy link

commented Nov 21, 2017

Spec says 'When the event is created by the user agent the following attributes of the coalesced events will always have the same value as the dispatched event'

The test case pointerevent_constructor.html dispatches a JS created event and expects that some attributes (e.g. target, pointerId, pointerType, isPrimary) are set up as the same as the dispatched event. Looks like the test expects that these attributes are set up for all events no matter whether they are created by the UA or JS.

Wondering whether the behavior of the spec or the test case is expected?

@NavidZ

This comment has been minimized.

Copy link
Member

commented Nov 21, 2017

No. The test is creating those coalesced events with the same dictionary. With the exception of target the rest of the attributes are being set of the same dictionary and hence are the same. However, I remember @smaug---- mentioning about the target and I did tell him that in Chrome we do set the target of all the events including the target. To me target is a little different as js doesn't set it explicitly. They just fire the event to a target and user agent sets the target attribute as well as other related attributes affected by the target like offsetX/Y of the events. So we extend this model to include coalesced events as well. Would it make sense? Do you have any suggestion on the spec to make it clearer if you believe this behavior is reasonable?

@smaug----

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2017

I don't understand how the model Chrome has works if there is pointer event (A) being dispatched, then that pointer event if passed to a constructor of another pointer event (B). Then B is dispatched. Do you end up changing the target of A too, even though that is already being dispatched?

@NavidZ

This comment has been minimized.

Copy link
Member

commented Nov 21, 2017

You are right @smaug---- . We do that.

@smaug----

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2017

You override .target of an event which is being dispatched? That sounds... weird.
I know, edge case, but still weird.

@NavidZ

This comment has been minimized.

Copy link
Member

commented Nov 21, 2017

I agree with you. It does. We didn't quite have this scenario in mind at the time. One solution to this would be us getting a dictionary for the events instead of actual pointerevents in the constructor for the coalesced events. This way developers can never pass the same dispatched events. Does that make it better?

@smaug----

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2017

Or perhaps .target just shouldn't be changed if it already has non-null value? Or perhaps pointerevents passed in the ctor shouldn't have non-null target?

@NavidZ

This comment has been minimized.

Copy link
Member

commented Nov 21, 2017

We can do the first one. If we were to go with the second option how do we enforce it? like throwing an exception or something when the given pointerevents had some non-null target?

@smaug----

This comment has been minimized.

Copy link
Contributor

commented Nov 21, 2017

yeah, ctor would throw.

@NavidZ

This comment has been minimized.

Copy link
Member

commented Nov 21, 2017

I cannot imagine any downside for either of these solutions. The first one might be easier and more silent and I'd be happy to fix the Chrome implementation whichever we choose. Anyone else has any comments about this? @RByers @dtapuska @mustaqahmed

@mingchou

This comment has been minimized.

Copy link
Author

commented Dec 7, 2017

Both solutions are good to me. I'm going to implement the getCoalescedEvents API with the first solution for Firefox. Would be happy to change it if we choose the other one.

@NavidZ

This comment has been minimized.

Copy link
Member

commented Dec 8, 2017

Sounds good. I'll change Chrome implementation as well to match.

@patrickhlauke patrickhlauke added the test label Feb 9, 2018

@NavidZ NavidZ added the v3 label Mar 21, 2018

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