Skip to content
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.

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

Clarify behaviour of setWindowRect when only setting window size or position is possible #1826

Open
jgraham opened this issue Jul 1, 2024 · 5 comments

Comments

@jgraham
Copy link
Member

jgraham commented Jul 1, 2024

Linux running Wayland doesn't allow applications to set (or know) their screen coordinates, but does allow them to specify the size.

Currently in WebDriver one can imagine handling this in multiple different ways (not all mutually exclusive):

  • Return false in the setWindowRect capability because the implementation doesn't support all the window resizing and repositioning commands (although arguably it does support all the commands, just not fully).
  • Support setWindowRect, but fail the command entirely if either x or y arguments are provided
  • Support setWindowRect, but just ignore any x and y arguments (the spec's language around "implementation defined steps" would arguably allow this, but it wouldn't have the desired effect).

In practice I suspect that for compatibility with existing tests we'd be best off with only the third option i.e. silently ignore any attempts to set x and y in setWindowRect. That's because I suspect a lot of tests start by trying to set the window to a known size and position for consistency. They almost never actually care what the window position is, only that it's visible.

However that seems least in the spirit of the current spec and could surprise test authors in the (rare?) case that they really do care about the position for whatever reason. That said they get the WindowRect in the return value, so you can at least notice that the return value didn't match what you asked for (which can already be the case in some scenarios).

A sort of in-between option would be to fail the command if you only get x and y arguments i.e. don't get either width or height. That might catch some cases where people really are trying to set the window position. But it might also cause unnecessary breakage in cases where they don't really care about the window position, but just happen to be setting the position in a different command for some reason.

My sense if that we should clarify the spec to make it optional to actually move the window when x and y are supplied, and if authors really care whether that move happened they need to check the return value. That's not very theoretically pure, but does seem likely to break the fewest users, so I think it's preferred from a priority of constituencies point of view. But other input would be very appreciated.

@whimboo
Copy link
Contributor

whimboo commented Jul 1, 2024

CC'ing @OrKoN, @sadym-chromium and @gsnedders to get input from Google and Apple.

@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed setWindowRect where position can't be set.

The full IRC log of that discussion <AutomatedTester> topic: setWindowRect where position can't be set
<AutomatedTester> github: https://github.com//issues/1826
<AutomatedTester> jgraham: This is initially about classic but will affect setWindowRect
<AutomatedTester> s/setWindowRect/bidi
<AutomatedTester> ... in linux there are times where the answers we are returned does not always match reality
<AutomatedTester> q+
<AutomatedTester> ... so for the setter, should it fail or should it ignore you?
<AutomatedTester> ... we are doing this in WPT and run it on wayland it will fail and gecko tests time out
<AutomatedTester> ... Mozilla has a major push to fix because of the above
<AutomatedTester> ... there are different scenarios in the issue that we can do
<AutomatedTester> ack next
<AutomatedTester> AutomatedTester: what happens if you set it and it fails?
<orkon> q+
<AutomatedTester> jgraham: in wayland it allows either x,y or size. not both
<AutomatedTester> AutomatedTester: if I remember that we do the best case here. E.g. try pass it and then do best guess. The main reason was mobile can-'t do any of these things so I am happy with it returning where it thinks everything is
<AutomatedTester> ack next
<AutomatedTester> orkon: it sounds like apply as much as possible but we can error. I know we the browser will not always be able to do this. Our position is we can return without setting the position and returning the value of where it thinks it is
<AutomatedTester> jgraham: I think that we need to make sure that we dont break tests. I don't think we need to make this fatal if things don't work out
<AutomatedTester> ... and set the value and then return where it thinks it is
<AutomatedTester> gsnedders_irc: do we expect the return the command with error? on mobile we can return errors
<AutomatedTester> AutomatedTester: I am happy to return the 0, 0 and window size and not error?
<AutomatedTester> gsnedders_irc: do we expect people to run desktop tests on mobile?
<AutomatedTester> automatedtester: yes, 80% of tests will work that way unless people think there are feature differences between the platforms. They want write once run everywh
<AutomatedTester> ...ere
<AutomatedTester> automatedtester: do we want it to try on desktop and reutn the values it gets to and on mobile error
<AutomatedTester> jgraham: yes. There are some imprementation defined ways to error

@jgraham
Copy link
Member Author

jgraham commented Jul 16, 2024

#1830

@whimboo
Copy link
Contributor

whimboo commented Jul 17, 2024

Here a reply from @jgraham from the already merged #1830:

After merging this I noticed we have some uniformly passing tests that assume that input like {x: 200} is just silently ignored rather than either producing an error or setting the x-coordinate to 200 (where possible). Silently ignoring broken input seems like bad design, but I guess I should revert that part on the basis that's the defacto behaviour.

@whimboo
Copy link
Contributor

whimboo commented Jul 19, 2024

The tests @jgraham is referring to are these:

https://github.com/web-platform-tests/wpt/blob/e8b8d20c83f1f548464eb62caa5093ed30c2d432/webdriver/tests/classic/set_window_rect/set.py#L200-L207

To keep backward compatibility we probably should still allow to pass only one value, but should make sure that we don't follow the same for WebDriver BiDi.

@OrKoN, @gsnedders, @shs96c, @AutomatedTester any feedback from your side?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants