- Fennec nightly - Load a zoomable page, e.g. the one in the URL field - Pan down some so the dynamic toolbar goes offscreen - Do a pinch-zoom in and quickly lift your fingers Expected results: page zooms in and that's it Actual: page zooms in, and when you lift your fingers it kicks off a fast fling that leaves you at the bottom or top of the page I can reproduce this fairly easily, happens maybe 50% of the time I try. I ran mozregression just to be sure and it came back with this range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=27311156637f9b5d4504373967e01c4241902ae7&tochange=c8198aa6e7677e90cc7f1e2df0a14a5cc2719055 which includes the new dynamic toolbar, so I'm fairly sure that's the culprit. However it might be an unintended interaction between the new dynamic toolbar and bug 1180799 which also landed recently and also touched relevant codepaths.

I'm pretty sure this is caused by the dynamic toolbar since when I reproduced it, if the toolbar is hidden, it becomes visible at the same time as the super fling. This implies to me there is something weird going on in the animator when the touch from the pinch is released.

Comment on attachment 8861574 [details] Bug 1358774 - Prevent dynamic toolbar from changing state when transitioning from multiple touch sources to a single touch source https://reviewboard.mozilla.org/r/133544/#review136992 ::: gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp:109 (Diff revision 1) > MultiTouchInput& multiTouch = aEvent.AsMultiTouchInput(); > ScreenIntCoord currentTouch = 0; > > + switch (multiTouch.mType) { > + case MultiTouchInput::MULTITOUCH_START: > + mControllerTouchCount++; It's possible for a single MULTITOUCH_START event to be fired when two fingers are placed on the screen at the same time, and for a single MULTITOUCH_END to be fired when two fingers are removed at the same time. So, incrementing and decrementing the touch count is not sufficient. Instead, you can test |multiTouch.mTouches.Length()|. For MULTITOUCH_START, that will be the total number of touches that are now active, including the new one(s). For MULTITOUCH_END, that will be the total number of touches that are being removed.

Comment on attachment 8861574 [details] Bug 1358774 - Prevent dynamic toolbar from changing state when transitioning from multiple touch sources to a single touch source https://reviewboard.mozilla.org/r/133544/#review137404 The updated handling of the touch count looks fine, but what's with the added TOOLBAR_SNAPSHOT_FAILED stuff? Is that related to the rest of the patch?

(In reply to Botond Ballo [:botond] from comment #5) > Comment on attachment 8861574 [details] > Bug 1358774 - Prevent dynamic toolbar from changing state when transitioning > from multiple touch sources to a single touch source > > https://reviewboard.mozilla.org/r/133544/#review137404 > > The updated handling of the touch count looks fine, but what's with the > added TOOLBAR_SNAPSHOT_FAILED stuff? Is that related to the rest of the > patch? That's code from a different patch that has landed. You need to move the diff slider back to zero as MozReview doesn't seem to handle rebases very well.

Comment on attachment 8861574 [details] Bug 1358774 - Prevent dynamic toolbar from changing state when transitioning from multiple touch sources to a single touch source https://reviewboard.mozilla.org/r/133544/#review137404 I meant move the left diff slider to "orig" not zero to see the actual patch diff.

Comment on attachment 8861574 [details] Bug 1358774 - Prevent dynamic toolbar from changing state when transitioning from multiple touch sources to a single touch source https://reviewboard.mozilla.org/r/133544/#review137466 Ah, my bad. Thanks!

Comment on attachment 8861574 [details] Bug 1358774 - Prevent dynamic toolbar from changing state when transitioning from multiple touch sources to a single touch source https://reviewboard.mozilla.org/r/133544/#review137824 ::: gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp:590 (Diff revisions 2 - 4) > // This overrides the touch event so just return > - if (mControllerCancelTouchTracking) { > - mControllerCancelTouchTracking = false; > + if (cancelTouchTracking) { > + return; > + } > + > + // The toolbar is already where it needs to be so just return; semicolon at end of comment :)

Pushed by rbarker@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/644ac0ba21b3 Prevent dynamic toolbar from changing state when transitioning from multiple touch sources to a single touch source r=botond