1. Enable apz.allow_zooming
2. Access https://twitter.com/Haiganon/status/1286415015713497088
3. Click the photo
4. See it changes scroll position, in other words, whether closing the photo shows the same screen as it was.

Expected: No change
Actual: It scrolls up

Thanks for filing, I can reproduce.

In ScrollFrameHelper::ReflowFinished the patch changes us from using GetVisualScrollRange to using GetScrollRangeForUserInputEvents. When the photo is open the page sets overflow-y: hidden. This makes GetScrollRangeForUserInputEvents return a 0 scroll range because it looks at overflow hidden (GetVisualScrollRange does not). Then I assume that makes us scroll to 0.

When the slider frame gets notified about the max/min pos change in nsSliderFrame::AttributeChanged and notices that the curpos is not between it, it scrolls the scrollframe.

This is tricky because we want the scrollbar attrs to match what the user can scroll, but this bug indicates one way that the scrollbars are strongly linked to (ie they can change instead of the other way around) the actual layout scroll position.

Set release status flags based on info from the regressing bug 1651332

Should we even be having a scrollbar if overflow-y:hidden is set? Presumably this is not zoomed in so maybe the scrollbars shouldn't even be active.

Root scroll frames always get scroll bars so we can toggle overflow quickly (like this page does), the scroll bars are just hidden (by moving them off to the side of the frame, out of view) when they shouldn't be visible. I'm pretty sure that optimization is important enough that we want to keep it.

Ok, fair enough. But maybe they should check whether they're visible or not before the fiddle with the scroll offset?

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)

Ok, fair enough. But maybe they should check whether they're visible or not before the fiddle with the scroll offset?

Yeah, that was a thought I had. But another thought I had was that GetScrollRangeForUserInputEvents returning the rect (0,0,0,0) is logically wrong for overflow hidden scroll frames whose scroll position isn't (0,0). It should probably be returning (scrollpos.x, scrollpos.y, 0, 0), I can get this approach to fix this bug but it causes other problems I haven't investigated yet.

It should probably be returning (scrollpos.x, scrollpos.y, 0, 0),

Also a good point

UpdateScrollbarPosition and ReflowFinished both use the scroll range for this. It usually doesn't matter, but the next patch will make it important.

Depends on D85983

https://hg.mozilla.org/mozilla-central/rev/475381b12bd5
https://hg.mozilla.org/mozilla-central/rev/f2886b1341e7