Bug 1391770 - Don't allow a faraway second tap to start a one-touch-p…

…inch gesture. r=botond

This patch adds a new tolerance pref, which controls how far the second touchdown
is allowed to be from the first touchdown when detecting a multi-tap gesture
such as double-tap or one-touch-pinch. This stops the one-touch-pinch code
from inadvertently triggering when the user does a tap followed by a second tap
far away from the first.

The default value for the new pref is 5x the touch-start tolerance pref. This
seems to provide a reasonable behaviour for me, although this value could
probably be tuned.

MozReview-Commit-ID: 63aAyGCbvoN

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -361,6 +361,14 @@ typedef GenericFlingAnimation FlingAnimation;
  * \li\b apz.record_checkerboarding
  * Whether or not to record detailed info on checkerboarding events.
  *
+ * \li\b apz.second_tap_tolerance
+ * Constant describing the tolerance in distance we use, multiplied by the
+ * device DPI, within which a second tap is counted as part of a gesture
+ * continuing from the first tap. Making this larger allows the user more
+ * distance between the first and second taps in a "double tap" or "one touch
+ * pinch" gesture.\n
+ * Units: (real-world, i.e. screen) inches
+ *
  * \li\b apz.test.logging_enabled
  * Enable logging of APZ test data (see bug 961289).
  *
@@ -832,6 +840,12 @@ AsyncPanZoomController::GetTouchStartTolerance()
   return (gfxPrefs::APZTouchStartTolerance() * APZCTreeManager::GetDPI());
 }
 
+/* static */ScreenCoord
+AsyncPanZoomController::GetSecondTapTolerance()
+{
+  return (gfxPrefs::APZSecondTapTolerance() * APZCTreeManager::GetDPI());
+}
+
 /* static */AsyncPanZoomController::AxisLockMode AsyncPanZoomController::GetAxisLockMode()
 {
   return static_cast<AxisLockMode>(gfxPrefs::APZAxisLockMode());

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -115,6 +115,12 @@ class AsyncPanZoomController {
    *       distance, but it's the closest thing we currently have.
    */
   static ScreenCoord GetTouchStartTolerance();
+  /**
+   * Same as GetTouchStartTolerance, but the tolerance for how close the second
+   * tap has to be to the first tap in order to be counted as part of a multi-tap
+   * gesture (double-tap or one-touch-pinch).
+   */
+  static ScreenCoord GetSecondTapTolerance();
 
   AsyncPanZoomController(uint64_t aLayersId,
                          APZCTreeManager* aTreeManager,

gfx/layers/apz/src/GestureEventListener.cpp

@@ -178,7 +178,20 @@ nsEventStatus GestureEventListener::HandleInputTouchSingleStart()
     CreateMaxTapTimeoutTask();
     break;
   case GESTURE_FIRST_SINGLE_TOUCH_UP:
-    SetState(GESTURE_SECOND_SINGLE_TOUCH_DOWN);
+    if (SecondTapIsFar()) {
+      // If the second tap goes down far away from the first, then bail out
+      // of the gesture.
+      CancelLongTapTimeoutTask();
+      CancelMaxTapTimeoutTask();
+      mSingleTapSent = Nothing();
+      SetState(GESTURE_NONE);
+    } else {
+      // Otherwise, reset the touch start position so that, if this turns into
+      // a one-touch-pinch gesture, it uses the second tap's down position as
+      // the focus, rather than the first tap's.
+      mTouchStartPosition = mLastTouchInput.mTouches[0].mLocalScreenPoint;
+      SetState(GESTURE_SECOND_SINGLE_TOUCH_DOWN);
+    }
     break;
   default:
     NS_WARNING("Unhandled state upon single touch start");
@@ -240,12 +253,25 @@ nsEventStatus GestureEventListener::HandleInputTouchMultiStart()
   return rv;
 }
 
-bool GestureEventListener::MoveDistanceIsLarge()
+bool GestureEventListener::MoveDistanceExceeds(ScreenCoord aThreshold) const
 {
   const ParentLayerPoint start = mLastTouchInput.mTouches[0].mLocalScreenPoint;
   ParentLayerPoint delta = start - mTouchStartPosition;
   ScreenPoint screenDelta = mAsyncPanZoomController->ToScreenCoordinates(delta, start);
-  return (screenDelta.Length() > AsyncPanZoomController::GetTouchStartTolerance());
+  return (screenDelta.Length() > aThreshold);
+}
+
+bool GestureEventListener::MoveDistanceIsLarge() const
+{
+  return MoveDistanceExceeds(AsyncPanZoomController::GetTouchStartTolerance());
+}
+
+bool GestureEventListener::SecondTapIsFar() const
+{
+  // Allow a little more room here, because the is actually lifting their finger
+  // off the screen before replacing it, and that tends to have more error than
+  // wiggling the finger while on the screen.
+  return MoveDistanceExceeds(AsyncPanZoomController::GetSecondTapTolerance());
 }
 
 nsEventStatus GestureEventListener::HandleInputTouchMove()

gfx/layers/apz/src/GestureEventListener.h

@@ -145,7 +145,9 @@ class GestureEventListener final {
 
   void TriggerSingleTapConfirmedEvent();
 
-  bool MoveDistanceIsLarge();
+  bool MoveDistanceExceeds(ScreenCoord aThreshold) const;
+  bool MoveDistanceIsLarge() const;
+  bool SecondTapIsFar() const;
 
   /**
    * Returns current vertical span, counting from the where the user first put

gfx/thebes/gfxPrefs.h

@@ -333,6 +333,7 @@ class gfxPrefs final
   DECL_GFX_PREF(Live, "apz.popups.enabled",                    APZPopupsEnabled, bool, false);
   DECL_GFX_PREF(Live, "apz.printtree",                         APZPrintTree, bool, false);
   DECL_GFX_PREF(Live, "apz.record_checkerboarding",            APZRecordCheckerboarding, bool, false);
+  DECL_GFX_PREF(Live, "apz.second_tap_tolerance",              APZSecondTapTolerance, float, 0.5f);
   DECL_GFX_PREF(Live, "apz.test.fails_with_native_injection",  APZTestFailsWithNativeInjection, bool, false);
   DECL_GFX_PREF(Live, "apz.test.logging_enabled",              APZTestLoggingEnabled, bool, false);
   DECL_GFX_PREF(Live, "apz.touch_move_tolerance",              APZTouchMoveTolerance, float, 0.1f);

mobile/android/app/mobile.js

@@ -562,6 +562,7 @@ pref("apz.fling_friction", "0.004");
 pref("apz.fling_stopped_threshold", "0.0");
 pref("apz.max_velocity_inches_per_ms", "0.07");
 pref("apz.overscroll.enabled", true);
+pref("apz.second_tap_tolerance", "0.3");
 pref("apz.touch_move_tolerance", "0.03");
 pref("apz.touch_start_tolerance", "0.06");
 

modules/libpref/init/all.js

@@ -746,6 +746,7 @@ pref("apz.record_checkerboarding", true);
 #else
 pref("apz.record_checkerboarding", false);
 #endif
+pref("apz.second_tap_tolerance", "0.5");
 pref("apz.test.logging_enabled", false);
 pref("apz.touch_start_tolerance", "0.1");
 pref("apz.touch_move_tolerance", "0.1");