Putting this bug on hold for now. The Network domain related events seem to be more important to get started on first.

This method is used for screenshots and various mouse input actions like hover, tap, and click as implemented on the ElementHandle class. That class is being used in DOMWorld.js to retrieve elements by a selector. As such it is necessary for the alpha release because Gutenberg e2e tests use a lot of calls to click().

Botond and/or Hiro, do we have some examples in the tree in how to get details about the layoutViewport and visibleViewport? Both are required to implement this CDP endpoint in Firefox. See https://chromedevtools.github.io/devtools-protocol/tot/Page#method-getLayoutMetrics. Thanks

So, based on the descriptions in that document:

• LayoutViewport.pageX/Y would be window.scrollX/Y
• LayoutViewport.clientWidth/Height would be document.documentElement.clientWidth/Height, assuming "layout viewport" (which is a somewhat ambiguous term) is being interpreted as "initial containing block" here, rather than "fixed viewport". (It may be worth running this by David Bokan to double-check the intention).
• VisualViewport fields other than zoom would be the corresponding fields of window.visualViewport, where that exists.
• Where window.visualViewport does not exist, zooming is not supported, so:
  • pageX/Y and clientWidth/Height are equal to the corresponding LayoutViewport fields
  • offsetX/Y are 0
  • scale is 1
• VisualViewport.zoom would be window.devicePixelRatio

Does that help?

