Closed Bug 1713403 Opened 3 years ago Closed 2 years ago

Implement dominant axis scrolling on Mac

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: botond, Assigned: dlrobertson)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

Attachments

(3 files)

Safari has a behaviour called "dominant axis scrolling", where touchpad scrolls never result in diagonal scrolling: the smaller of the x- and y-axis deltas is dropped instead.

In bug 1704080, we attempted to match another behaviour of Safari (that the rubber-band overscroll effect can be activated even for an axis along which the page is not scrollable), and discovered that this results in a poor user experience without dominant axis scrolling. Bug 1704080 was subsequently backed out for this reason (as of this writing, the backout patch is under review).

This bug tracks implementation of dominant axis scrolling in Firefox. To minimize the amount of behaviour change relative to previous versions of Firefox, we may start by having this behaviour during overscrolling only (which would be enough to unblock a re-landing of bug 1704080), and consider extending it to regular scrolling at a later time.

Just as a note, I tested on my mac and dominant axis scrolling seems not apply to the mouse wheel.

Assignee: nobody → drobertson

When panning and both directions have a delta, zero out the smaller to
implement dominant axis scrolling.

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

To minimize the amount of behaviour change relative to previous versions of Firefox, we may start by having this behaviour during overscrolling only (which would be enough to unblock a re-landing of bug 1704080), and consider extending it to regular scrolling at a later time.

Now that I'm thinking about this in more detail, I wonder if "having this behaviour during overscrolling only" makes sense from a UX point of view.

We'd have to flesh out a bit what it means. Let's say you transition from a regular scroll into overscroll, and dominant axis scrolling kicks in. What happens if you pan back out of overscroll in the same gesture? Are you now stuck with dominant-axis scrolling for the remainder of the gesture, or do we revert to potentially-diagonal scrolling as soon as overscroll is relieved?

There's also the fact that the boundary between regular scroll and overscroll could happen in the middle of a single pan event's displacement. Is it important that dominant axis scrolling starts precisely at the boundary, or is it good enough if it kicks in at the next event?

Markus, I'm curious if you have thoughts on this.

Flags: needinfo?(mstange.moz)

"Having this behaviour during overscrolling only" sounds complicated and messy. I think Dan's patch is the right way to go: Just do dominant-axis scrolling everywhere all the time. It's what all the other browsers do on macOS, and it is desirable anyway. For example, in Firefox, when I scroll down on pages that have a width of 110% (intentionally or not), the page often shifts horizontally during vertical scrolling because I happen to scroll slightly diagonally to the bottom right. This gives scrolling a bit of an unstable feel. Other browsers don't have that problem, because of dominant axis scrolling.

We just need to make sure that we still report diagonal deltas on the DOM wheel events we send to web pages, so that web pages which desire diagonal scrolling can still implement this behavior, e.g. for maps. (Though all map sites I can find are using wheel events only for zooming, and not for panning.)

Flags: needinfo?(mstange.moz)

We just need to make sure that we still report diagonal deltas on the DOM wheel events we send to web pages, so that web pages which desire diagonal scrolling can still implement this behavior, e.g. for maps. (Though all map sites I can find are using wheel events only for zooming, and not for panning.)

Do you mean strictly mouse wheel events here? Or would a touchpad pan event be included in this?

