Bug 1802466 Comment 8 Edit History

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

So with the attached patch, we pass the test `text-input-vertical-overflow-no-scroll.html` on my local machine, but we fail on TreeHerder with an off-by-1px scroll position in some of the `scrollTop` comparisons. I think that failure depends on font  & font-size & maybe other factors to trigger the off-by-1px issue (and then also you need a little bit of luck).  I was able to trigger it locally after changing the font and the font-size; but it still wasn't 100% reproducible. It did interestingly seem to toggle between "rounded-up" vs. "rounded-down" when I backgrounded/foregrounded the window that the test was running in, which was a clue.

Looking at this off-by-1px failure on pernosco, I chased the pixel change back a bit and landed here:
https://searchfox.org/mozilla-central/rev/132ffbfd6842e5ecd3813673c24da849d3c9acf8/gfx/layers/FrameMetrics.cpp#52,59-62
```cpp
void FrameMetrics::RecalculateLayoutViewportOffset() {
...
  // For the root, the two viewports can diverge, but the layout
  // viewport needs to keep enclosing the visual viewport.
  KeepLayoutViewportEnclosingVisualViewport(GetVisualViewport(),
                                            mScrollableRect, mLayoutViewport);
```
At this point in my pernosco run, I have:
```
mLayoutViewport.y = 617
GetVisualViewport().y = 616.016663
```
...and this^ difference ends up reducing the y-position of our layout viewport to `616.016663` and generating an `RepaintRequest` with that updated `mLayoutViewport` which ultimately causes a change in the scroll position of our root frame from 617 to 616. And that's observable to the test.

I'm chasing that back further to see what more I can find out, but I wonder if that's something we can suppress or avoid by doing some sort of flushing, or by ensuring certain sizes or positions are whole-pixel-valued, or if there's any other recommended workaround to avoid this?  (Or maybe it's a legit bug? It definitely seems a bit fiddly that this is producing ~arbitrary-seeming 1px changes to `scrollTop` without the user actually scrolling.)
So with the attached patch, we pass the test `text-input-vertical-overflow-no-scroll.html` on my local machine, but we fail on TreeHerder with an off-by-1px scroll position in some of the `scrollTop` comparisons. I think that failure depends on font  & font-size & maybe other factors to trigger the off-by-1px issue (and then also you need a little bit of luck).  I was able to trigger it locally after changing the font and the font-size; but it still wasn't 100% reproducible. It did interestingly seem to toggle between "rounded-up" vs. "rounded-down" when I backgrounded/foregrounded the window that the test was running in, which was a clue.

Looking at this off-by-1px failure on pernosco, I chased the pixel change back a bit and landed here:
https://searchfox.org/mozilla-central/rev/132ffbfd6842e5ecd3813673c24da849d3c9acf8/gfx/layers/FrameMetrics.cpp#52,59-62
```cpp
void FrameMetrics::RecalculateLayoutViewportOffset() {
...
  // For the root, the two viewports can diverge, but the layout
  // viewport needs to keep enclosing the visual viewport.
  KeepLayoutViewportEnclosingVisualViewport(GetVisualViewport(),
                                            mScrollableRect, mLayoutViewport);
```
At this point in my pernosco run, I have:
```
mLayoutViewport.y = 617
GetVisualViewport().y = 616.016663
```
...and this^ difference ends up reducing the y-position of our `mLayoutViewport` to `616.016663` and generating a `RepaintRequest` with that updated `mLayoutViewport` which ultimately causes a change in the scroll position of our root frame from 617 to 616. And that's observable to the test.

I'm chasing that back further to see what more I can find out, but I wonder if that's something we can suppress or avoid by doing some sort of flushing, or by ensuring certain sizes or positions are whole-pixel-valued, or if there's any other recommended workaround to avoid this?  (Or maybe it's a legit bug? It definitely seems a bit fiddly that this is producing ~arbitrary-seeming 1px changes to `scrollTop` without the user actually scrolling.)

Back to Bug 1802466 Comment 8