web-platform-tests
Add test for wheel event groups
| Test | Blink | WebKit | Gecko |
|---|---|---|---|
| Wheel events are grouped and are not sent to a child element | :white_check_mark: | :white_check_mark: | :x: |
| Wheel events are not bound to a scroll frame | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| Wheel event groups beyond the origin bound have the same target | :white_check_mark: | :x: | :x: |
As stated in https://github.com/w3c/uievents/issues/338 the WebKit failure in my local tests is due to a webdriver exception, but follows the expected behavior with manual tests.
I also wanted to add a test for a moved element, but that turns out to be a bit more complicated and the tests get super flaky, so I'm not really sure what the correct behavior is.
Reorganized the tests and rewrote the removal/move initial target tests to greatly improve their reliability. All tests pass very reliably for chrome and firefox (with the bug 1168182 patch) for me on the main box I'm using to work on this. I'll try to run the tests on WebKit and maybe other OSes over the next few days.
Added dom/events/scrolling/wheel-event-groups-target-elements.html which ensures the text is not the event target for the wheel event group. Blink handles this correctly, but I'm currently unable to test on WebKit. This is the last failing test (that I know of 😆) with D163484 for Gecko.
The firefox failure is expected until the fix for bug 1168182 lands (D163484).
This bug has been fixed two months ago. I've re-triggered the tests so hopefully they should all pass now. @dlrobertson is there anything else this PR would need?
This bug has been fixed two months ago. I've re-triggered the tests so hopefully they should all pass now.
Thanks for re-triggering the tests! Just re-ran these tests locally on chrome beta and m-c (with and without the bug 1821733 patchset). Tests pass reliably for me, but I'd be interested if others see the same results.
@dlrobertson is there anything else this PR would need?
I think the tests accurately describe browsers current behavior and are stable in my local testing. @masayuki-nakano is there anything else you think this PR needs?
Added dom/events/scrolling/wheel-event-groups-multiple-action-chains.html. It is largely focused on testing that the webdriver functions like we'd hope. Tested on both chrome and mozilla-central (fails on current m-c and pasess with bug 1821733 patches). If this test should exist somewhere else, I'm happy to move it... it has enough in common with the other wheel-event-group-* tests that I placed it with the rest of them.
@dlrobertson thanks for adding the test! It would be good if you could retrigger the CI checks (at least those for Firefox) once the fix from bug 1821733 made it into the next Nightly build.
@dlrobertson thanks for adding the test! It would be good if you could retrigger the CI checks (at least those for Firefox) once the fix from bug 1821733 made it into the next Nightly build.
@whimboo good point. Re-triggered the CI checks and the check wpt-firefox-nightly-results seems to pass now.
@smaug---- or @masayuki-nakano PTAL, I'd like to get more testing coverage for this, since this behavior is now the default in firefox (riding the trains with bug 1823700)). If this doesn't look like it is ready to be merged soon-ish, I can pull this into the mozilla specific wpts for now.
I think I'd still prefer to call this thing something else than "event group". It is more like a wheel transaction or some such.
And sorry being late here. Getting the review through bugzilla would be faster in my case, even for public wpts (not only because the reviewing tools we use are quite a bit nicer than github).
But I'll take a look here.
And sorry being late here. Getting the review through bugzilla would be faster in my case, even for public wpts (not only because the reviewing tools we use are quite a bit nicer than github).
No problem, thanks for the feedback!
@smaug---- as it looks like this PR is waiting for another round of review. Could you please have a look?
We might want to put all the filed under tentative/ just because there is no spec for this. But other than that looks good
Renamed all tests to <test-name>.tentative.html. Good suggestion
Hi @dlrobertson. Do you plan to update this PR? It might be nice to have those tests for the upcoming work for widget level events in Marionette. Thanks.
Re tentative, now there is a spec since https://github.com/w3c/uievents/issues/338 was merged.
Re tentative, now there is a spec since w3c/uievents#338 was merged.
:+1: Thanks for the note about this. Forgot that I'd need to update this.
Also it looks like chrome no longer passes the tests reliably. I'll have to dig into the tests to figure out what changed.
@zcorpan Actually to what extent should I rename things? For example, AFAIK it isn't explicitly specified what should happen when the initial target of the wheel transaction is removed, but I have a test for that in this PR.
Keep tentative for any tests where there's no specified correct behavior, and file a spec issue. Bonus points for pointing to the spec issue in the test (with <link rel=help>).
@masayuki-nakano could you please take another look. It looks like that after the last push it was missed to request review again. Thanks.
@dlrobertson I assume that this PR doesn't need another review, so what is the merge blocked on?
Merged! Just wasn't sure if I was supposed to merge it, or if I was supposed to wait for a reviewer to merge it. Thanks for the ping!