Wheel transactions should not outlive the action chain
Categories
(Remote Protocol :: Agent, task, P1)
Tracking
(firefox115 fixed)
Tracking | Status | |
---|---|---|
firefox115 | --- | fixed |
People
(Reporter: dlrobertson, Assigned: dlrobertson)
References
Details
(Whiteboard: [webdriver:m7][webdriver:external][webdriver:relnote])
Attachments
(2 files)
Summary
After bug 1168182, wheel events are bound to a transaction, which may result in surprising behavior for users of the webdriver.
Discussion
Assuming dom.event.wheel-event-groups.enabled=true
, the behavior is intermittently observed with these tests. The first test (test_wheel_scroll_overflow
) will create a wheel event group that subsequent wheel events will be bound to. There is no mouse event or other user interaction between test cases here and we are not guaranteed to exceed the mousewheel.transaction.timeout. This causes the wheel events for the following tests (test_wheel_scroll_iframe1
) to still be bound to the event target of the prior test.
The average user will not see this, since we'd expect some form of mousemove
when the user intends to send wheel events to another target, but in the webdriver case we cannot rely on users to do the same.
Solution?
This seems like a great candidate for WebDriver:ReleaseAction
. It would make sense for all wheel events in an action that occur before a performActions
call to belong to the same group. This is currently used to test wheel event groups in web-platform-tests/wpt#37445. It is currently unclear to me how we could do this. Feedback on this would be greatly appreciated :-)
Assignee | ||
Updated•2 years ago
|
Comment 1•2 years ago
|
||
As discussed on Matrix we will turn off this feature temporarily for WebDriver until this bug is fixed. Therefor dom.event.wheel-event-groups.enabled
will be added to the recommended preferences of the Remote Agent.
Given that this will soon affect a shared library between WebDriver classic and BiDi I'm going ahead and moving this bug to the Remote Agent. We will also discuss next week in our triage meeting how to best handle it.
In the meantime you could maybe give us a JavaScript excerpt of what would have to be done to end a wheel transaction? That would be helpful. Thanks!
Comment 2•2 years ago
|
||
I don't think it makes sense to bind this to releaseActions
; that's really about resetting state, not committing a change.
At the very least the transaction needs to end at the end of an action chain. But the WebDriver spec actually requires that the DOM events are dispatched before the end of a "tick" (which is a WebDriver actions concept, not related to the event loop, or graphics refresh, or any other context which uses the term tick :) ). So I think that we should end the transaction before the end of each tick.
Assignee | ||
Comment 3•2 years ago
|
||
(In reply to James Graham [:jgraham] from comment #2)
At the very least the transaction needs to end at the end of an action chain. But the WebDriver spec actually requires that the DOM events are dispatched before the end of a "tick" (which is a WebDriver actions concept, not related to the event loop, or graphics refresh, or any other context which uses the term tick :) ). So I think that we should end the transaction before the end of each tick.
When does a "tick" occur? In web-platform-tests/wpt#37445, we use a series of wheel actions to test wheel transactions. Would the tests still work if the transaction is ended on a "tick"? Tests like this would still work if the transaction were to be cancelled at the end of an action chain.
Comment 4•2 years ago
|
||
Yeah, so each scroll
there corresponds to a seperate tick. I don't quite know how standard wheel event transactions are, and it might indeed be hard to test with WebDriver, but at the moment the WebDriver spec says (informatively) "The next tick begins after the user agent has had a chance to process all DOM events generated in the current tick.", and normatively says:
Wait until the following conditions are all met:
- There are no pending asynchronous waits arising from the last invocation of the dispatch tick actions steps.
- The user agent event loop has spun enough times to process the DOM events generated by the last invocation of the dispatch tick actions steps.
- At least tick duration milliseconds have passed.
So although I agree there's room for interpretation, it seems reasonable to assume that before we move on to the next tick we should have dispatched all the events for the current tick, not be buffering them somewhere.
But if the events are being dispatched, but the event target isn't being updated, I don't know how to reason about that in webdriver terms. In that case maybe ending the transaction at the end of the event chain makes more sense than ending it per tick, since conceptually event event chain is a different user interaction.
Assignee | ||
Comment 5•2 years ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #1)
[...]
In the meantime you could maybe give us a JavaScript excerpt of what would have to be done to end a wheel transaction? That would be helpful.
Something like this would end a wheel transaction.
(In reply to James Graham [:jgraham] from comment #4)
[...]
In that case maybe ending the transaction at the end of the event chain makes more sense than ending it per tick, since conceptually event event chain is a different user interaction.
This makes the most sense to me, but I'm still quite new to things.
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 6•1 years ago
|
||
Hi Dan, and sorry for the delay here. I hope that we can continue on this bug soon. For now let me ask one more question below...
(In reply to Dan Robertson (:dlrobertson) from comment #5)
(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #1)
[...]
In the meantime you could maybe give us a JavaScript excerpt of what would have to be done to end a wheel transaction? That would be helpful.Something like this would end a wheel transaction.
Well, that is the pytest fixture for our WebDriver tests. My question was actually more targeted to what needs to be done on the platform side to end a wheel transaction? What code needs to be called from our side. Doing it at the end of performAction
seems to be possible solution as I get when reading the above text.
Also would you be interested to work on this particular bug once it's clear what needs to be done? Thanks.
Comment 7•1 years ago
|
||
Given that this should be a priority moving the bug into the M7 milestone.
Assignee | ||
Comment 8•1 years ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #6)
Hi Dan, and sorry for the delay here. I hope that we can continue on this bug soon. For now let me ask one more question below...
No problem! You've been extremely helpful so far.
(In reply to Dan Robertson (:dlrobertson) from comment #5)
(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #1)
[...]
In the meantime you could maybe give us a JavaScript excerpt of what would have to be done to end a wheel transaction? That would be helpful.Something like this would end a wheel transaction.
Well, that is the pytest fixture for our WebDriver tests. My question was actually more targeted to what needs to be done on the platform side to end a wheel transaction? What code needs to be called from our side. Doing it at the end of
performAction
seems to be possible solution as I get when reading the above text.Also would you be interested to work on this particular bug once it's clear what needs to be done? Thanks.
I'll post WIP patch that I think could fix this. I have not interacted much (if at all) with the browser chrome and the webdriver, so feedback would be greatly appreciated! Also thoughts and feedback on the best way to test this would be appreciated.
Assignee | ||
Comment 9•1 years ago
|
||
Add a chrome-only method for ending a wheel event group. This can then
be used by the webdriver to ensure that the wheel event group does not
live longer than the action chain.
Assignee | ||
Comment 10•1 years ago
|
||
Ensure that a wheel event group does not live longer than the action
chain, by calling EndTransaction on performActions().
Depends on D177923
Comment 11•1 years ago
|
||
Thanks for the WiP patches. That looks reasonable to me. So lets see what platform folks will respond.
Updated•1 years ago
|
Updated•1 years ago
|
Updated•1 years ago
|
Comment 12•1 years ago
|
||
Comment 13•1 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7fb13d96f26e
https://hg.mozilla.org/mozilla-central/rev/2d1665dc41ba
Updated•1 year ago
|
Description
•