Add support for the "devicePixelRatio" argument for "browsingContext.setViewport"
Categories
(Remote Protocol :: WebDriver BiDi, task, P2)
Tracking
(firefox127 fixed)
| Tracking | Status | |
|---|---|---|
| firefox127 | --- | fixed |
People
(Reporter: whimboo, Assigned: Sasha)
References
(Blocks 3 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.
|
Reporter | |
Updated•9 months ago
|
|
|
Reporter | |
Comment 1•7 months ago
|
||
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.
|
|
Reporter | |
Updated•7 months ago
|
|
||
Comment 2•7 months ago
|
||
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?
|
|
Reporter | |
Comment 3•6 months ago
|
||
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.
|
|
Reporter | |
Comment 4•6 months ago
|
||
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.
|
|
Reporter | |
Comment 5•6 months ago
|
||
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.
|
|
Reporter | |
Comment 6•6 months ago
|
||
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.
|
|
Reporter | |
Comment 7•6 months ago
|
||
Somehow the milestone status wasn't updated, and we already planned it for M10.
|
|
Reporter | |
Updated•5 months ago
|
|
Assignee | |
Updated•4 months ago
|
|
||
Updated•3 months ago
|
|
|
Assignee | |
Comment 8•3 months ago
|
||
|
|
Assignee | |
Comment 9•3 months ago
|
||
|
||
Comment 10•2 months ago
|
||
Pushed by [email protected]: 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
|
||
Comment 12•2 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/bcd84900dd3f
https://hg.mozilla.org/mozilla-central/rev/3bc4640f938f
Upstream PR merged by moz-wptsync-bot
|
|
||
Comment 14•2 months ago
|
||
Pushed by [email protected]: 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•2 months ago
|
||
| bugherder | ||
|
|
Reporter | |
Updated•1 month ago
|
Description
•