1855028 - Implement "browser.setClientWindowState" command

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P3)

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The WebDriver BiDi spec is not done yet. As such we cannot implement it for M9. Lets push onto the backlog for now.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I accidentally took over the points when cloning the former bug. We need to discuss the complexity.

Comment 3

[Dan]

I know this is in not is not directly blocking implementing browser.getClientWindows, but shouldn't implementing this come first? browser.getClientWindows needs the getClientWindowState.

Comment 4

[Dan]

(In reply to Dan from comment #3)

I know this is in not is not directly blocking implementing browser.getClientWindows, but shouldn't implementing this come first? browser.getClientWindows needs the getClientWindowState.

Okay, I looked at the dependency graph again, and this issue is dependent on browser.getClientWindows. browser.getClientWindows does need to return the current state of the client window as part of its return object. So I am not sure if setting it should come first.

Comment 5

[Dan]

Attachment: Bug 1855028 - [webdriver-bidi] Implement "browser.setClientWindowState" command.

Comment 6

[Dan]

Attachment: Bug 1855028 - Migrated GeckoDriver window methods to Window Manager. r?whimboo

Comment 7

[Dan]

Attachment: Bug 1855028 - [wdspec] Implement "browser.setClientWindowState" command tests. r?whimboo

Comment 8

[Phabricator Automation]

Comment on attachment 9445393 [details]
Bug 1855028 - Migrated GeckoDriver window methods to Window Manager. r?whimboo

Revision D233020 was moved to bug 1942849. Setting attachment 9445393 [details] to obsolete.

Comment 9

[Dan]

Attachment: Bug 1855028 - [wdspec] Implement "browser.setClientWindowState" command tests. r?whimboo Differential Revision: https://phabricator.services.mozilla.com/D233065

Comment 10

[Julian Descottes [:jdescottes]]

No update for > 1 month, maybe should be moved to backlog for now.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sorry that is my fault. I didn't notice that the revision, that we are waiting for, was updated. I'll do the review.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I pushed a try build with the latest modifications but I still see quite a number of test failures across platforms:
https://treeherder.mozilla.org/jobs?repo=try&revision=84b8c7f0343094540cbd5aa7b45abcc1d69411ef

Here some things to ensure:

1. We need metadata files for Android given that all these tests cannot be run and need to be disabled for that platform
2. Transitions between states seem to be fragile - especially minimized to fullscreen seems to fail a lot
3. We probably want to run each test in a new window that we can close at the end of the test to not affect the following tests
4. We need special handling of assertions for Wayland given that we cannot move the window

Dan, are you still interested to continue working on this bug? I'll make sure to more promptly reply, and in case I miss something you can always ping me on Matrix. Please let us know. Thanks.

Comment 13

[Dan]

Hi Henrik. Yes, I am still interested in working on the bug.

Comment 14

[Julian Descottes [:jdescottes]]

Dan: I'm removing it from the milestone because we don't have to track contributions as milestone bugs, but this has no impact on the priority. Feel free to keep working on this and thanks again for your help on this bug!

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Dan, I want to check if you may have an update for us regarding this implementation. If you don't have the time please give us a note and someone can take over the remaining work. Thanks!

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Dan. I assume that you may not find the time to update this patch. Are you ok when one of us is taking over the remaining work? Thanks for the contribution so far!

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dan, I think that we can pick-up the remaining work to finally get this API landed. For that work we expect 3 points and we could do it in the next milestone starting in October.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Liam will take a look at finishing up the patches. Thanks!

Comment 19

[Liam DeBeasi]

Hey there,

I'm working on getting the tests for this passing. I'm using the work from the previous contributor as a starting point since that has already received reviews from you. Can you clarify the intent behind the stress.py tests? I'd like to better understand what part of the implementation this is testing.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Great and that's a good starting point!

Regarding the stress tests.... in the past we had issues with the stability of the window resizing methods and especially when you change between different modes and sizes side effects were shown. As such we are trying to stress-test Firefox to see that the way how we implemented the related API works as expected. The tests were already present for the WebDriver classic tests and were just ported over.

Comment 21

[Liam DeBeasi]

Attachment: Bug 1855028 - [webdriver-bidi] Add the "browser.setClientWindowState" command.

Comment 22

[Liam DeBeasi]

Attachment: Bug 1855028 - [wdspec] Add initial tests for the "browser.setClientWindowState" command.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We cannot directly transition from the minimized to the fullscreen state and need to restore the window first if minimized. There is https://github.com/w3c/webdriver-bidi/issues/1035 on file for it.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I've created https://github.com/w3c/webdriver-bidi/pull/1063 to fix the transition issue.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As discussed with Liam I'm going to fix the remaining issue so that we can get this API landed soon. For the remaining work we agreed on 2 points.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I just noticed that the WebDriver tests need to be updated so that we use Javascript to get the actual window state. Right now we only rely on the values as returned by the command, which doesn't mean it's true.

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #26)

I just noticed that the WebDriver tests need to be updated so that we use Javascript to get the actual window state. Right now we only rely on the values as returned by the command, which doesn't mean it's true.

Actually there’s no way to retrieve the actual OS-level window state via a web API, so we can’t add those checks; at least not in the public tests. However, I can add Mozilla-specific tests that obtain the window state via chrome JS.

Otherwise I noticed a discrepancy in the current BiDi spec between OS-level fullscreen and document fullscreen. Right now, BiDi is using document fullscreen, which is not equivalent to the OS-level fullscreen window state. Since this command is intended to modify the OS-level window state, I’m not sure why the document fullscreen API was chosen. It seems like this should be changed.

Lets get this discussed in our next triage meeting.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As discussed in the meeting today there is not a clear concept of a OS-level window between different operating systems and browsers. So it should be fine to use window.fullScreen = true to activate the fullscreen mode, a``nd win.fullScreen = false to restore the normal state.

If we get any complain that something is not working as expected we can see in how we can update the specification.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1855028 - [wdspec] Set initial window position to (200,100) to account for possible OS docks and menus.

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I can actually see issues on Wayland with the newly added minimization tests. Under that config a request for window.minimize() I can see that the window gets minimized but the window state stays at the former window state. Also waiting for a sizemodechange event doesn't seem to be successful and we timeout after 5s.

Martin, are you aware of issues with minimizing a window on Wayland? Do you have an idea why no sizemodechange event is emitted? If that's expected we might not have to wait for it, but then the window state should be correct.

Comment 31

[Martin Stránský [:stransky] (ni? me)]

Hello Henrik, Wayland client generally can't manage its state for security reasons - for instance hide itself and run on background. So there's perhaps a bug in event processing or so but we can't expect client itself can control its state (window position & so). AFAIK only exception is fullscreen state which can be set by application itself.

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #31)

Hello Henrik, Wayland client generally can't manage its state for security reasons - for instance hide itself and run on background. So there's perhaps a bug in event processing or so but we can't expect client itself can control its state (window position & so). AFAIK only exception is fullscreen state which can be set by application itself.

Hm, but when I run the tests locally with Wayland enabled I can see that the window is hidden. If it's not allowed I would expect that this is a no-op. Also there is bug 1923833 which doesn't let us restore the window from minimize. Given all that I wonder if we should not allow a window to get minimized when Firefox is run in headful mode with Wayland active. The command may return an unsupported error.

Comment 33

[Martin Stránský [:stransky] (ni? me)]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #32)

Hm, but when I run the tests locally with Wayland enabled I can see that the window is hidden.

Yes, the outcome may differ on actual environment - but it's not guaranteed.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b3a1c8dde023
https://hg.mozilla.org/integration/autoland/rev/0a7afdef5078
[webdriver-bidi] Add the "browser.setClientWindowState" command. r=Sasha
https://github.com/mozilla-firefox/firefox/commit/2fa364b2e057
https://hg.mozilla.org/integration/autoland/rev/e17bbefb75d9
[wdspec] Set initial window position to (200,100) to account for possible OS docks and menus. r=Sasha
https://github.com/mozilla-firefox/firefox/commit/05faf9fa7d8d
https://hg.mozilla.org/integration/autoland/rev/8e5815c0aa24
[wdspec] Add initial tests for the "browser.setClientWindowState" command. r=Sasha

Comment 35

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/58882 for changes under testing/web-platform/tests

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/edb7554b8d64
https://hg.mozilla.org/integration/autoland/rev/59e243611dcb
Revert "Bug 1855028 - [wdspec] Add initial tests for the "browser.setClientWindowState" command. r=Sasha" for causing wd failures at window_rect/set.py

Comment 37

[Silaghi Andreea]

Backed out for causing wd failures at window_rect/set.py

Backout link
Push with failures
Failure Log
Failure line TEST-UNEXPECTED-FAIL | /webdriver/tests/classic/set_window_rect/set.py | test_partial_input[x] - AssertionError: assert 0 == 250

Comment 38

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The problem here is:

[task 2026-03-31T18:34:13.003+00:00] 18:34:13     INFO -         # Wayland forbids programmatic window movement in headful mode.
[task 2026-03-31T18:34:13.003+00:00] 18:34:13     INFO -         if is_wayland_headful:
[task 2026-03-31T18:34:13.003+00:00] 18:34:13     INFO - >           assert value["x"] == rect.get("x", original["x"])

Given that this affects the WebDriver classic tests the changes in https://phabricator.services.mozilla.com/D287168 should have caused that. And indeed it looks like that I made a mistake when resolving the merge conflicts. The logic of the test shouldn't have touched at all and should remain as:

https://searchfox.org/firefox-main/rev/65f89c383b216d3b4f210657c9af12a32048fb67/testing/web-platform/tests/webdriver/tests/classic/set_window_rect/set.py#212-218

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/34a9fcf52f8a
https://hg.mozilla.org/integration/autoland/rev/f0d334bd721d
[webdriver-bidi] Add the "browser.setClientWindowState" command. r=Sasha
https://github.com/mozilla-firefox/firefox/commit/c77bd8f68762
https://hg.mozilla.org/integration/autoland/rev/e9dae3de1d2c
[wdspec] Set initial window position to (200,100) to account for possible OS docks and menus. r=Sasha
https://github.com/mozilla-firefox/firefox/commit/f88febf73988
https://hg.mozilla.org/integration/autoland/rev/c7a664437631
[wdspec] Add initial tests for the "browser.setClientWindowState" command. r=Sasha

Comment 40

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/f0d334bd721d
https://hg.mozilla.org/mozilla-central/rev/e9dae3de1d2c
https://hg.mozilla.org/mozilla-central/rev/c7a664437631

Comment 41

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/f0d334bd721d
https://hg.mozilla.org/mozilla-central/rev/e9dae3de1d2c
https://hg.mozilla.org/mozilla-central/rev/c7a664437631

Comment 42

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5836b8265b7e
https://hg.mozilla.org/integration/autoland/rev/6a0707fa2b9f
[wpt PR 58882] - [Gecko Bug 1855028] [wdspec] Set initial window position to (200,100) to account for possible OS docks and menus., a=testonly

Comment 44

[rperta]

https://hg.mozilla.org/mozilla-central/rev/6a0707fa2b9f