(the link doesn't work with enabling contents blocking)

Thanks Botond! That's pretty helpful information. Especially for the visibleViewport availability. As you say the Browser Zoom feature in Firefox doesn't have an effect here? It's only about pinching in mobile apps?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)

(the link doesn't work with enabling contents blocking)

Sometimes the page throws a 404. Go to a different page and back to Page to get content.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

As you say the Browser Zoom feature in Firefox doesn't have an effect here? It's only about pinching in mobile apps?

The browser zoom will have an impact on window.devicePixelRatio (and therefore on VisualViewport.zoom).

(In reply to Botond Ballo [:botond] from comment #7)

As you say the Browser Zoom feature in Firefox doesn't have an effect here? It's only about pinching in mobile apps?

The browser zoom will have an impact on window.devicePixelRatio (and therefore on VisualViewport.zoom).

So as it looks like the visual viewport support is not enabled by default in Firefox yet, and you will have to set the dom.visualviewport.enabled preference to true. Which means that with the CDP feature enabled we will have to also set this preference to make use of the visual viewport details.

But even with it enabled the value for scale sticks with 1 even I zoom in our out in Firefox. Given your comment 4 I assume I can simply ignore this property?

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #8)

So as it looks like the visual viewport support is not enabled by default in Firefox yet,

It's currently only enabled on Android. It will be enabled on desktop at a later time, as part of the desktop zooming effort.

and you will have to set the dom.visualviewport.enabled preference to true. Which means that with the CDP feature enabled we will have to also set this preference to make use of the visual viewport details.

I wouldn't recommend changing the preference, as the implementation is not passing tests on desktop yet and may return unexpected values.

I would recommend testing for the existence of window.visualViewport, and using the mentioned fallbacks if it doesn't exist.

But even with it enabled the value for scale sticks with 1 even I zoom in our out in Firefox.

The names are a bit confusing, but based on the descriptions, scale reflects pinch-zoom, and zoom reflects browser zoom.

By "zoom in or out in Firefox", I assume you mean browser zoom on Firefox desktop, in which case I would expect scale to remain at 1 and zoom to change.

Given your comment 4 I assume I can simply ignore this property?

I'm not sure what exactly the "CDP feature" is, but if it's applicable to Android, you don't want to ignore scale as that's relevant for pinch-zooming there.

Even if it's not applicable to Android, since we're planning to bring pinch-zooming to desktop, I would suggest not ignoring scale but hooking it up with the fallback as suggested (but that's more negotiable).

LayoutViewport.clientWidth/Height would be document.documentElement.clientWidth/Height, assuming "layout viewport" (which is a somewhat ambiguous term) is being interpreted as "initial containing block" here, rather than "fixed viewport". (It may be worth running this by David Bokan to double-check the intention).

Not quite sure about "intent" but my read of [1] in Chromium is that we do actually use the "fixed viewport" here. i.e. if you have a 600px width device with <meta name="viewport" content="width=device-width,minimum-scale=2">, the LayoutViewport.clientWidth will be 300 because the fixed viewport is based on minimum scale.

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/inspector/inspector_page_agent.cc?l=1170&rcl=953620c30979c1ba97b8e0cfaa72fea972333ea5

Thanks, David.

In that case, the computation for LayoutViewport.clientWidth would be something like document.scrollingElement.scrollWidth - window.scrollMaxX (and similarly for height). It can be changed to window.innerWidth/Height after bug 1514429 is fixed.

Thank you all for these information! I'm hoping to get to this bug soon.

(In reply to Botond Ballo [:botond] [standards meeting Nov 4-9] from comment #11)

In that case, the computation for LayoutViewport.clientWidth would be something like document.scrollingElement.scrollWidth - window.scrollMaxX (and similarly for height). It can be changed to window.innerWidth/Height after bug 1514429 is fixed.

I think that might miss some edge-cases related to the viewport meta tag. E.g. the fixed viewport can be restricted with minimum-scale=1 for example which might not be reflected by document.scrollingElement.scrollWidth - window.scrollMaxX. Granted, I don't know how Gecko works in these cases but definitely something to check.

(In reply to David Bokan from comment #13)

(In reply to Botond Ballo [:botond] [standards meeting Nov 4-9] from comment #11)

In that case, the computation for LayoutViewport.clientWidth would be something like document.scrollingElement.scrollWidth - window.scrollMaxX (and similarly for height). It can be changed to window.innerWidth/Height after bug 1514429 is fixed.

I think that might miss some edge-cases related to the viewport meta tag. E.g. the fixed viewport can be restricted with minimum-scale=1 for example which might not be reflected by document.scrollingElement.scrollWidth - window.scrollMaxX. Granted, I don't know how Gecko works in these cases but definitely something to check.

So I tried the attached simple testcase, where the content size is twice the ICB size, so there would be room for the fixed viewport to be expanded, but the minimum-scale=1 restricts it.

In this scenario, I am seeing:

• document.scrollingElement.scrollWidth is twice my screen width (on the phone I'm testing with, 720px)
• window.scrollMaxX is equal to my screen width (360px)
• their difference is equal to my screen width (360px)

That seems like the desired value of the fixed viewport width. Not sure which part of the calculation you were expecting to trip up?

As it looks like there is still a bit of discussion going on. As such I will work on the device emulation support (bug 1588432) first. This will also help to prove that particular values can be compared with Chrome not only for Firefox, but for the full set of devices as supported by Puppeteer. The first thing to implement is bug 1544417.

That seems like the desired value of the fixed viewport width. Not sure which part of the calculation you were expecting to trip up?

If you change to minimum-scale=0.75 does it still work? If the scrollMaxX is now 240 then this works correctly (since the fixed viewport will how be 480px wide and the scrollWidth should remain 720px). Thanks for checking.

(In reply to David Bokan from comment #16)

If you change to minimum-scale=0.75 does it still work? If the scrollMaxX is now 240 then this works correctly (since the fixed viewport will how be 480px wide and the scrollWidth should remain 720px). Thanks for checking.

Yep, still works.

The reason this works is that window.scrollMaxX is the largest value that window.scrollX can take on, and as per the discussion we had here, window.scrollX/Y is limited to a scroll range determined by the fixed viewport.

After tinkering with Bug 1549465, this looks like the next method that is blocking gutenberg, since Puppeteer uses it for mouse clicks.

Maja, if that is blocking you feel free to pick it up. Otherwise I would start working on this bug after bug 1544417 has been fixed.

Puppeteer's page.click(someSelector) breaks in ElementHandle._clickablePoint, and in particular it relies on the layoutViewport value from the Page.getLayoutMetrics command.

(In reply to Botond Ballo [:botond] from comment #11)

In that case, the computation for LayoutViewport.clientWidth would be something like document.scrollingElement.scrollWidth - window.scrollMaxX (and similarly for height). It can be changed to window.innerWidth/Height after bug 1514429 is fixed.

Please note that the patch from bug 1514429 landed 8h ago. So I would suggest to wait and directly make use of window.innerWidth/Height.

Setting priority back to 'P3' so the bug appears in the project's Bugzilla queries.

We are moving out the more complex "visibleViewport" part to bug 1602926 given that it is not immediately necessary. Means as first step I'm going to implement the contentSize (for full screenshots) and layoutViewport (Gutenberg tests) first.

So I did two tests with Chrome and Firefox. All with a maximized window. One test is for a page without scrollbars, and another one with vertical scrollbars.

No scrollbars: http://example.org

Chrome:
"layoutViewport":{"pageX":0,"pageY":0,"clientWidth":1680,"clientHeight":846}
"contentSize":{"x":0,"y":0,"width":1680,"height":846}

Firefox:
"layoutViewport":{"pageX":0,"pageY":0,"clientWidth":1680,"clientHeight":895}
"contentSize":{"x":0,"y":0,"width":1680,"height":895}

Our available height in content is a bit larger because in Firefox we do not show the notification bar that the browser is under remote control. So I assume that this are those 49px.

With vertical scrollbar: https://www.mozilla.org

Chrome:
"layoutViewport":{"pageX":0,"pageY":0,"clientWidth":1665,"clientHeight":846}
"contentSize":{"x":0,"y":0,"width":1665,"height":7114}

Firefox:
"layoutViewport":{"pageX":0,"pageY":0,"clientWidth":1680,"clientHeight":895}
"contentSize":{"x":0,"y":0,"width":1665,"height":6965}

Similar situation for the available height, but check the clientWidth for the layoutViewport. There is a difference of 15px, which is exactly the scrollbar width.

Chromes implementation seems to be located here:
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc?type=cs&g=0&l=716

Given that it includes the scrollbar the scrollbar_inclusion flag is true for at least CDP. So just using window.innerWidth and window.innerHeight doesn't seem to be enough.

Hm, for my daily profile and when starting Firefox with a new profile outside of Puppeteer I get the expected 1680px. I should investigate.

Sorry, 1680px isn't actually expected. So Chrome captures the screenshot definitely without the scrollbars, and as such their width and height will have to be taken into account. When doing that I get:

{"layoutViewport":{"pageX":0,"pageY":0,"clientWidth":1665,"clientHeight":895},"contentSize":{"x":0,"y":0,"width":1665,"height":6965}}

It's exactly what it should be. So all fine.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bce509b03c5368666fd07de8cb89b2b22853592f

https://hg.mozilla.org/mozilla-central/rev/4a36df9e03e9

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #26)

Sorry, 1680px isn't actually expected. So Chrome captures the screenshot definitely without the scrollbars, and as such their width and height will have to be taken into account. When doing that I get:

{"layoutViewport":{"pageX":0,"pageY":0,"clientWidth":1665,"clientHeight":895},"contentSize":{"x":0,"y":0,"width":1665,"height":6965}}

Just so I understand this: if Chromium takes screenshots of the full document area as if the scrollbars are not present, would we want to follow Juggler’s example with ScrollbarManager.js and hide them prior to taking the screenshot?

(In reply to Andreas Tolfsen 「:ato」 from comment #31)

Just so I understand this: if Chromium takes screenshots of the full document area as if the scrollbars are not present, would we want to follow Juggler’s example with ScrollbarManager.js and hide them prior to taking the screenshot?

Would totally make sense also given that there is Emulation.setScrollbarsHidden(), which we haven't implemented yet. I filed bug 1604799 for it. Thanks for noticing that.