1857571 - Update Marionette to handle window positioning on Wayland

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P2)

Description

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

Fix Marionette window positions tests

Comment 1

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

try: https://treeherder.mozilla.org/jobs?repo=try&revision=d738bbb7c313df6152982d0f117c4638b136961b

Comment 2

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

Attachment: Bug 1857571 [Linux] Add moz:wayland system capability for tests r?ahal

Comment 3

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

Attachment: Bug 1857571 [Linux] Don't test window positioning on Wayland r?ahal

Comment 4

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

Wayland protocol doesn't allow to position toplevel windows (or place them to selected workspace). It also doesn't support to get toplevel window position - random coordinates may be returned.

There's no point to test toplevel wayland window position unless it's in maximized/fullscreen or tiled mode. In such case we know window positions.

Comment 5

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

Thank you for the information Martin! Based on your feedback I think that we should do the following:

The Webdriver classic specification includes the setWindowRect capability which should basically indicate if a remote end supports all the resizing and positioning commands. Given that this is not possible with Wayland we should set this capability to false when returning the session capabilities in the New Session command. With that we won't need another Wayland specific capability.

Also we would have to assert for Wayland in the WebDriver:SetWindowRect command and return a unsupported operation error.

Do we have a mozinfo flag that we actually could use to indicate that these tests should not be run?

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit BugBot documentation.

Comment 7

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

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

Do we have a mozinfo flag that we actually could use to indicate that these tests should not be run?

AFAIK Wayland flags is available as nsIGfxInfo.windowProtocol or nsIXULRuntime.idl
https://searchfox.org/mozilla-central/rev/1865e9fba4166ab2aa6c9d539913115723d9cc53/xpcom/system/nsIXULRuntime.idl#346

C++ code uses GdkIsWaylandDisplay() call:
https://searchfox.org/mozilla-central/source/widget/gtk/WidgetUtilsGtk.h#33

Comment 8

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

What we actually need is flag for mozinfo so that it can be used to skip tests. As it looks like mozinfo has the display flag available which is set to wayland.

If we could still resize the window we should probably split the Marionette unit tests into two tests. One for setting the position and another one for resizing.

Comment 9

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

We should re-evaluate the priority of this bug given that it blocks moving all web-platform related tests to the Wayland pool. Also users on such machines using WebDriver classic are affected.

Maybe we should consider this bug for our M12 milestone.

Comment 10

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

Not sure if it's relevant but we already have such checks in Firefox itself:

let isWayland = gfxInfo.windowProtocol == "wayland";

https://searchfox.org/mozilla-central/rev/6b17ef1f53b0bc68a0d746599c01ad8723c80880/browser/base/content/test/popups/head.js#288

Or in tests:

skip-if = [
  "os == 'android'",
  "os == 'mac'", # Bug 1579623, 1776996
  "display == 'wayland' && os_version == '22.04'",  # Bug 1857240
  "http3",
  "http2",
]

https://searchfox.org/mozilla-central/rev/6b17ef1f53b0bc68a0d746599c01ad8723c80880/dom/base/test/fullscreen/mochitest.toml#52

Comment 11

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

Mozinfo has it too:

 sandbox.wayland = mozinfo.display == "wayland";

https://searchfox.org/mozilla-central/source/layout/tools/reftest/manifest.sys.mjs#633

Comment 12

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

Thank you for the additional information! It's good to see that we can easily get the Wayland status in Firefox and mozinfo. So it means that we only have to take care of Marionette and possible manifest updates.

Comment 13

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

I actually wonder if there is a way to use the Ubuntu 22.04 workers without Wayland given that otherwise we wouldn't be able to test the window positioning feature at all anymore. Joel, do you know if we have a different worker type, or is Wayland the only supported display manager?

Comment 14

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

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

I actually wonder if there is a way to use the Ubuntu 22.04 workers without Wayland given that otherwise we wouldn't be able to test the window positioning feature at all anymore. Joel, do you know if we have a different worker type, or is Wayland the only supported display manager?

You can set MOZ_ENABLE_WAYLAND=0 env variable and Firefox will run on X11 (XWayland). We support both (Wayland and X11/XWayland) everywhere on Linux.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

that hasn't been tested on the VM worker, most likely it will work though.

there is a desire to get X11 22.04 workers available in CI, but that will be later in H2.

Comment 16

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

