w3c / webdriver

Remote control interface that enables introspection and control of user agents.
https://w3c.github.io/webdriver/
Other
676 stars 190 forks source link

setWindowRect should be matched as a capability #1793

Open gsnedders opened 4 months ago

gsnedders commented 4 months ago

c.f. https://matrix.to/#/!zvtoFTlOBeOwHrBlEK:matrix.org/$yumW_YPZ6kAsKlD3SB_muMP9ImAUoF893INwdNegTEw?via=matrix.org&via=mozilla.org&via=igalia.com

Nowhere does https://w3c.github.io/webdriver/#dfn-matching-capabilities match setWindowRect, so it effectively is ignored when passed as a capability (see: https://github.com/w3c/webdriver/issues/1792).

However, per @whimboo:

In Firefox we match that capability and fail if the value is not appropriate

I have not tested what other implementations do.

whimboo commented 4 months ago

Matching on our side takes place as:

https://searchfox.org/mozilla-central/rev/9e5b13e92b50c61e60a42bdc2cb2e5966ae28795/remote/shared/webdriver/Capabilities.sys.mjs#560-573

Note scenarios like window manipulation tests that want to position or resize browser windows. If you would run these tests on desktop it should basically work in nearly all the cases (exception would be Wayland) but on Android it should basically fail.

With matching clients can request that this feature is present to actually run all the tests on that given platform and if it's not supported the session will not be created.

wpt.fyi shows the difference between Firefox on Android and Chrome on Android. For Chrome an assertion only fails without any details that the command actually doesn't work, while for Firefox an unsupported operation is raised.

With matching the capability we would get a clear failure right in New Session for all browsers.

OrKoN commented 4 months ago

Chrome does not do matching for this capability and it is a return-only value: https://source.chromium.org/chromium/chromium/src/+/main:chrome/test/chromedriver/session_commands.cc;l=189;drc=21802bc29a3f1f96a333bc52039dc56764a85524 (it will reject the property as invalid argument if it is in the capabilities request)

whimboo commented 4 months ago

@shs96c @AutomatedTester @titusfortner @jimevans from the Selenium side of view how would you expect a remote end to behave when it doesn't support window manipulation? Should matching the setWindowRect capability fail if it's value in the requested capabilities is set to true? Or would you expect all the tests trying to manipulate the window to fail because the requested operation cannot be achieved?

gsnedders commented 4 months ago

It likewise is return-only in safaridriver.

(Despite being the OP, I was just doing what the action item from the meeting said; I don't have any particularly strong opinion on whether we should make it possible to match on it.)

jimevans commented 4 months ago

Generally speaking, Selenium expects that capability to be return-only. I don’t know of anyone attempting to match on that capability in the Selenium world.

whimboo commented 2 months ago

Given that matching is not expected should we close the issue? Or should we try to match all known and supported capabilities?

titusfortner commented 2 months ago

I've never understood why it was there in the first place. 😂

My philosophy is that your test should already have all the information it needs to execute a deterministic test, and it shouldn't be toggling the action to take based on the returned capabilities, so it shouldn't need to be something that gets returned, let alone set, but that's my opinion.

That said, if the spec says these need to be matching, do we at least need to change the spec language?