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
Added a test for event sequence after implicit release. #4364
Conversation
Notifying @EvgenyAgafonchikov, @NavidZ, @RByers, @Steditor, @bethge, @jacobrossi, @plehegar, @scottgonzalez, and @staktrace. (Learn how reviewing works.) |
} else if (event.type != "pointermove" && start_logging) { | ||
event_log.push(event.type); | ||
if (event.type == "pointercancel") | ||
seen_pointercancel = true; |
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.
Why should we expect a pointercancel while the touch-action is none?
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.
It's touch-action:auto, to test implicit release caused by a pointercancel.
<script type="text/javascript" src="pointerevent_support.js"></script> | ||
<script type="text/javascript"> | ||
var test_pointerEvent_0 = async_test("Event sequence at implicit release after click"); | ||
var test_pointerEvent_1 = async_test("Event sequence at implicit release after drag"); |
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.
Any reason we put these two tests in the same file? I mean they don't seem to share the instructions either.
Also is it possible to use setup_pointerevent_test if we expect multiple pointers to be used for this test?
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.
Done splitting into two. The second (drag) one is touch-only.
|
||
function end_of_current_test() { | ||
if (test_num == 0) { | ||
var expected_events = "pointerup, lostpointercapture, pointerout, pointerleave"; |
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 doesn't seem right for the mouse case. If we just click in the black why do we get pointerout/leave for mouse?
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.
We get out/leave right before clicking the green.
ptal. |
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.
lgtm
|
||
on_event(document.getElementById("done"), "click", function() { | ||
var expected_events = "pointerup, lostpointercapture, pointerout, pointerleave"; | ||
test(function () { |
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: We could have done the test_pointer_event.step(function() ...) as this is the only thing that we test here. We can do that to reduce one line in the output as the async test doesn't have any other assert and always passes.
But we also need to add the step function to the MultiPointerTypeTest to call step function on the current active test.
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.
Does it look better? Or you prefer hiding currentTest from the test?
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.
I would have preferred defining the step function for MultiPointerTypeTest in pointerevent_support.js so that we could have just changed the test_pointer_event with a normal async test and everything should just work. Basically I tried to hide the fact that there are multiple tests from the test itself except in a few places.
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.
Here it is, ptal.
Fixes w3c/pointerevents#142.