Closed Bug 1443231 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

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

Attachments

(1 file, 2 obsolete files)

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
Depends on: 1457743
You need to log in before you can comment on or make changes to this bug.