(In reply to Joel Maher ( :jmaher ) (UTC -8) from comment #15)

there is a desire to get X11 22.04 workers available in CI, but that will be later in H2.

Does it mean that the MOZ_ENABLE_WAYLAND=0 environment variable doesn't work yet, or can it be used as a workaround until realy workers are available?

Comment 17

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

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

(In reply to Joel Maher ( :jmaher ) (UTC -8) from comment #15)

there is a desire to get X11 22.04 workers available in CI, but that will be later in H2.

Does it mean that the MOZ_ENABLE_WAYLAND=0 environment variable doesn't work yet, or can it be used as a workaround until realy workers are available?

Yes, you can use as a workaround until real workers are available.
Also the env variable set may be the only difference between X11 and Wayland workers.

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

that assumes xvfb is installed as per references in:
https://searchfox.org/mozilla-central/source/taskcluster/scripts/tester/test-linux.sh

^ it is likely that could silently fail as I don't believe we need xvfb as we are on a VM not a docker image.

I am not aware of it being tested, but it is worth trying.

Comment 19

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

There is a little bit that we should check for tests running on Wayland:

• Setting the size of the window should always work but not the position.
• What does calling WebDriver:GetWindowRect return for both the size and the position? Are these some fake numbers?
• Because setting the size and position is handled by the same command maybe we can just ignore x and y when Wayland is active.
• Extending the setWindowRect capability might not be an option because getWindowRect is also affected

Overall more users will switch to Wayland so that we indeed have to make sure that this command works. But we cannot only test with Wayland enabled in CI because we would loose test coverage for setting and getting the window position. As such we may have to run another wdspec test job that uses X11 for just these tests for classic and BiDi.

Beside that the wptrunner should be updated to not depend on the positioning of the test window.

James filed a WebDriver spec issue to clarify the Wayland behavior:
https://github.com/w3c/webdriver/issues/1826

Comment 20

[Julian Descottes [:jdescottes]]

Keeping for triage for next session. Will need to address this in the near future, so probably a good candidate for P2 backlog.

Comment 21

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

It is acceptable to ignore the actual values when trying to position a window on Wayland or any other window manager that cannot set a specific position for the window. This means we should not fail when x and y coordinates are provided but the target position is not reached due to the use of Wayland.

However, we still need to address how to run tests under both Wayland and X11 environments to ensure that window positioning is correctly tested for window managers that support it. But not sure if it's worth to spin a separate test job for just a handful of tests to run. But if we do we should get the wdspec tests running and move all the not yet covered Marionette tests over. So maybe 2 points are not enough but lets see.

Comment 22

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

https://github.com/w3c/webdriver/pull/1830 covers the required WebDriver classic spec changes.

Comment 23

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1857571 - [marionette] Handle window positioning on Wayland.

Comment 24

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1857571 - [wdspec] Update setting window position and dimensions tests to not fail on Wayland.

Comment 25

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1857571 - [wdspec] Put window minimization tests in the separate files to avoid breaking other tests.

Comment 26

[Alexandra Borovova [:Sasha]]

While working on handling window positioning, I've noticed a couple of more things which don't work as expected around window management:

1. The window maximizing, when calling window.maximize() command the window size is increased but not to the size of the screen (smaller than that). But it seems to also not work when trying to increase the size of the window to the size of the screen, again the window size increases but still stays smaller than the desired screen dimensions.
2. The restoring of the window after minimization. If we call window.minimize() and then window.restore() the window stays hidden (sizemodechange event is also not fired. Maybe it's also related to the inability to set coordinates, but I'm not sure.

Comment 27

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

Yes I see the same results on Fedora 40. It's not guaranteed on Wayland that any app size/state request is accepted by compositor.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b84cb063260
[marionette] Handle window positioning on Wayland. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/57fc69f2fc84
[wdspec] Update setting window position and dimensions tests to not fail on Wayland. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/b10786d2b3db
[wdspec] Put window minimization tests in the separate files to avoid breaking other tests. r=webdriver-reviewers,jdescottes

Comment 29

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

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

Comment 30

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1857571 - [wdspec] Disable window minimization tests on Android.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/179e501f7a4a
[wdspec] Disable window minimization tests on Android.

Comment 32

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2b84cb063260
https://hg.mozilla.org/mozilla-central/rev/57fc69f2fc84
https://hg.mozilla.org/mozilla-central/rev/b10786d2b3db
https://hg.mozilla.org/mozilla-central/rev/179e501f7a4a

Comment 33

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

Upstream PR merged by moz-wptsync-bot