1857961 - Add support for the "devicePixelRatio" argument for "browsingContext.setViewport"

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

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

The devicePixelRatio argument has just been added to the WebDriver BiDi specification.

Tests will be added as part of https://github.com/web-platform-tests/wpt/pull/42398, which will be synced via bug 1857517.

Comment 1

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

I was trying to get this argument working locally and while doing so I noticed a problem. The wpt-runner sets the layout.css.devPixelsPerPx preference to 1 in its user.js file. This actually seems to lock the window.devicePixelRatio to the value of 1 even when I modify the BrowsingContext.fullZoom property. That means that tests fail when checking the window.devicePixelRatio property whenever a setting is requested that that not 1.

Emilio, is that the expected behavior? If yes, what could we do to actually reflect the zoom setting via window.devicePixelRatio? We cannot remove this pref for just specific WebDriver BiDi test.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

I can't reproduce that (and I don't see what in the code would cause that). If I change devPixelsPerPx to 1 and zoom in, I see window.devicePixelRatio react to that. Are you looking at the right window object?

Comment 3

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

Yes, running it manually all works fine. But there is a problem when running an existing wdspec test with the added feature. Then it fails and the zoom level gets reset when I modify it right after a navigation. That happens even with the preference browser.zoom.siteSpecific set to false. I'll have to investigate that tomorrow. For today I worked around it by adding an extra sleep to the test.

Comment 4

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

So even with bug 1873916 fixed and the preference correctly set I see a race condition in that reset behavior. I can get around that when waiting for the next animation frame before doing any viewport changes. Adding that makes the test stable and I didn't run into such an issue again.

Before I'm going to consider using it I may check certain places in our browser code where the fullZoom property gets set.

Comment 5

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

Yesterday I run our already existing wdspec tests with Chrome and this time in headful mode. By surprise I noticed that the visual output does not change at all when they override the devicePixelRatio. After talking to them they second that this is actually the expected behavior and the emulation is only about the window.devicePixelRatio value. Nevertheless by overriding this value the CSS media queries will be triggered.

So this is fundamentally different to what I've implemented at the moment by using fullZoom. On top of the actual visually change, which also increases or shrinks the viewport) we also have the situation that when changing the zoom the window.devicePixelRatio changes as well. This only applies to Firefox and no other browser (see the wontfix'ed bug 809788).

Taking all that into account and the fact that in our CDP implementation we already use BrowsingContext.overrideDPPX, which is basically similar to what Chrome is doing, and we never got feedback that this approach is not working, I think that the best approach for now is to continue using this exact same method for the emulation. If it turns out that this approach is not good enough we can fix / improve it at a later time.

Comment 6

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

Implementing that would cause a discrepancy to the BiDi spec. So I filed https://github.com/w3c/webdriver-bidi/issues/638 to clarify the expectation from a users point of view first.

Comment 7

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

Somehow the milestone status wasn't updated, and we already planned it for M10.

Comment 8

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1857961 - [bidi] Add support for the "devicePixelRatio" argument for "browsingContext.setViewport".

Comment 9

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1857961 - [wdspec] Add tests for the "devicePixelRatio" argument for "browsingContext.setViewport".

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bcd84900dd3f
[bidi] Add support for the "devicePixelRatio" argument for "browsingContext.setViewport". r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/3bc4640f938f
[wdspec] Add tests for the "devicePixelRatio" argument for "browsingContext.setViewport". r=webdriver-reviewers,whimboo

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/45841 for changes under testing/web-platform/tests

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/bcd84900dd3f
https://hg.mozilla.org/mozilla-central/rev/3bc4640f938f

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f0bd6eaf0f9
[wpt PR 45841] - [Gecko Bug 1857961] [wdspec] Add tests for the "devicePixelRatio" argument for "browsingContext.setViewport"., a=testonly

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/5f0bd6eaf0f9