Bug 1644271 Comment 23 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

The dt6 failure is interesting. The test starts off with the devtools open and docked to the bottom, and checks some things. Then it moves the devtools to the right side of the window and checks more things. The problem is that when the devtools get moved over to the right, we end up with a spurious vertical scrollbar on the content.

The reason that happens is as follows. When the root scrollframe goes through reflow, it tries the [various combinations of scrollbars](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#826-843) to see which one is "consistent". For each attempt, it [reflows the contents assuming the indicated scrollbar configuration](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#411), and then checks to see if the [scrollbars wanted at that layout](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#501-503,518-520) match the initial assumption.

However, in order to see if the scrollbars are wanted, it computes a visual viewport using [this fishy code](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#464-475) where in particular the call to `CalculateCompositionSizeForFrame` turns around and asks the scrollframe for its [scroll port rect](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/base/nsLayoutUtils.cpp#8633). But that scrollport rect is stale, because it doesn't get updated until *after* there's a consistent layout arrived at, [here](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#541).

So in this test case, the stale scrollport height that gets used is the one from when the devtools was docked on the bottom, and so it ends up "wanting" a vertical scrollbar.

That being said, this addition of the vertical scrollbar does shrink the visual viewport width, which could/should kick off another reflow cycle because it marks the scrollbars dirty [here](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/base/PresShell.cpp#11062). And that second reflow should then get rid of the vertical scrollbar by detecting a consistent layout with no scrollbars. However, that second reflow doesn't happen because of local changes I added to fix other issues.

I think overall it's better to try and make this arrive at a consistent state in a single reflow, so I'd prefer to make the visual viewport size calculation use a not-stale scrollport rect, rather than find another solution for the other issues.
The dt6 failure (`devtools/client/inspector/test/browser_inspector_highlighter-rulers_03.js`) is interesting. The test starts off with the devtools open and docked to the bottom, and checks some things. Then it moves the devtools to the right side of the window and checks more things. The problem is that when the devtools get moved over to the right, we end up with a spurious vertical scrollbar on the content.

The reason that happens is as follows. When the root scrollframe goes through reflow, it tries the [various combinations of scrollbars](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#826-843) to see which one is "consistent". For each attempt, it [reflows the contents assuming the indicated scrollbar configuration](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#411), and then checks to see if the [scrollbars wanted at that layout](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#501-503,518-520) match the initial assumption.

However, in order to see if the scrollbars are wanted, it computes a visual viewport using [this fishy code](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#464-475) where in particular the call to `CalculateCompositionSizeForFrame` turns around and asks the scrollframe for its [scroll port rect](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/base/nsLayoutUtils.cpp#8633). But that scrollport rect is stale, because it doesn't get updated until *after* there's a consistent layout arrived at, [here](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/generic/nsGfxScrollFrame.cpp#541).

So in this test case, the stale scrollport height that gets used is the one from when the devtools was docked on the bottom, and so it ends up "wanting" a vertical scrollbar.

That being said, this addition of the vertical scrollbar does shrink the visual viewport width, which could/should kick off another reflow cycle because it marks the scrollbars dirty [here](https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/layout/base/PresShell.cpp#11062). And that second reflow should then get rid of the vertical scrollbar by detecting a consistent layout with no scrollbars. However, that second reflow doesn't happen because of local changes I added to fix other issues.

I think overall it's better to try and make this arrive at a consistent state in a single reflow, so I'd prefer to make the visual viewport size calculation use a not-stale scrollport rect, rather than find another solution for the other issues.

Back to Bug 1644271 Comment 23