(In reply to Markus Stange [:mstange] from comment #4)

For example, in Firefox, when I scroll down on pages that have a width of 110% (intentionally or not), the page often shifts horizontally during vertical scrolling because I happen to scroll slightly diagonally to the bottom right. This gives scrolling a bit of an unstable feel. Other browsers don't have that problem, because of dominant axis scrolling.

I wonder why axis locking (enabled on Mac in bug 1467380) is not preventing that.

(In reply to Dan Robertson (:dlrobertson) from comment #5)

We just need to make sure that we still report diagonal deltas on the DOM wheel events we send to web pages, so that web pages which desire diagonal scrolling can still implement this behavior, e.g. for maps. (Though all map sites I can find are using wheel events only for zooming, and not for panning.)

Do you mean strictly mouse wheel events here? Or would a touchpad pan event be included in this?

The web platform doesn't have a distinct event type for touchpad pans, at the DOM level they get turned into the same event type (WheelEvent) as mouse wheel events.

Come to think of it, dominant axis scrolling is just a special case of axis locking that should be achievable using our existing prefs, specifically apz.axis_lock.mode = 1 (that's "STANDARD" mode, which doesn't allow the lock to be broken), and apz.axis_lock.lock_angle = [45 degrees in radians] (which will lock us to the closer axis for any angle of scroll).

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

I wonder why axis locking (enabled on Mac in bug 1467380) is not preventing that.

Good question. Maybe because it's not locking aggressively enough. I just tried to reproduce it, and it mostly only happens on very "soft" flings. It's possible that during these flings the non-momentum part of the pan is axis-aligned (maybe just one or two events) and then the diagonal deltas start after the fingers have left the touchpad. I'm not sure if axis locking treats momentum and non-momentum parts of the pan differently.

I also noticed that during faster diagonal-but-axis-locked-vertical flings, once the fling reaches the top or bottom edge, the axis locking is broken and we scroll horizontally during the vertical rubber banding. So if we implement dominant axis scrolling as a form of axis locking, we should make sure that it doesn't have this property, because that's exactly what we're trying to avoid here.

(In reply to Markus Stange [:mstange] from comment #4)

"Having this behaviour during overscrolling only" sounds complicated and messy. I think Dan's patch is the right way to go: Just do dominant-axis scrolling everywhere all the time. It's what all the other browsers do on macOS, and it is desirable anyway.

In any case, you've convinced me that trying to do dominant axis scrolling during overscroll only would be too much complexity to be worth it. I'm revising the bug title accordingly.

My suggestion for the way forward here is:

  • Try to implement dominant scrolling by just adjusting the values of the mentioned prefs (the adjustments can be made in the file defining the prefs). The changes can be limited to Mac, and for now, probably Nightly only.
  • To the extent that the resulting behaviour falls short of what we'd like (e.g. overscroll not respecting axis locking), let's fix that. Quite possibly the fixes would be appropriate for all platforms / axis lock modes, but we can discuss that on a case by case basis (and if necessary add new prefs or even a new axis lock mode).
Blocks: apz-mac
Summary: Consider implementing dominant axis scrolling when overscrolling (perhaps for all scrolling) → Implement dominant axis scrolling on Mac

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

My suggestion for the way forward here is:

  • Try to implement dominant scrolling by just adjusting the values of the mentioned prefs (the adjustments can be made in the file defining the prefs). The changes can be limited to Mac, and for now, probably Nightly only.
  • To the extent that the resulting behaviour falls short of what we'd like (e.g. overscroll not respecting axis locking), let's fix that. Quite possibly the fixes would be appropriate for all platforms / axis lock modes, but we can discuss that on a case by case basis (and if necessary add new prefs or even a new axis lock mode).

This does seem like a good approach. I'm not familiar with axis locking and the various modes at the moment, but I'll explore some and ask questions.

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

  • To the extent that the resulting behaviour falls short of what we'd like (e.g. overscroll not respecting axis locking), let's fix that. Quite possibly the fixes would be appropriate for all platforms / axis lock modes, but we can discuss that on a case by case basis (and if necessary add new prefs or even a new axis lock mode).

I think we can get this to work with a preference, but I've certainly hit issues with apz.axis_lock.mode=1 and/or axis locking. Even when in state PANNIN_LOCKED_Y with the X axis locked, I still get a side of X axis scroll with my order of Y axis scroll.

(In reply to Markus Stange [:mstange] from comment #9)

I'm not sure if axis locking treats momentum and non-momentum parts of the pan differently.

There does seem to be something off here. Once the APZC transitions to a PANNING_MOMENTUM state the axis lock flags are removed for me. I'm not entirely sure what I'm looking at here, but some quality time in gdb should help a bit with that.

I think I understand why apz.axis_lock.mode=1 is only able to partially create a dominant axis mode. While we're in PANNING_LOCKED_<axis> we are indeed locked on the dominant axis and the intended behavior is seen. Once the pan ends, we call EndTouch and the axis locks are cleared. When we eventually transition to PAN_MOMENTUM, we are then free to (and often do) scroll in both directions. I tried adding a MOMENTUM_LOCK mode to apz.axis_lock.mode (probably could fold this into STANDARD mode) which would not release the axis lock in the state transition. This works, but it has negative consequences for interrupting momentum scrolls when the user scrolls on the opposite axis. So I think we have two options:

  1. Alter the things that check if the axis islocked to ignore axis locks when in a momentum scroll
  2. Do something closer to the original patch, but limited on some apz.axis_lock.mode variant and only in PAN_MOMENTUM or overscroll

(In reply to Dan Robertson (:dlrobertson) from comment #13)

I tried adding a MOMENTUM_LOCK mode to apz.axis_lock.mode (probably could fold this into STANDARD mode) which would not release the axis lock in the state transition. This works, but it has negative consequences for interrupting momentum scrolls when the user scrolls on the opposite axis.

I assume this means that we want scrolling on the opposite axis to interrupt a momentum scroll, but this change is preventing that from happening?

  1. Alter the things that check if the axis islocked to ignore axis locks when in a momentum scroll

I'd like to understand better the mechanism by which the "failure to interrupt" (assuming I've got that right) occurs.

Conceptually, I feel like the fingers going down on the touchpad (i.e. a PANGESTURE_START or PANGESTURE_MAYSTART event) should break any existing axis lock, and that may be sufficient to address the issue.

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

(In reply to Dan Robertson (:dlrobertson) from comment #13)
I assume this means that we want scrolling on the opposite axis to interrupt a momentum scroll, but this change is preventing that from happening?

After the change, the interrupt does not happen with the same consistency as the other modes. This could be in implementation issue.

I'd like to understand better the mechanism by which the "failure to interrupt" (assuming I've got that right) occurs.

Conceptually, I feel like the fingers going down on the touchpad (i.e. a PANGESTURE_START or PANGESTURE_MAYSTART event) should break any existing axis lock, and that may be sufficient to address the issue.

You might be onto something! I think I misinterpreted what I was seeing. After viewing the logs, I'm semi-consistently seeing the parent APZC as the target for the gesture that should interrupt the momentum scroll. Also after viewing the logs it looks like the momentum animation is cancelled, so the issue is with the selection of the APZC.

Attachment #9274091 - Attachment description: Bug 1713403 - Implementing dominant axis scrolling when overscrolling. r=botond → Bug 1713403 - Implementing momentum locking mode. r=botond
Attachment #9274091 - Attachment description: Bug 1713403 - Implementing momentum locking mode. r=botond → Bug 1713403 - Implementing dominant axis scrolling on mac. r=botond

Just coming around to looking at this comment now:

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

Come to think of it, dominant axis scrolling is just a special case of axis locking that should be achievable using our existing prefs, specifically apz.axis_lock.mode = 1 (that's "STANDARD" mode, which doesn't allow the lock to be broken)

It seems to me that STANDARD mode is very different from the dominant axis scrolling in other macOS apps. Dominant axis scrolling re-evaluates the locked axis on every single event. If your events are (2, 1), (1, 3), (-5, 4), this should translate to (2, 0), (0, 3), (-5, 0). But STANDARD mode requires scrolling to stop before the axis can be switched.

(In reply to Markus Stange [:mstange] from comment #16)

Just coming around to looking at this comment now:

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

Come to think of it, dominant axis scrolling is just a special case of axis locking that should be achievable using our existing prefs, specifically apz.axis_lock.mode = 1 (that's "STANDARD" mode, which doesn't allow the lock to be broken)

It seems to me that STANDARD mode is very different from the dominant axis scrolling in other macOS apps. Dominant axis scrolling re-evaluates the locked axis on every single event. If your events are (2, 1), (1, 3), (-5, 4), this should translate to (2, 0), (0, 3), (-5, 0). But STANDARD mode requires scrolling to stop before the axis can be switched.

Great catch! Safari will indeed behave differently than what I currently have implemented. We could implement this kind of dominant axis locking with apz.axis_lock.mode set to STICKY, apz.axis_lock.lock_angle set to 45 degrees, and apz.axis_lock.breakout_angle set to 45 degrees. We'd also need to change the way the "axis break" is currently handled. We currently implement the axis break as PANNING_LOCKED_<axis> -> PANNING. We'd need axis break to be PANNING_LOCKED_<axis> -> PANNING_LOCKED_<other axis>. Basic tests seem to work, but I don't know if this would cause other things to break.

(In reply to Markus Stange [:mstange] from comment #16)

It seems to me that STANDARD mode is very different from the dominant axis scrolling in other macOS apps. Dominant axis scrolling re-evaluates the locked axis on every single event. If your events are (2, 1), (1, 3), (-5, 4), this should translate to (2, 0), (0, 3), (-5, 0). But STANDARD mode requires scrolling to stop before the axis can be switched.

Ah, I didn't realize that!

(In reply to Dan Robertson (:dlrobertson) from comment #17)

We could implement this kind of dominant axis locking with apz.axis_lock.mode set to STICKY, apz.axis_lock.lock_angle set to 45 degrees, and apz.axis_lock.breakout_angle set to 45 degrees. We'd also need to change the way the "axis break" is currently handled. We currently implement the axis break as PANNING_LOCKED_<axis> -> PANNING. We'd need axis break to be PANNING_LOCKED_<axis> -> PANNING_LOCKED_<other axis>. Basic tests seem to work, but I don't know if this would cause other things to break.

+1, this seems like a promising approach.

Attachment #9274091 - Attachment description: Bug 1713403 - Implementing dominant axis scrolling on mac. r=botond → Bug 1713403 - Implement axis locking in momentum scrolls on mac. r=botond

When in STICKY axis locking mode and the axis lock is broken, lock onto
the opposite axis, if the input angle is within the breakout_angle from
the opposite axis.

Depends on D144854

Implement dominant axis scrolling on mac by using the default axis
locking mode (sticky), setting the locking angle to 45 degrees, and
setting the breakout angle to 45 degrees.

Depends on D146609

Attachment #9274091 - Attachment description: Bug 1713403 - Implement axis locking in momentum scrolls on mac. r=botond → Bug 1713403 - Implement axis locking for PanGestureInput momentum scrolls. r=botond
Attachment #9276965 - Attachment description: Bug 1713403 - Implement domminant axis scrolling on mac. r=botond → Bug 1713403 - Implement dominant axis scrolling on mac. r=botond
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c7abfd8bc34c
Implement axis locking for PanGestureInput momentum scrolls. r=botond
https://hg.mozilla.org/integration/autoland/rev/bc3cfe267c74
Implement axis lock switching for the sticky axis lock mode. r=botond
https://hg.mozilla.org/integration/autoland/rev/bd50429be83e
Implement dominant axis scrolling on mac. r=botond
Blocks: 1770421
Blocks: 1770416

Dan, could you file a follow-up bug for letting dominant axis scrolling "ride the trains" (meaning progress from Nightly to Beta and Release) please?

Flags: needinfo?(drobertson)
Blocks: 1770515

+1 added a bug. Should the other axis locking issues block it?

Flags: needinfo?(drobertson)

Thanks!

(In reply to Dan Robertson (:dlrobertson) from comment #24)

Should the other axis locking issues block it?

Bug 1770416 doesn't need to since it will be a no-op on Mac.

For bug 1770421 let's have it block for now, if only as a means of tracking. When we approach a future merge date and decide if we want dominant axis scrolling to ride the trains with that merge, we can look through the open dependencies and see if we actually want them to block.

Depends on: 1773378
No longer depends on: 1773378
Regressions: 1771512
Regressions: 1773381
Regressions: 1789815
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: