Closed Bug 1700215 Opened 4 years ago Closed 3 years ago

Vertical momentum scroll is impossible (bounces back) while the page is in horizontal overscroll

Categories

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

defect

Tracking

()

VERIFIED FIXED
89 Branch
Tracking Status
firefox89 --- verified

People

(Reporter: mstange, Assigned: hiro)

References

Details

Attachments

(6 files)

Steps to reproduce:

  1. Use a build that includes the fixes for bug 1697679 and has apz.overscroll.enabled set to true.
  2. Go to a page that is scrollable both vertically and horizontally, for example https://planet.mozilla.org/ . (This step will be unnecessary once bug 1686151 is fixed.)
  3. Scroll up and to the left into overscroll, so that you are in diagonal overscroll at the top left corner. Don't lift your finger yet.
  4. From the diagonal overscroll position, fling downwards.
  5. For added fun, keep triggering additional vertical flings before the overscroll animation fully completes.

Expected results:
The vertical fling should work. During the fling, simultaneously, the horizontal overscroll should be resolved.

Actual results:
The vertical scroll is aborted as soon the finger is lifted, and the page "bounces back", i.e. scrolls back up.
If another fling is performed before the overscroll animation has fully completed, the upwards bounce happens again.

Here's another way to get into the "perpetual bounce" state:

  1. On https://planet.mozilla.org/ , scroll a bit to the right.
  2. Do a horizontal fling back to the left edge.
  3. As the page is bouncing from the left edge, try to start a vertical fling.

And yet another:

  1. Do a vertical fling on a Magic Mouse and unintentionally go diagonal towards the end of the motion. (This seems to be a easier to do with the mouse than with two-finger touchpad scrolling.)

Do the "expected results" happen in both Chrome and Safari?

Chrome: yes and yes
Safari: yes and no; The fling happens, but the overscroll is only resolved at the end of the fling.

Neither browser gets into a state where momentum scroll is blocked.

Attached video video recording of perpetual bounce state (deleted) β€”

Thanks. Sounds like we should at least partially fix this (i.e. match Safari behaviour) for the MVP.

Severity: -- → S2
Priority: -- → P2
Attached patch A patch to mimic the Safari's behavior (deleted) β€” β€” Splinter Review

I think I could manage to mimic the Safari's behavior without introducing dominant axis scrolling. Though both Chrome and Safari have the dominant axis scrolling, which is that any diagonal scrolling is going to be a vertical or horizontal scroll, so it will be a user visible behavior change, I think we should do the change in an individual bug. (the patch Markus wrote before includes the change though)

