8000 webdriver: Get the window position as well as the size when resolving "Get Window Rect" by yezhizhen · Pull Request #37812 · servo/servo · GitHub
[go: up one dir, main page]
Skip to content

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

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
Jul 2, 2025
Merged

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

merged 3 commits into from
Jul 2, 2025

Conversation

yezhizhen
Copy link
Member
  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 <yezhizhenjiakang@gmail.com>
@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 mrobinson, jdm and xiaochengh July 1, 2025 16:36
yezhizhen
yezhizhen commented Jul 2, 2025
View reviewed changes
Copy link
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
Use outer_width instead and simplify code
0e48f3a 
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@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
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 <yezhizhenjiakang@gmail.com>
@yezhizhen
Copy link
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
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
0