1959040 - Add log information when trying to re-position a window which is not possible on Wayland

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P3)

Description

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

On bug 1857571 we updated the window positioning code for Wayland so that the Set Window Rect command no longer times out after 5s when trying to re-position a window, which is not allowed to do programmatically.

Based on the feedback from https://github.com/mozilla/geckodriver/issues/2224 and the provided trace log it is currently not clear why the command returns with the position not updated and a trace log entry with Requested window geometry matches makes it even more confusing:

1744063666429	Marionette	DEBUG	0 -> [0,7,"WebDriver:SetWindowRect",{"x":50,"y":50}]
1744063666430	RemoteAgent	TRACE	Checking window geometry 488x240 @ (0, 0)
1744063666430	RemoteAgent	TRACE	Requested window geometry matches
1744063666430	Marionette	DEBUG	0 <- [1,7,null,{"x":0,"y":0,"width":488,"height":240}]

We should add a INFO log line that explains that programmatically positioning the window is not allowed.

Comment 1

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

Actually based on the following comment by a user it seems to be possible to position a window for Chrome when running under Wayland.

Martin, over on bug 1857571 comment 4 you mentioned that it is not possible to position a window. But maybe there is something that we are missing in Firefox' implementation if Chrome is able to do that?

Comment 2

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

Oh I was reminded by Emilio about the fact that Chrome might use XWayland by default. I queried the other bug, that I mentioned in my last comment, and found the reference to the MOZ_ENABLE_WAYLAND=0 environment variable that will force Firefox to use XWayland as well. I'll ask the user if that fixes the issue for them.

Comment 3

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

The reporter mentioned that it still doesn't work whether MOZ_ENABLE_WAYLAND=0 is used or not. The implementation of Services.appinfo.isWaylandEnabled shows that we check DISPLAY first. Could this be a problem so that we return true before actually checking the value of the MOZ_ENABLE_WAYLAND env?

Comment 4

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

Replied at https://github.com/mozilla/geckodriver/issues/2224#issuecomment-2800037083

Comment 5

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

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

The reporter mentioned that it still doesn't work whether MOZ_ENABLE_WAYLAND=0 is used or not. The implementation of Services.appinfo.isWaylandEnabled shows that we check DISPLAY first. Could this be a problem so that we return true before actually checking the value of the MOZ_ENABLE_WAYLAND env?

I see. Okay let's mark it as Wayland only if WAYLAND_DISPLAY is set and don't check DISPLAY for it.

Comment 6

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

Hm, that code is actually correct. If we're missing DISPLAY we need to run on Wayland if WAYLAND_DISPLAY is set.
IsWaylandEnabled() from nsAppRunner is executed in non-headless mode only so we have only these two options.

I think the Services.appinfo.isWaylandEnabled is set from different location, IsWaylandEnabled()/nsAppRunner looks used for statup only.

Comment 7

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

Oh sorry, we use Services.appinfo.isWayland. Not sure what's different here. I was not able to find the connection to the actual implementation then.

Comment 8

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

Looks like we set window protocol here:
https://searchfox.org/mozilla-central/rev/8d018b89ae027c1f166dee5b8bc6c66b65a58712/widget/gtk/GfxInfo.cpp#547
and that's based on recently used window protocol.

Comment 9

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

There's the related protocol propagation code:
https://searchfox.org/mozilla-central/rev/8d018b89ae027c1f166dee5b8bc6c66b65a58712/widget/gtk/GfxInfo.cpp#1372

Comment 10

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

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

Oh sorry, we use Services.appinfo.isWayland. Not sure what's different here. I was not able to find the connection to the actual implementation then.

That should be set here:
https://searchfox.org/mozilla-central/rev/8d018b89ae027c1f166dee5b8bc6c66b65a58712/toolkit/xre/nsAppRunner.cpp#1619

So again only recent protocol is used.

Comment 11

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

Also if there's position difference between MOZ_ENABLE_WAYLAND 0/1 then that env has to have an effect.

Comment 12

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

Thank you for checking!

I tried myself locally now and it all works fine. Services.appinfo.isWayland reports the correct value when Firefox is started normally, and with MOZ_ENABLE_WAYLAND set to 0 or 1. In all those cases DISPLAY has a value of :0. So as you said it must be a bug in that used Window manager.

As such lets add a better log entry if someone tries to move a window when Wayland is in use.

Comment 13

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

Attachment: Bug 1959040 - [remote] Add log information when trying to re-position a window which is not possible on Wayland.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6af390f851dc
https://hg.mozilla.org/integration/autoland/rev/51372d01ada1
[remote] Add log information when trying to re-position a window which is not possible on Wayland. r=webdriver-reviewers,jgraham

Comment 15

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/51372d01ada1