The way I did is;

  1. Stop starting an overscroll animation if mFollowedByMomentum flag is false (I know Markus said the flag is not accurate, but haven't found any other solutions yet)
  2. Stop starting any overscroll animation on pan momentum if the momentum is opposed to the direction of overscrolling which has already happened
  3. Start an overscroll animation on pan momentum end if there is still overscrolled area

I understand these are not what Markus has been suggesting..

I agree about making the "dominant axis scrolling" change in a separate bug. And I believe it's a valuable change to make on its own. Maybe it can be implemented as an axis lock mode.

For the overscroll behavior, if matching Safari is what's easiest, let's do that. But if doing "the right thing" is easier, let's do the right thing.
In my opinion, treating the two axes independently is the right thing because it makes more sense from a physics perspective.

Assigning to me. Though I don't yet have any clear vision how to achieve it, what I have in my mind is doing more work in OverscrollAnimation class so that we can easily have different behaviors such as Chrome's or Safari's depending on platforms (or on a pref value) with inherited classes.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)

so that we can easily have different behaviors such as Chrome's or Safari's depending on platforms

Why?

I consider Safari's behavior on macOS a bug. We shouldn't try to match it. It is acceptable to match it as an intermediate solution if it is easier to implement than proper separate-axis overscrolling, but it's not a goal of itself.

I have a couple of thoughts on what we could try here:

  1. It would be interesting to understand (and probably good to fix) the issue that causes the "bounce at the wrong end" that Markus observed (which doesn't require a page that's scrollable on both axes). (I am referring to the issue discussed on Matrix starting from here).

  2. To address the main issue here, would it make sense to not start an overscroll animation if we have fling momentum in a direction that relieves the overscroll? Instead, we could just let the fling momentum itself relieve the overscroll (and continue flinging if there is velocity left).

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

  1. To address the main issue here, would it make sense to not start an overscroll animation if we have fling momentum in a direction that relieves the overscroll? Instead, we could just let the fling momentum itself relieve the overscroll (and continue flinging if there is velocity left).

I guess that sounds a lot like steps 2-3 from comment 6. (I'm not sure about the motivation of step 1 in that comment.)

Why have we discarded this approach?

Though to be honest I am not sure the Safari's overscroll behavior is a bug or not because I am seeing the same behavior on other native applications (e.g. Pages)

Anyway, I've written patches for mimicking the Chrome behavior but there remains an issue I am aware of and I haven't solved. But the issue is less noticeable so I've filed bug 1702949 for the issue, and I am going to handle the issue in the bug once after this bug was fixed since this bug is more noticeable and should be fixed as soon as possible.

And I am also concerned that bug 1702949 will likely cause regressions so it would be nice to be handled in a different bug.

Boolean literals in call sites make it hard to tell what the value means.

Depends on D110791

This is a prerequisite change to keep momentum scrolls happening on the other
axis where no overscroll animation is running.

Depends on D110792

This change makes us match Chrome's overscroll behavior (i.e. restoring
overscroll state on an axis while scrolling by pan momentum keeps going on the
other axis).

Basic mechanisms to do the behavior are;

a) if there is an on-going overscroll animation on an axis and the overscroll
direction is opposed to the momentum direction, stop the overscroll animation
immediately and keep the momentum scroll (or overscroll and scroll if the
current position is in overscroll gutter).
b) if there is an on-going overscroll animation on an axis and the overscroll
direction is same as the momentum direction, drop the momentum and keep
the animation overscrolling.
c) if there is an on-going overscroll animation on an axis and no overscroll
animation on the other axis and if a pan momentum event overscrolls on
the other axis, start a new overscroll animation on the axis.

With this change there still remains a (relatively) less noticeable issue that
pan momentum scrolling on an axis stops immediately after the other axis
overscroll animation stopped. For example, given that there is a horizontal
overscroll animation and vertical pan momentums keep going, subsequent vertical
pan moments after the horizontal overscroll animation finished will be discarded
since we do set the APZC state to NOTHING in
AsyncPanZoomController::UpdateAnimation(). To fix this issue we need to handle
OVERSCROLL_ANIMATION separately from the others, in the example case, once after
the overscroll animation finished, the state should be MOMENTUMPAN, thus we can
keep consuming momentum events as scroll. That said, the fix will not be easy,
because there are also cumbersome cases where we don't want to keep consuming
the events, one of the cases is b) in the basic mechanisms above, in the case
we do want to discard incoming momentum events after the overscroll animation
finished if the incoming momentum events are in the initial overscroll gutter,
IF the incoming momentum events are in the another edge of the overscroll
gutter, we have to re-initiate a new overscroll animation in the other side
gutter. We will handle these cases in a followup bug (bug 1702949).

Depends on D110793

Blocks: overscroll-nightly
No longer blocks: overscroll-release
Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f962368b563b
Factorout GetDisplacementsForPanGesture. r=botond
https://hg.mozilla.org/integration/autoland/rev/302d1ffd7b29
Introduce FingersOnTouchpad enum to represent whether fingers are still on touchpad. r=botond
https://hg.mozilla.org/integration/autoland/rev/a20a1507ea32
Generate OverscrollAnimation only if it's overscrolled on the axis. r=botond
https://hg.mozilla.org/integration/autoland/rev/15023df770af
Make OverscrollAnimation coexist with momentum scrolling in the other axis. r=botond
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch

Verified fixed on Nightly 90.0a1 (2021-04-21) (20210421095627) on MacOS 11.2.3.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: