w3c / webdriver-bidi

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

What is the expected behavior for device pixel emulation from the user's point of view? #638

Closed whimboo closed 1 month ago

whimboo commented 5 months ago

With the browsingContext.setViewport command and the devicePixelRatio argument it should be possible to override the screen's default device pixel ratio to simulate different screen densities. Setting the DPR is hereby covered under step 6 and 6.2 contains the following:

If device pixel ratio is not null, change the size of the CSS Pixel in context’s layout viewport such that it corresponds to device pixel ratio in device pixels.

This basically means that any change of the device pixel ratio should be reflected in the size of CSS Pixels affecting the layout viewport, eg. if DPR=2 is set a CSS pixel will be twice that large as a device pixel.

While implementing the support for devicePixelRatio in Firefox I noticed that when running the existing WebDriver BiDi tests with Chrome in headful mode that any other value for that argument is not reflected in the layout viewport. So this is basically a behavior that is not expected by the BiDi specification.

It's unclear for us what a user actually gains from that DPR override behavior compared to the spec'ed version. If we indeed don't want to affect the layout rendering, the BiDi spec would have to be updated to match that behavior.

@OrKoN or @jrandolf could you please take a look? Thanks!

CC @sadym-chromium, @jgraham, @juliandescottes, @lutien

OrKoN commented 5 months ago

@whimboo could you clarify what do you mean by the device pixel ratio not affecting the layout viewport? My understanding is that it is expected that the layout viewport remains the same in CSS pixels and only the rendered size might change.

P.S. the CSS pixel size is meant to be changed only in the context of determining the DPR as the CSS pixel size links to the determine device pixel ratio algorithm https://drafts.csswg.org/cssom-view/#determine-the-device-pixel-ratio

whimboo commented 5 months ago

@OrKoN before going too much into detail lets try to get an idea what users actually would expect.

So in Chrome it looks like that changing the devicePixelRatio doesn't actually change the size of the CSS pixels, but only triggers the media queries and image source selection. Is that what users expect?

Also when capturing screenshots, what dimension should the target screenshot have? Is that based on the device pixels? If not please note that when creating screenshots with a Retina display attached the MacOS internal screenshot feature and as well Firefox use that - so it's basically twice that large.

jrandolf commented 5 months ago

@whimboo I think saying change the size of the CSS pixel may be confusing. To clarify, let's say instead we are changing the size of a CSS pixel with respect to hardware pixels which is what we understand change the size of the CSS pixel as.

For example, if the viewport is 1000 hardware pixels with a DPR of 1 and we change the DPR to 2, then the viewport is 2000 hardware pixels (emulated). In CSS pixels, the viewport remains the same.

Also when capturing screenshots, what dimension should the target screenshot have?

This size of the screenshot should scale with the device pixel ratio, so dpr = 2 implies twice the size.

that any other value for that argument is not reflected in the layout viewport

Can you clarify this? What change do you expect? An illustrative example would be helpful.

jrandolf commented 4 months ago

From looking at the capabilities of CDP, we are unable to truthfully mimic DPR scaling as performed by page zoom. This is because CDP doesn't have a method of getting the DPR and cannot scale the CSS width and CSS height by percentage.

Before discussing this further, perhaps @gsnedders could express her opinions about this because Safari doesn't implement page zoom the same way Firefox and Chromium do. In particular, Safari doesn't set the DPR. Is this intended behavior?

whimboo commented 1 month ago

With PR #693 merged a while ago this issue should be solved.