8000 webdriver: Focus when "switch to window" by yezhizhen · Pull Request #38889 · servo/servo · GitHub
[go: up one dir, main page]
Skip to content

webdriver: Focus when "switch to window" #38889

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
Merged
merged 3 commits into from
Aug 24, 2025
Merged

webdriver: Focus when "switch to window" #38889

merged 3 commits into from
Aug 24, 2025

Conversation

yezhizhen
Copy link
Member
@yezhizhen yezhizhen commented Aug 23, 2025 •
edited
Loading

Add logic that was accidentally removed in #38745. Otherwise it is very weird when using webdriver as a human, as we still stays on the original tab.

Testing: this should not affect any test. Even if the tab is not "visible" previously, all programmatic interaction works fine. Stably pass test_history_pushstate in back.py and forward.py.

@yezhizhen yezhizhen requested review from mrobinson and jdm August 23, 2025 15:57
@yezhizhen yezhizhen marked this pull request as draft August 23, 2025 16:59
@yezhizhen
Copy link
Member Author
yezhizhen commented Aug 23, 2025 •
edited
Loading

Converting to draft as I see two stably passing tests due to this change.. I wanna know why.

EDIT: We are stably passing following test related to history navigation

servo/tests/wpt/tests/webdriver/tests/classic/back/back.py

Line 112 in 0a81461

assert session.url == "{}#pushstate".format(pushstate_page)

This involves ScriptToConstellationMessage::PushHistoryState.

servo/components/constellation/constellation.rs

Lines 1740 to 1741 in 0a81461

ScriptToConstellationMessage::PushHistoryState(history_state_id, url) => {
self.handle_push_history_state_msg(source_pipeline_id, history_state_id, url);

I suspect source_pipeline_id is wrong when we didn't focus the tab where interaction happens..

@yezhizhen yezhizhen marked this pull request as ready for review August 23, 2025 17:43
@yezhizhen
Copy link
Member Author
yezhizhen commented Aug 24, 2025 •
edited
Loading

This is really interesting.

When we didn't focus the tab 2 (webview_id = TopLevel(0,2)), it seems the DocumentEventHandler receiving the input event is the visible one (TopLevel(0,1)).

servo/components/script/dom/document_event_handler.rs

Lines 65 to 67 in 1fc8578

pub(crate) struct DocumentEventHandler {
/// The [`Window`] element for this [`DocumentEventHandler`].
window: Dom<Window>,

The log says constellation indeed forward to the tab 2. However, the DocumentEventHandler handling it is for tab 1.

constellation.rs:2973:9] "Constellation forward for webview_id:" = "Constellation forward for webview_id:"
constellation.rs:2973:9] &webview_id = TopLevel(0,2)
script_thread] Handling event: FromConstellation(ConstellationControlMsg::SendInputEvent)
DEBUG script::dom::document_event_handler] TopLevel(0,1)

It is not related to this PR anyway.

@yezhizhen
focus when switch to window
0231ef2 
Signed-off-by: Euclid Ye <euclid.ye@huawei.com>
@yezhizhen
Update WPT
8257c14 
Signed-off-by: Euclid Ye <euclid.ye@huawei.com>
@yezhizhen
unstable sort window handles
eca814d 
Signed-off-by: Euclid Ye <euclid.ye@huawei.com>
@yezhizhen yezhizhen added this pull request to the merge queue Aug 24, 2025
Merged via the queue into servo:main with commit 0b5bcfb Aug 24, 2025
22 checks passed
@yezhizhen yezhizhen deleted the add-focus branch August 24, 2025 14:01
@yezhizhen
Copy link
Member Author
yezhizhen commented Aug 25, 2025 •
edited
Loading

cc @PotatoCP @longvatrong111 After this, there are many flaky TIMEOUT due to focus race condition I said before. I've been working on it.

@longvatrong111 longvatrong111 mentioned this pull request Aug 25, 2025
Closed
@yezhizhen yezhizhen mentioned this pull request Aug 25, 2025
Merged
@yezhizhen yezhizhen mentioned this pull request Sep 2, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Sep 2, 2025
@yezhizhen
webdriver: Focus browsing context when switch frame (#39086)
7FB0
802fdd9 
#38889 adds back the mechanism to focus the window when [switch to
window](https://w3c.github.io/webdriver/#switch-to-window). After that,
it causes many flaky TIMEOUT. Turns out the real reason is same as the
phenomenon which I thought was unrelated:
#38889 (comment): we
need to focus the browsing context as well according to spec. This is
important for Servo because it relies on
https://github.com/servo/servo/blob/f4dd2960b89bcc6bae3f9dae5be8964c64c1c2c7/components/constellation/constellation_webview.rs#L64
to determine which pipeline to send `InputEvent` to.

Testing: Before, there are 9 ~ 13 flaky results. [Before
1](https://github.com/yezhizhen/servo/actions/runs/17379170889), [Before
2](https://github.com/yezhizhen/servo/actions/runs/17392219417), [Before
3](https://github.com/yezhizhen/servo/actions/runs/17379172612).
Now there are 2 ~ 4 flaky results. [After
1](https://github.com/yezhizhen/servo/actions/runs/17394359570), [After
2](https://github.com/yezhizhen/servo/actions/runs/17394358218), [After
3](https://github.com/yezhizhen/servo/actions/runs/17394357400).
Fixes: #38889 (comment)
Fixes: #38906, which is last blocking point to enable WebDriver CI.

---------

Signed-off-by: Euclid Ye <euclid.ye@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrobinson mrobinson mrobinson approved these changes

@jdm jdm Awaiting requested review from jdm

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

Successfully merging this pull request may close these issues.
2 participants
@yezhizhen @mrobinson
0