To reproduce this issue, you need to pinch in/out repeatedly and move drag the contents rightward and leftward repeatedly.

This issue can probably be more noticeable once after bug 1423013 since the bug makes overflow-x:hidden area reachable.

Presumably this is a dup of bug 1516056.

Reopening since Botond says bug 1516056 is not going to fix this isssue.

STR is probably:

1. Load https://www.spit-ct.ro/documente-declarare-persoane-fizice#
2. Wait for finish loading
3. Shrink the content as possible by pinch zoom
4. Pinch zoom in a bit
5. Swipe rightward

I will try to generate a reduced test case.

No need, I have a tentative diagnosis and fix.

Here's what's happening on this page:

• The page has a content size of (1980.03, 1790.15) CSS pixels, and a layout viewport size of (1440, 1790.15). The page is overflow-x: hidden, so the layout viewport cannot be horizontally scrolled via user input.
• When zoomed out as far as possible, the zoom level is ~0.858. The composition bounds on my phone is (1080, 1536) screen pixels, leading to a visual viewport size of (1258.74, 1790.21) CSS pixels. Observe how this is a fraction of a pixel taller than the layout viewport due to rounding error.
• The visual viewport is legitimately horizontally scrollable within the layout viewport. When the user scrolls it to the right, the horizontal visual viewport offset increases from 0 to some larger value, say 100.
• We then run the logic in KeepLayoutViewportEnclosingVisualViewport, which is described as follows:

    // If visual viewport size is greater than the layout viewport, move the
    // layout viewport such that it remains inside the visual viewport. Otherwise,
    // move the layout viewport such that the visual viewport is contained
    // inside the layout viewport.

• To determine which of these is the case, we test whether the visual viewport is smaller than the layout viewport in both dimensions. Due to the rounding error mentioned above, the check for the vertical dimension fails. We do have a FuzzyEqualsMultiplicative check, but the difference here is too large for it.
• Therefore, we attempt to do the second thing, "move the layout viewport such that the visual viewport is contained inside it", which results in increasing the layout viewport offset to 100 as well.
• Layout grumbles about APZ moving the layout viewport in an overflow: hidden dimension, but accepts in nonetheless.
• When layout recomputes the APZ scrollable rect, its origin is the new layout viewport offset, which is now 100 horizontally.
• As far as APZ is concerned, the allowable scrollable range for the page is now (100,0,1440,1790.15), rather than (0,0,1440,1790.15). That is, 100 pixels on the left side of the page just became inaccessible.
• If you're still at this zoom level, you can scroll back there via the same process, but if you zoom in, the visual viewport is now properly contained inside the layout viewport, so the ability to sneakily async-scroll the layout viewport out of bounds is lost, and the [0,100] range is truly inaccessible until you zoom back out.

Given this diagnosis, the fix approach in my patch is to re-clamp the layout viewport to the scrollable rect at the end of KeepLayoutViewportEnclosingVisualViewport. However, there are other changes that would fix this too:

• Having the main thread reject APZ scrolling of the layout viewport in an overflow: hidden direction, as suggested in this comment. I've tried this locally and confirmed it also fixes the problem.
• Somehow dealing with the rounding error, such as by increasing the epsilon used for the comparison (though the discrepancy here is large, at 0.06 pixels it's 6 times larger than COORDINATE_EPSILON even), or trying to eliminate it closer to its source. I haven't investigated this direction.

I'd like to get a fix here uplifted to 67.

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

Here's what's happening on this page:

• The page has a content size of (1980.03, 1790.15) CSS pixels, and a layout viewport size of (1440, 1790.15). The page is overflow-x: hidden, so the layout viewport cannot be horizontally scrolled via user input.

Do we clamp the layout viewport height to the content height? It seems to me that we probably shouldn't (I'm imagining a simple page with a very short content height and a position:fixed; bottom:0 div, which I would expect to attach to the bottom of the layout viewport which should also be at the bottom of the screen, implying the layout viewport is taller than the content).

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

Do we clamp the layout viewport height to the content height? It seems to me that we probably shouldn't (I'm imagining a simple page with a very short content height and a position:fixed; bottom:0 div, which I would expect to attach to the bottom of the layout viewport which should also be at the bottom of the screen, implying the layout viewport is taller than the content).

The content size is a function of (and in particular no smaller than) the ICB size, which is the thing MobileViewportManager passes to ResizeReflow(). I believe MVM chooses the ICB size and zoom bounds in such a way that it's never smaller than the screen.

However, that might change with bug 1508177, at which point I think we'll need to allow a layout viewport height that's taller than the content height, like you say.

The fix is causing test failures because reftest-async-scroll works by forcing APZ to move the visual and layout viewports on an overflow: hidden page.

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

• Somehow dealing with the rounding error, such as by increasing the epsilon used for the comparison (though the discrepancy here is large, at 0.06 pixels it's 6 times larger than COORDINATE_EPSILON even), or trying to eliminate it closer to its source. I haven't investigated this direction.

I tracked the source of the discrepancy to the snapping done in GetScrolledRect().

Some places use the snapped scroll rect and others the raw (unsnapped) scrolled rect, and on this page there is a 3 app unit difference between the two at the minimum zoom level, giving rise to our ~0.05 pixel discrepancy.

Markus says fixing this is not easy, because the place that uses the unsnapped scrolled rect is involved in computing the minimum zoom level, but the snapping depends on the zoom level.

Depends on D24826

https://hg.mozilla.org/mozilla-central/rev/18fe70978426
https://hg.mozilla.org/mozilla-central/rev/3fcb0f47a5fa

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

I'd like to get a fix here uplifted to 67.

Please request uplift to beta when you get a chance. :)

Comment on attachment 9053466 [details]
Bug 1519007 - Do not allow APZ to move the layout viewport outside the scrollable rect. r=kats

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1423011
• User impact if declined: On some websites such as this one, after zooming in the user can get stuck in a state where they cannot scroll back to the beginning (left or top edge) of the page.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 4. The expected results are that you can scroll back to the left edge of the page.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): There is no automated test coverage because it's tricky to create the exact conditions that trigger this bug. However, the fix is low risk because it's enforcing an invariant that is already assumed to be holding.
• String changes made/needed:

(Uplift request applies to both patches.)

Hello,

I performed the test case posted in Comment 2 on a Samsung Galaxy S8+ (Android 8.0.0) and Huawei P9 Lite (Android 6.0).
On Nightly 68.0a1 (2019-04-05). When vising the page all panning and zooming actions can be performed smoothly.

Due to my findings, I'll mark this issue as verified in Firefox 68.

Comment on attachment 9053466 [details]
Bug 1519007 - Do not allow APZ to move the layout viewport outside the scrollable rect. r=kats

Fix for a correctness issue which can be perceived as a revent regression, verified by QA on nightly, uplift accepted for 67 beta, thanks.

https://hg.mozilla.org/releases/mozilla-beta/rev/b71adadd3a3c
https://hg.mozilla.org/releases/mozilla-beta/rev/0d43ca71665f

Hello,

I performed the test case posted in Comment 2 on a Samsung Galaxy S8+ (Android 8.0.0) and Huawei P9 Lite (Android 6.0).
On Beta 67.0b11. The issue does no longer occur on the page in question.

Due to my findings, I'll mark this issue as verified in Firefox 67.