Closed Bug 1502614 Opened 1 year ago Closed 1 year ago

Autoscroll stops on www.reddit.com

Categories

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

57 Branch
x86_64
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla65
Tracking Status
firefox-esr60 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- verified

People

(Reporter: alice0775, Assigned: botond)

References

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(3 files)

No description provided.
Summary: Autoscroll occationally stops → Autoscroll stops on www.reddit.com
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
Severity: normal → minor
Keywords: regression
Version: Trunk → 57 Branch
Attached video screencast
Time:
approx. 0:03 Open the page
approx. 0:14 Autoscroll start
approx. 0:20 Autoscroll unexpectedly stopped
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
Blocks: 1390247
Component: Untriaged → Panning and Zooming
Product: Firefox → Core
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?
Blocks: 1385468
Flags: needinfo?(botond)
Attached video screenshot twitter
twitter.com is also affected
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.
Flags: needinfo?(botond)
See Also: → 1502059
(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.
The source of the main thread scroll update is ScrollFrameHelper::ReflowFinished():

https://searchfox.org/mozilla-central/rev/8848b9741fc4ee4e9bc3ae83ea0fc048da39979f/layout/generic/nsGfxScrollFrame.cpp#5729
Priority: -- → P3
Whiteboard: [gfx-noted]
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.
(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.
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.
Assignee: nobody → botond
Depends on: 1502059
See Also: 1502059
This matches the main thread behaviour prior to implementing APZ autoscrolling.
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80060f3827bb
Continue autoscroll animation after any main-thread scroll update (absolute or relative). r=rhunt
https://hg.mozilla.org/mozilla-central/rev/80060f3827bb
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.