Swiping left/right gesture for back/forward navigation broken

VERIFIED FIXED in Firefox 44

Status

()

VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: beingalink, Assigned: mstange)

Tracking

({regression})

Trunk
mozilla44
x86_64
Mac OS X
regression
Points:
---

Firefox Tracking Flags

(firefox44 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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)).
(Reporter)

Updated

3 years ago
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
(Assignee)

Updated

3 years ago
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Version: 44 Branch → Trunk
(Assignee)

Comment 1

3 years ago
This is a regression from bug 1202050.
Blocks: 1202050
Component: General → Panning and Zooming
(Assignee)

Comment 2

3 years ago
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...
Assignee: nobody → mstange
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8671837 [details]
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
Attachment #8671837 - Flags: review?(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?
Attachment #8671837 - Flags: review?(botond) → review+
(Assignee)

Comment 5

3 years ago
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
https://hg.mozilla.org/mozilla-central/rev/ab253aae32aa
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
beingalink@googlemail.com: can you please verify this is fixed in today's Nightly build?
Flags: needinfo?(beingalink)
(Reporter)

Comment 8

3 years ago
Yes, the left/right swiping gesture works again. Thanks for fixing this so fast!
Flags: needinfo?(beingalink)
(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.
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified
You need to log in before you can comment on or make changes to this bug.