w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
378 stars 42 forks source link

Add support for "isLandscape" argument for "browsingContext.setViewport" command #771

Open whimboo opened 2 months ago

whimboo commented 2 months ago

Both Puppeteer and Playwright support an isLandscape boolean parameter for the setViewport command, which is currently missing in WebDriver BiDi. We should add support for this parameter with the following behavior:

jgraham commented 2 months ago

I don't really understand the use case here. If you want to set a landscape display, just set a larger width than height. One can also imagine a tablet or a fold-out phone with a square display. Is there anything else that this is controlling?

whimboo commented 2 months ago

We would have to let Puppeteer or Playwright folks speak up here. But I assume that this is helpful for the knownDevices class so that you don't have to define all the viewports twice, but just swap the width and height.

jgraham commented 2 months ago

Yes, feedback from client authors would be very helpful. On the face of it I don't see why setting a boolean is much easier than swapping argument values, but maybe I'm missing some extra bit of state that this value controls.

mxschmitt commented 2 months ago

We use CDP's screenOrientation, like Puppeteer to emulate screen.orientation.angle, screen.orientation.type and window.orientation - see here and here for our tests. I think thats what it is mostly for from our side.

OrKoN commented 2 months ago

Puppeteer uses it to emulate https://w3c.github.io/screen-orientation/#dom-screen-orientation (tests) and uses the same method. A workaround for a one-time emulation can be probably done using a preload script but it might be also useful to emulate the screen orientation dynamically to test transitions from one mode to another after the page is loaded.

From the WebDriver BiDi spec perspective, we should probably closer follow the corresponding Web spec and what CDP does instead of only exposing isLandscape flag. Perhaps, we should even define the WebDriver API in https://w3c.github.io/screen-orientation/#dom-screen-orientation (although I am not sure if this feature might span multiple specifications in the future).

jgraham commented 2 months ago

At least in gecko, the effective orientation seems to initially be set with just the height >= width logic I suggested: https://searchfox.org/mozilla-central/source/widget/Screen.cpp#19 and a quick test on my phone didn't give a value of angle other than 0 or 90 (i.e. portrait or landscape), although that may depend on phone settings or similar.

So I wouldn't be opposed to adding an angle parameter, but we might need some mechanism to indicate that certain values are unsupported; I'm not sure.

jgraham commented 2 months ago

(alternatively I suppose just allowing setting the orientation type to one of the four supported values might be enough, and the angle could be calculated).