8000 Synchronize `WebdriverScriptEvent` with pending input events by yezhizhen · Pull Request #36830 · servo/servo · GitHub
[go: up one dir, main page]
Skip to content

Synchronize WebdriverScriptEvent with pending input events #36830

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Synchronize WebdriverScriptEvent with pending input events #36830

wants to merge 1 commit into from

Conversation

yezhizhen
Copy link
Member
@yezhizhen yezhizhen commented May 4, 2025 •
edited
Loading

Follow up of #36714

  1. ScriptThread delays the handling of input event to the next update_the_rendering. This PR try to make the routes of execute_script same as input events.
  2. Resolve following assertion failure
    assertion failed: !GetCurrentRealmOrNull(*cx).is_null() (thread Script(1,2), at components\script_bindings\realms.rs:25)

Testing: tests\wpt\tests\webdriver\tests\classic\element_click\events.py
Fixes: #36676 probably

cc @longvatrong111 @jdm @xiaochengh @PotatoCP

@yezhizhen
Copy link
Member Author

I have many doubts. If we handle pending_webdriver_script_events in update_the_rendering, it does not make sense, as the JavaScript may alter the DOM and we need to update_the_rendering again..

Also, I still run into the following problem with this PR
#36676 (comment)

@yezhizhen @longvatrong111
Synchronize WebdriverScriptEvent with pending input events
569c5c7 
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
Co-authored-by: batu_hoang <longvatrong111@gmail.com>
@yezhizhen yezhizhen force-pushed the fix-script-webdriver-order branch from 2132768 to 569c5c7 Compare May 6, 2025 01:58
@longvatrong111
Copy link
Contributor
longvatrong111 commented May 6, 2025 •
edited
Loading

I have many doubts. If we handle pending_webdriver_script_events in update_the_rendering, it does not make sense, as the JavaScript may alter the DOM and we need to update_the_rendering again..

Also, I still run into the following problem with this PR #36676 (comment)

Yes I think we can find another solution, as long as script_events, which are supposed to querry the result of input_events should be handled after input_events.

About this issuse, it's a newly separated issue of execute script.

@yezhizhen
Copy link
Member Author
yezhizhen commented May 6, 2025 •
edited
Loading

It seems the reason I only get "mousemove, mousedown, mouseup" but never the last "mouseclick" is because the wrong button is sent by webdriver.. Middle button is sent instead of left.

https://www.w3.org/TR/pointerevents/#the-button-property

servo/components/webdriver_server/lib.rs

Lines 1631 to 1641 in 8b05b74

// Steps 8.7 - 8.8
let pointer_down_action = PointerDownAction {
button: 1,
..Default::default()
};
// Steps 8.9 - 8.10
let pointer_up_action = PointerUpAction {
button: 1,
..Default::default()
};

@longvatrong111 is going to fix this one :)

@xiaochengh
Copy link
Contributor

I don't this this will work. If web driver fires interleaving input and script events, this approach still doesn't produce the correct handling order.

IIUC the goal is to fix the WebDriver conformance tests, which are written in python. Is it possible to make the input event calls wait for the event completion?

Btw, script events updating DOM might be fine, since input events on their own can also have listeners that update the DOM.

@longvatrong111
Copy link
Contributor
longvatrong111 commented May 6, 2025 •
edited
Loading

I don't this this will work. If web driver fires interleaving input and script events, this approach still doesn't produce the correct handling order.

IIUC the goal is to fix the WebDriver conformance tests, which are written in python. Is it possible to make the input event calls wait for the event completion?

Btw, script events updating DOM might be fine, since input events on their own can also have listeners that update the DOM.

We can let input event calls wait, but we need start with webdriver_server in Rust first.
Currently in webdriver thread, it only sends events to Servo then immediately return Ok (for command like ElementClick) to python client.

I think we have some options:

  1. Make all commands relating to input wait for result from script thread.

  2. Add synchronization for couples like input - script.

  3. We can make an option for webdriver server command to be sync or async, and let python client choose.

I'm not sure which is most fit with the standard. This's my previous concern.

@yezhizhen
Copy link
Member Author

I think we have some options:

  1. Make all commands relating to input wait for result from script thread.

I considered this option as the ultimate solution before.. But it seems not possible according to my prior discussion...
#35685 (comment)

@longvatrong111
Copy link
Contributor

Offline discussed with @xiaochengh :
The synchronization we need is defined in Webdriver spec . Just Servo implementation misses a lot of steps.

@yezhizhen yezhizhen mentioned this pull request Jun 10, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gterzian gterzian Awaiting requested review from gterzian gterzian will be requested when the pull request is marked ready for review gterzian is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

get_events (aka. allEvents.events) of WebDriver test returns wrong number of events
3 participants
@yezhizhen @longvatrong111 @xiaochengh
0