OnScaleEnd() leaves APZC in pinching state if we don't trigger a momentum scroll

RESOLVED FIXED in Firefox 60

Status

()

defect
P3
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: botond, Assigned: botond)

Tracking

({regression})

Trunk
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox58 wontfix, firefox59 wontfix, firefox60 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment, 2 obsolete attachments)

In bug 1180799, we made some modifications to AsyncPanZoomController::OnScaleEnd() to support momentum scrolling after a two-finger pan.

As part of that change, we removed the SetState(NOTHING) call in that function, and didn't replace it with any other SetState() calls, except in the case where the trigger the momentum scrolling. As a result, if we don't trigger momentum scrolling, the APZC remains in the PINCHING state.

This is undesirable; after completion of a gesture and associated movement / animation, the APZC should be in the NOTHING state.
Comment on attachment 8956160 [details]
Bug 1443231 - Ensure we are in the NOTHING state after a pinch gesture, if both fingers are lifted and no animation is triggered.

https://reviewboard.mozilla.org/r/225078/#review231036

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1598
(Diff revision 1)
>          if (HasReadyTouchBlock()) {
>            return HandleEndOfPan();
>          }
>        }
>      }
> +    SetState(NOTHING);

We probably don't want to do this if we enter the ScrollSnap() call on line 1586 and trigger a smooth scroll animation to the snap point.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> We probably don't want to do this if we enter the ScrollSnap() call on line
> 1586 and trigger a smooth scroll animation to the snap point.

Good point! Fixed and test added.
Comment on attachment 8956160 [details]
Bug 1443231 - Ensure we are in the NOTHING state after a pinch gesture, if both fingers are lifted and no animation is triggered.

https://reviewboard.mozilla.org/r/225078/#review231050

Nice, thanks!
Attachment #8956160 - Flags: review?(bugmail) → review+
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0306a56960c
Ensure we are in the NOTHING state after a pinch gesture, if both fingers are lifted and no animation is triggered. r=kats
Attachment #8956160 - Attachment is obsolete: true
Comment on attachment 8956224 [details]
Bug 1443231 - Follow-up to fix warning-as-error on Windows (CLOSED TREE).

https://reviewboard.mozilla.org/r/225136/#review231104
Attachment #8956224 - Flags: review+
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d6dc27d428c
Follow-up to fix warning-as-error on Windows (CLOSED TREE). r=botond
Attachment #8956224 - Attachment is obsolete: true
Comment on attachment 8956235 [details]
Bug 1443231 - Another follow-up to fix a debug assertion in the gtest.

https://reviewboard.mozilla.org/r/225142/#review231108
Attachment #8956235 - Flags: review?(botond) → review+
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cfe4f208baac
Another follow-up to fix a debug assertion in the gtest. r=botond
You need to log in before you can comment on or make changes to this bug.