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

mousemove_prevent_default_action.tentative.html unnecessarily listens for selectionchange events when asserting dragstart is fired #576

Closed
aprotyas opened this issue Oct 10, 2023 · 6 comments
Labels
focus area: Pointer and Mouse Events test-change-proposal Proposal to add or remove tests for an interop area

Comments

@aprotyas
Copy link
Member

aprotyas commented Oct 10, 2023

Test List

https://wpt.fyi/results/uievents/mouse/mousemove_prevent_default_action.tentative.html

Rationale

The second subtest in the mousemove_prevent_default_action.tentative.html WPT asserts that a cancelled mousemove event fires a dragstart, which is a fine expectation on its own. Unfortunately, since this behavior is asserted by comparing the logged events (obtained from respective event listeners) against a static set of expected events (["mousemove", "mousedown", "mousemove", "dragstart"]), the test may listen to selectionchange events and include any selectionchange events in the test assertion, producing an unnecessary test failure.

We should not care whether or not a selectionchange event was fired in this case, since we only want to assert that a dragstart event fires from a cancelled mousemove event on a draggable element. These two events are not mutually exclusive in any manner relevant to this WPT.

In fact, the same argument also applies for not listening to dragstart events when we only want to assert that a selectionchange event was fired.


We should only be listening for selectionchange events, and not also dragstart events, in the first subtest. Similarly, we should only be listening for dragstart events, and not also selectionchange events, in the second subtest.

@aprotyas aprotyas added test-change-proposal Proposal to add or remove tests for an interop area focus area: Pointer and Mouse Events labels Oct 10, 2023
@aprotyas
Copy link
Member Author

cc @flackr @foolip for Chromium input.

cc @EdgarChen @smaug---- for Mozilla input.

@aprotyas
Copy link
Member Author

I've implemented the test change proposal in web-platform-tests/wpt#42471. Hopefully that makes it easier to review!

@smaug----
Copy link

The test was initially added to explicitly test selectionchange.
See also w3c/uievents#278

@smaug----
Copy link

Oh, the pr is just moving where selectionchange listener is registered. Yeah, I think that change is fine.

@aprotyas
Copy link
Member Author

@smaug---- yes, testing for selectionchange is totally fine here, I'm proposing we don't keep the listener around for the dragstart test. Thanks!

@aprotyas
Copy link
Member Author

Based on the comments here and on the PR, I think there's consensus that we want to make this test change. I'll merge web-platform-tests/wpt#42471 to reflect that shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus area: Pointer and Mouse Events test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

No branches or pull requests

2 participants