1701738 - Improve overscroll experience on pages that dynamically load more content at the bottom

Status: VERIFIED FIXED

(Core :: Panning and Zooming, task, P2)

Description

[Markus Stange [:mstange]]

Steps to reproduce:

1. Be on macOS and enable apz.overscroll.enabled.
2. Go to https://firefox-source-docs.mozilla.org/search.html?q=using+c%2B%2B&check_keywords=yes&area=default# and scroll down to the bottom.
3. As more content loads, keep scrolling down.

Expected results:
It should be easy to keep tracking the bottom of the page.

Actual results:
Overscroll rubberbanding keeps bouncing upwards even as more content is already displayed below.

Comment 1

[Markus Stange [:mstange]]

If the scrollable size of the page changes, and we're in overscroll at the bottom or right edge, we should cancel the overscroll animation. And we should allow momentum scroll events to cause more scrolling, even if the momentum is from the same scroll gesture that initially hit overscroll.

Comment 2

[Botond Ballo [:botond]]

(I'm tentatively marking some incoming overscroll bugs as marking overscroll-mvp, but we can discuss and re-prioritize as needed.)

Comment 3

[Botond Ballo [:botond]]

Dropping as release blocker per discussion with Martin, though this is definitely one of the higher priority issues I'd like to get to, even if it only makes 90.

Comment 4

[Timothy Nikkel (:tnikkel)]

I ran into the following issue scrolling my strava.com feed (you need to log in and follow a bunch of people to get a useful feed, I can create a test account if needed): scroll down until the page needs to load more content, page loads more content (as you can tell by the scroll bar) try to scroll down, it'll scroll down about half a page (there is much more than this available, probably at least 6x more) and then hit an overscroll bounce, keep trying to scroll down, keep hitting overscroll bounce but scrolling down about half a page, repeat, I can get it to keep triggering the whole way until I get to the bottom and it loads more content. It's a very bad experience. Is that covered by this bug? Or should I file a new bug? In either case, I think this bug on strava should block release.

Comment 5

[Botond Ballo [:botond]]

Based on your description, the symptoms sound similar to comment 0. Does the behaviour on the site from comment 0 feel similar to you?

Comment 6

[Timothy Nikkel (:tnikkel)]

No, strava feels much worse. The comment 0 site I can maybe get to bounce back once while it feels like there is more content to scroll. On strava I can get it to bounce back continually (about half a page lower each time even though new content has not loaded since the last bounce), until I actually run out of content again. And it came up during actual use, not when I was trying to look for that specific bug. Every time I try to scroll when I'm this state results in a bounce back, until I got the bottom and more content loads. It probably requires the user to keep trying to scroll and not pause in between scroll attempts, but that is the natural thing to do in that situation.

Comment 7

[Botond Ballo [:botond]]

Ok, interesting. Maybe there's something else going on there.

(In reply to Timothy Nikkel (:tnikkel) from comment #4)

I can create a test account if needed

That would be helpful.

Comment 8

[Timothy Nikkel (:tnikkel)]

I sent you a test account login to your mozilla.com account.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

I've tracked down the strava case, when new contents were appended the scrollable rect for the target APZC in question gets expanded, but we don't clobber the overscroll amount (which is stored in each Axis class), then we start a new OverscrollAnimation in OnPanEnd.

Calling ClearOverscroll() in AsyncPanZoomController::NotifyLayersUpdated if either scrollOffsetUpdated or needToReclampScroll is true seems to fix the problem.

Comment 10

[Botond Ballo [:botond]]

It looks like a simple fix (along the lines of comment 9) significantly improves the behaviour on both Strava and the site for comment 0 (and likely dynamically loading sites in general).

So, let's land the simple fix in advance of the release. If additional improvements are desired, we can explore those post-release.

Comment 11

[Botond Ballo [:botond]]

Attachment: Bug 1701738 - Cancel overscroll animation if scroll range changes. r=hiro

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56d4e0c096c7
Cancel overscroll animation if scroll range changes. r=hiro

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/56d4e0c096c7

Comment 14

[Botond Ballo [:botond]]

Comment on attachment 9218559 [details]
Bug 1701738 - Cancel overscroll animation if scroll range changes. r=hiro

Beta/Release Uplift Approval Request

• User impact if declined: [Needed for MR1 / Proton] When reaching the bottom of a page that dynamically loads more content, the overscroll effect interacts poorly with the dynamic loading behaviour, creating a poor user experience (user has to wait for snap-back animation to complete before they can see the new content)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1704659, Bug 1704410
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Marking this one as Medium because it's changing code (AsyncPanZoomController::NotifyLayersUpdated) that is fairly tricky. However, the effects should still be limited to situations where the user is panned into overscroll or in a snap-back animation.
• String changes made/needed:

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9218559 [details]
Bug 1701738 - Cancel overscroll animation if scroll range changes. r=hiro

Approved for 89 beta 7, thanks.

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/76f6a932dd0d

Comment 17

[Andrei Purice]

Seems like this issue is still reproducible on Firefox Beta 89.0b8 (20210504185920) on Facebook (mostly visible), reddit and the reporter's link at least.
For more info check this video : https://drive.google.com/file/d/1vfJK3vTnUpGhVS9-I6FUiqrUP9jv5AZH/view?usp=sharing

Comment 18

[Timothy Nikkel (:tnikkel)]

I don't think I see any problem in the video. It looks like you only get the overscroll bounce when you are actually at the bottom of the page while we are waiting for new content to load. Can you describe in more detail what the problem is that you are seeing/showing in the video?

Comment 19

[Andrei Purice]

As per Actual result from Comment 0 which states (Overscroll rubberbanding keeps bouncing upwards even as more content is already displayed below) we saw the same bounce back when reaching the end of the page as the new content was loaded. Due to the patch named "Cancel overscroll animation if scroll range changes" we assumed that the bounce-back animation shouldn't be present and that's why we raised this concern.
If this is indeed intended apologies for the misunderstanding and please let us know so QA can confirm this as verified-fixed.

Comment 20

[Botond Ballo [:botond]]

I agree that the behaviour shown in the video looks to be expected. We don't know in advance that the page will load additional content, its only when the new content arrives that we can cancel any overscroll animation that may be in progress and start showing the new content instead.

The previous behaviour was that, if an animation was in progress when new content arrived, we would wait for the animation to finish, and only on the next gesture continue scrolling to show the new content.

Comment 21

[Andrei Purice]

Thank you for the confirmation.
Marking this as Verified fixed on the latest Firefox Beta 89.0b8 (20210504185920) and Nightly 90.0a1 (20210505215208).