1589046 - [macOS] Jumpy scrolling when space is pressed in a small window

Status: VERIFIED FIXED

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

Description

[Catalin Sasca, Desktop QA [:csasca]]

Attachment: Jumpy scrollbar.mov

Affected versions

• Firefox 68.1.0esr
• Firefox 70.0
• Firefox 71.0b1 DevEd
• Firefox 71.0a1

Affected platforms

• macOS 10.14/10.15

Steps to reproduce

1. Launch Firefox.
2. Access a link of choice. For example link.
3. Adjust the window dimensions to make the vertical scrollbar appear.
4. Using the touchpad, scroll to the top of the page, then press space.

Expected result

• The page is scrolled down.

Actual result

• The page scrolls down, then jumps back up.

Regression range
I will see for a regression asap.

Additional notes

• The actual issue can be seen in the attachment.
• Issue is not reproducible on Firefox 60.9.0esr.
• The issue was reproducible on macbook pros with touchpad, as it requires to scroll with it to the top of the page in order to trigger the issue. I haven't tried on a iMac with a mouse and scroll.
• Couldn't reproduce on Windows 10 nor Ubuntu 16.04.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 2

[Botond Ballo [:botond]]

(In reply to Catalin Sasca, QA [:csasca] from comment #0)

Actual result

• The page scrolls down, then jumps back up.

The first thing I would suggest checking is whether the problem reproduces with layout.css.scroll-anchoring.enabled=false. This "jumping after scrolling" symptom is often a result of scroll anchoring.

Comment 3

[Catalin Sasca, Desktop QA [:csasca]]

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

(In reply to Catalin Sasca, QA [:csasca] from comment #0)

Actual result

• The page scrolls down, then jumps back up.

The first thing I would suggest checking is whether the problem reproduces with layout.css.scroll-anchoring.enabled=false. This "jumping after scrolling" symptom is often a result of scroll anchoring.

Tried again with layout.css.scroll-anchoring.enabled=false. The issue is still present.

Comment 4

[Catalin Sasca, Desktop QA [:csasca]]

Here's the regression for the issue:
Last good revision: e73aadc3a039337689f63caedf78e7c31cacda0a
First bad revision: 2b0f0d220a7cc3d6262706c160cc1900148dbe79
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e73aadc3a039337689f63caedf78e7c31cacda0a&tochange=2b0f0d220a7cc3d6262706c160cc1900148dbe79

Comment 5

[Botond Ballo [:botond]]

(In reply to Catalin Sasca, QA [:csasca] from comment #4)

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e73aadc3a039337689f63caedf78e7c31cacda0a&tochange=2b0f0d220a7cc3d6262706c160cc1900148dbe79

Points to bug 1474196.

Comment 6

[Botond Ballo [:botond]]

So the change made in bug 1474196 is that if we are in a momentum scroll (fingers lifted from the touchpad but still scrolling), receiving a scroll update from the main thread no longer interrupts the momentum scroll.

In that sense, this is more or less expected behaviour (we have upward momentum, the spacebar does not interrupt it, so after the spacebar scrolls us down, the upward momentum continues scrolling us up).

That said, the change in bug 1474196 was made with JS-originated scrolling in mind. Perhaps we want to distinguish between that and scrolling originated by user input such as pressing the spacebar, and still have the latter category cancel momentum.

Comment 7

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

P3, macos only, unassigned and we shipped several releases with this bug so wontfix 71.

Comment 8

[Catalin Sasca, Desktop QA [:csasca]]

Reproduced the issue on bbc.com, Firefox 73.0b5, macOS 10.15.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

I have a fix for this that I wrote last month and just got around to cleaning up.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=5bc41b3d71409a01de780eea04736ab156f69ada

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Hm, based on the gtest failure I guess what I wrote isn't actually what we want, although it fixes this particular issue.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

After thinking about this some more, I'm not sure this is even really a bug. Consider the case where the direction of trackpad scrolling is the same as the spacebar scrolling - i.e. you're using the trackpad to fling your way down the page and also mashing the spacebar to scroll at the same time. In this case, I would expect the two scroll methods to combine to scroll even faster. Or in the case of a long page, scrolling up with trackpad while also mashing spacebar to scroll down - again I would expect the two scroll methods to combine without stepping on each others' toes.

I think with the STR in comment 0 the unexpectedness actually arises from the fact that the momentum scroll "keeps going" even though we've reached the top of the page, so when you spacebar down you don't expect that momentum to keep carrying on. This is a similar class of bug as bug 767197 and friends. Maybe the right fix here is to cancel the momentum scrolling once we reach the end of the page.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=aa931ce7c4e670c28fec51c5d0fcb42a4e3f3357

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1589046 - Propagate the return value from AttemptScroll to DispatchScroll and CallDispatchScroll. r?botond

No functional changes here.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1589046 - Terminate momentum panning once we reach a point where there is uncomsumed scroll. r?botond

This prevents OS-produced momentum panning events from having an effect after
the user sees the momentum panning "end" due to scrolling as far as possible.

Depends on D69623

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b87c18be6ec6
Propagate the return value from AttemptScroll to DispatchScroll and CallDispatchScroll. r=botond
https://hg.mozilla.org/integration/autoland/rev/6aec54d11cbf
Terminate momentum panning once we reach a point where there is uncomsumed scroll. r=botond

Comment 17

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/b87c18be6ec6
https://hg.mozilla.org/mozilla-central/rev/6aec54d11cbf

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:kats, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Not worth uplifting I think.

Comment 20

[Bogdan Maris, Desktop QA]

Reproduced the initial issue using an old Nightly from 2019-10-16, verified that this is fixed using latest Firefox Beta 77.0b5 on macOS 10.15.5 Beta.