Skip to content

webdriver: Get the window position as well as the size when resolving "Get Window Rect"#37812

Merged
mrobinson merged 3 commits intoservo:mainfrom
yezhizhen:get-window-handle
Jul 2, 2025
Merged

webdriver: Get the window position as well as the size when resolving "Get Window Rect"#37812
mrobinson merged 3 commits intoservo:mainfrom
yezhizhen:get-window-handle

Conversation

@yezhizhen
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen commented Jul 1, 2025

  1. Rename GetWindowSize to GetWindowRect
  2. Return the WindowRect in device pixels correctly. Previously, it returns (0, 0, ScreenWidth, ScreenHeight) which is a static value.
  3. Add fn window_rect to WindowPortsMethods. Implement it for both Headless Window and Headed Window.

Testing: Tested manually with powershell script. Result is now dynamic and reflects the truth.
Fixes: Task 1 & 2 of #37804

@yezhizhen
Implement "GetWindowRect"
c75d0e1 
Signed-off-by: Euclid Ye <[email protected]>
@yezhizhen yezhizhen requested a review from atbrakhi as a code owner July 1, 2025 16:35
@yezhizhen yezhizhen changed the title [WebDriver] Implement "Get Window Rect" servoshell: Add fn window_rect to WindowPortsMethods to Implement "Get Window Rect" in WebDriver Jul 1, 2025
@yezhizhen yezhizhen changed the title servoshell: Add fn window_rect to WindowPortsMethods to Implement "Get Window Rect" in WebDriver servoshell: Add fn window_rect to WindowPortsMethods to Implement "Get Window Rect" in WebDriver Jul 1, 2025
@yezhizhen yezhizhen requested review from jdm, mrobinson and xiaochengh July 1, 2025 16:36
yezhizhen
yezhizhen commented Jul 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member Author

@yezhizhen yezhizhen left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also explored another possibility to reuse script handler: because we want result as close to window.ScreenY, window.OuterHeight etc. as possible.

Right now, this is done in Script like following:

servo/components/script/dom/window.rs

Lines 1662 to 1666 in a13f9bc

// https://drafts.csswg.org/cssom-view/#dom-window-screeny
fn ScreenY(&self) -> i32 {
let (_, origin) = self.client_window();
origin.y
}

It sends the request to compositor, who ignores the toolbar height etc, giving inaccurate result.

That's why I give up "reusing" and implements a new one directly in embedder. I filed a new issue below.

@yezhizhen yezhizhen mentioned this pull request Jul 2, 2025
Closed
yezhizhen
yezhizhen commented Jul 2, 2025
View reviewed changes
mrobinson
mrobinson requested changes Jul 2, 2025
View reviewed changes
@mrobinson mrobinson changed the title servoshell: Add fn window_rect to WindowPortsMethods to Implement "Get Window Rect" in WebDriver webdriver: Get the window position as well as the size when resolving "Get Window Rect" Jul 2, 2025
@yezhizhen yezhizhen requested a review from mrobinson July 2, 2025 08:48
mrobinson
mrobinson approved these changes Jul 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, with one nit:

@yezhizhen
Simplify further
fa1d5e9 
Signed-off-by: Euclid Ye <[email protected]>
@yezhizhen
Copy link
Copy Markdown
Member Author

yezhizhen commented Jul 2, 2025 •
edited
Loading

But I don't know why; outer_size and outer_position still not very accurate..

Fullscreen 2560 * 1600
@{x=-11; y=-11; width=2582; height=1622}

It seems Firefox does exactly the same as Servo. But Chrome/Edge correctly reports
@{x=0; y=0; width=2560; height=1600}

So I believe this cannot be easily fixed.. @mrobinson @jdm

EDIT: On Linux the result is perfect for Servo/Firefox as well. So it is misusage of Windows API from Winit & nsWindow I believe.🤣

@yezhizhen yezhizhen requested a review from mrobinson July 2, 2025 09:25
@mrobinson mrobinson enabled auto-merge July 2, 2025 09:27
@mrobinson mrobinson added this pull request to the merge queue Jul 2, 2025
@yezhizhen yezhizhen mentioned this pull request Jul 2, 2025
Closed
Merged via the queue into servo:main with commit 94f35ba Jul 2, 2025
21 checks passed
@yezhizhen yezhizhen deleted the get-window-handle branch July 2, 2025 11:07
@yezhizhen yezhizhen mentioned this pull request Jul 10, 2025
Merged
@yezhizhen
Copy link
Copy Markdown
Member Author

yezhizhen commented Jul 25, 2025 •
edited
Loading

But I don't know why; outer_size and outer_position still not very accurate..

Fullscreen 2560 * 1600
@{x=-11; y=-11; width=2582; height=1622}

It seems Firefox does exactly the same as Servo. But Chrome/Edge correctly reports @{x=0; y=0; width=2560; height=1600}

So I believe this cannot be easily fixed.. @mrobinson @jdm

EDIT: On Linux the result is perfect for Servo/Firefox as well. So it is misusage of Windows API from Winit & nsWindow I believe.🤣

servo/tests/wpt/tests/webdriver/tests/support/helpers.py

Lines 230 to 233 in 87bbe0b

# The maximized window can still have a border attached which would
# cause its dimensions to exceed the whole available screen.
return (dimensions["windowWidth"] >= dimensions["availWidth"] and
dimensions["windowHeight"] >= dimensions["availHeight"] and

I just saw this and believe Firefox/Servo + Chrome/Edge are all correct, with their own fashion.

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

@atbrakhi atbrakhi Awaiting requested review from atbrakhi atbrakhi is a code owner

@jdm jdm Awaiting requested review from jdm

@xiaochengh xiaochengh Awaiting requested review from xiaochengh

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