Closed Bug 1857961 Opened 1 year ago Closed 6 months ago

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

Categories

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

task
Points:
3

Tracking

(firefox127 fixed)

RESOLVED FIXED
127 Branch
Tracking Status
firefox127 --- fixed

People

(Reporter: whimboo, Assigned: Sasha)

References

(Blocks 2 open bugs, )

Details

(Whiteboard: [webdriver:m11], [wptsync upstream][webdriver:relnote])

Attachments

(2 files)

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.

Points: --- → 3
Priority: -- → P2
Whiteboard: [webdriver:backlog]

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.

Flags: needinfo?(emilio.alvarez96)
Flags: needinfo?(emilio.alvarez96) → needinfo?(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?

Flags: needinfo?(emilio) → needinfo?(hskupin)

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.

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.

Flags: needinfo?(hskupin)

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.

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.

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

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Whiteboard: [webdriver:backlog] → [webdriver:m10]
Assignee: hskupin → aborovova
Whiteboard: [webdriver:m10] → [webdriver:m11]
Pushed by aborovova@mozilla.com: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/45841 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m11] → [webdriver:m11], [wptsync upstream]
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch
Upstream PR merged by moz-wptsync-bot
Pushed by wptsync@mozilla.com: 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
Whiteboard: [webdriver:m11], [wptsync upstream] → [webdriver:m11], [wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: