1502614 - Autoscroll stops on www.reddit.com

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Comment 1

[Alice0775 White]

Steps To Reproduce:
1. Open https://www.reddit.com/r/firefox/
2. Start autoscroll and scroll to bottom

Actual Results:
 Sometimes, Auto-scroll stops unintentionally.(autoscroll marker disappears)

Expected Results:
  Auto-scroll should not stop

Comment 2

[Alice0775 White]

Attachment: screencast

Time:
approx. 0:03 Open the page
approx. 0:14 Autoscroll start
approx. 0:20 Autoscroll unexpectedly stopped

Comment 3

[Alice0775 White]

Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6a1f5c1d971dbee67ba6eec7ead7902351ddca2&tochange=8febdfacc7160c9ba90ba480e8bf5cd174e4fcab

Suspect: 	7f118ff40f8f	Botond Ballo — Bug 1390247 - Enable APZ autoscrolling on Nightly builds. r=kats

Comment 4

[Alice0775 White]

and Regression window with forcibly enabled apz.autoscroll.enabled = true:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=52285ea5e54c73d3ed824544cef2ee3f195f05e6&tochange=4c7317211990d6cf156c103a73a5b3ec41f2dd4d

Suspect: 7e558ba2e9a6	Botond Ballo — Bug 1385468 - Notify browser.xml when APZ cancels an AutoscrollAnimation. r=kats

@:botond,
Your patch seems to be root cause the problem. Could you please look into this?

Comment 5

[Alice0775 White]

Attachment: screenshot twitter

twitter.com is also affected

Comment 6

[Botond Ballo [:botond]]

Looks like we get a main-thread scroll offset update when we reach the bottom and the site adds more content, and that cancels the APZ autoscroll animation.

Coincidentally enough, bug 1502059 might fix this if we end up taking the autoscroll-related part of that patch.

Comment 7

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #6)
> Looks like we get a main-thread scroll offset update when we reach the
> bottom and the site adds more content, and that cancels the APZ autoscroll
> animation.
> 
> Coincidentally enough, bug 1502059 might fix this

Maybe not. The scroll offset we get from the main thread in this case is not relative.

Comment 8

[Botond Ballo [:botond]]

The source of the main thread scroll update is ScrollFrameHelper::ReflowFinished():

https://searchfox.org/mozilla-central/rev/8848b9741fc4ee4e9bc3ae83ea0fc048da39979f/layout/generic/nsGfxScrollFrame.cpp#5729

Comment 9

[Botond Ballo [:botond]]

That line is supposed to be a no-op most of the time (it's essentially doing "scroll to the current scroll position"), but it does clamp the scroll position to the scroll range, and so might change the scroll position if the scroll range has shrunk. I guess the scroll range can shrink temporarily during reflow, even if conceptually the page size is growing?

Anyways, the possible fixes are:

 1) Land bug 1502059, including the change to AutoscrollAnimation
    *and* make (at least) this internal scroll update "relative".

 2) Try to keep AutoscrollAnimation going even when receiving a
    non-relative scroll update.

    There's nothing technically preventing us from doing this,
    as AutoscrollAnimation works based on the locations of the
    cursor and and the autoscroll anchor relative to the screen,
    and neither of those is affected by the page scroll offset.
    It's just a matter of whether it's the UX we want.

Comment 10

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #9)
>     It's just a matter of whether it's the UX we want.

I guess it makes sense to preserve the UX we had with main-thread autoscroll.

I just tested the behaviour of that, and it looks like it keeps the autoscroll animation going even with conceptually non-relative updates (e.g. window.scrollTo()).

==> I would go with approach #2.

Comment 11

[Botond Ballo [:botond]]

I'll write a fix for this.

I'll wait for bug 1502059 to land first, as it will be touching some of the same code.

Comment 12

[Botond Ballo [:botond]]

Attachment: Bug 1502614 - Continue autoscroll animation after any main-thread scroll update (absolute or relative). r?rhunt

This matches the main thread behaviour prior to implementing APZ autoscrolling.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80060f3827bb
Continue autoscroll animation after any main-thread scroll update (absolute or relative). r=rhunt

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/80060f3827bb

Comment 15

[Cristian Baica [:cbaica], Release Desktop QA]

I have managed to reproduce the issue using Fx65.0a1 buildID: 20181027220121.

The issue is verified fixed using Fx65.0b7 and Fx66.0a1 on Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.10.4. The autoscroll no longer stops.