Closed Bug 1821733 Opened 1 year ago Closed 10 months ago

Wheel transactions should not outlive the action chain

Categories

(Remote Protocol :: Agent, task, P1)

task

Tracking

(firefox115 fixed)

RESOLVED FIXED
115 Branch
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: nobody → drobertson
Depends on: 1168182

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!

Component: WebDriver BiDi → Agent
Whiteboard: [webdriver:triage]

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.

Whiteboard: [webdriver:triage]

(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.

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.

(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.

Summary: Wheel transactions should end on ReleaseAction → Wheel transactions should not outlive the action chain
Blocks: 1823700
Priority: -- → P2
Whiteboard: [webdriver:backlog]
Severity: -- → S3

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.

Flags: needinfo?(drobertson)

Given that this should be a priority moving the bug into the M7 milestone.

Whiteboard: [webdriver:backlog] → [webdriver:m7]

(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.

Flags: needinfo?(drobertson)

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.

Ensure that a wheel event group does not live longer than the action
chain, by calling EndTransaction on performActions().

Depends on D177923

Thanks for the WiP patches. That looks reasonable to me. So lets see what platform folks will respond.

Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [webdriver:m7] → [webdriver:m7][webdriver:external]
Attachment #9333371 - Attachment description: WIP: Bug 1821733 - Add chrome-only method for ending a wheel event group. r=smaug,emilio,whimboo → Bug 1821733 - Add chrome-only method for ending a wheel event group. r=smaug,emilio,whimboo
Attachment #9333372 - Attachment description: WIP: Bug 1821733 - End the current wheel event group on perform action. r=whimboo,jgraham → Bug 1821733 - End the current wheel event group on perform action. r=whimboo,jgraham
Pushed by drobertson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7fb13d96f26e
Add chrome-only method for ending a wheel event group. r=smaug
https://hg.mozilla.org/integration/autoland/rev/2d1665dc41ba
End the current wheel event group on perform action. r=webdriver-reviewers,whimboo
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 115 Branch
Whiteboard: [webdriver:m7][webdriver:external] → [webdriver:m7][webdriver:external][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: