1212876 - Swiping left/right gesture for back/forward navigation broken

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[beingalink]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151008030232

Steps to reproduce:

Using two finger swipe gesture to navigate back.


Actual results:

Nothing


Expected results:

Previous page in navigation history should have opened. This started not to work with today's nightly (44.0a1 (2015-10-08)).

Comment 1

[Markus Stange [:mstange]]

This is a regression from bug 1202050.

Comment 2

[Markus Stange [:mstange]]

It's caused by this part of the patch, which changes TransformTo to UntransformTo:

@@ -696,12 +699,15 @@ APZCTreeManager::ReceiveInputEvent(InputData& aEvent,
         // gecko space should only consist of overscroll-cancelling transforms.
         Matrix4x4 transformToGecko = GetScreenToApzcTransform(apzc)
                                    * GetApzcToGeckoTransform(apzc);
-        MOZ_ASSERT(transformToGecko.Is2D());
-        ScreenPoint untransformedStartPoint = TransformTo<ScreenPixel>(
+        Maybe<ScreenPoint> untransformedStartPoint = UntransformTo<ScreenPixel>(
           transformToGecko, panInput.mPanStartPoint);
-        ScreenPoint untransformedDisplacement = TransformVector<ScreenPixel>(
+        Maybe<ScreenPoint> untransformedDisplacement = UntransformVector<ScreenPixel>(
             transformToGecko, panInput.mPanDisplacement, panInput.mPanStartPoint);

And UntransformTo has a bug that causes it to reverse the direction of the vector. This causes the swipe tracker to think that the user is trying to swipe in the opposite direction of the swipe start, and thus the swipe always fails.

Interestingly, simply fixing UntransformTo results in further breakage because other parts of pan gesture processing rely on this bug...

Comment 3

[Markus Stange [:mstange]]

Attachment: MozReview Request: Bug 1212876 - Fix a bug in UntransformVector and in code that was relying on that bug. r?botond

Bug 1212876 - Fix a bug in UntransformVector and in code that was relying on that bug. r?botond

Comment 4

[Botond Ballo [:botond]]

Comment on attachment 8671837 [details]
MozReview Request: Bug 1212876 - Fix a bug in UntransformVector and in code that was relying on that bug. r?botond

https://reviewboard.mozilla.org/r/21639/#review19493

::: gfx/layers/apz/src/Axis.cpp:79
(Diff revision 1)
> -  float newVelocity = mAxisLocked ? 0.0f : (float)(mPos - aPos + aAdditionalDelta) / (float)(aTimestampMs - mPosTimeMs);
> +  float newVelocity = mAxisLocked ? 0.0f : (float)(mPos - aPos - aAdditionalDelta) / (float)(aTimestampMs - mPosTimeMs);

I don't really understand the purpose of |aAdditionalDelta| here. Could you please document it in Axis.h?

Comment 5

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab253aae32aa21fb5d8ce2bf557a7e710498e834
Bug 1212876 - Fix a bug in UntransformVector and in code that was relying on that bug. r=botond

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ab253aae32aa

Comment 7

[u279076]

[e-mail]: can you please verify this is fixed in today's Nightly build?

Comment 8

[beingalink]

Yes, the left/right swiping gesture works again. Thanks for fixing this so fast!

Comment 9

[u279076]

(In reply to beingalink from comment #8)
> Yes, the left/right swiping gesture works again. Thanks for fixing this so
> fast!

Thank you for verifying, and for reporting the issue.