Bug 1391770 - Don't allow a faraway second tap to start a one-touch-pinch 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
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -356,16 +356,24 @@ typedef GenericFlingAnimation FlingAnima
* \li\b apz.popups.enabled
* Determines whether APZ is used for XUL popup widgets with remote content.
* Ideally, this should always be true, but it is currently not well tested, and
* has known issues, so needs to be prefable.
*
* \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).
*
* \li\b apz.touch_move_tolerance
* See the description for apz.touch_start_tolerance below. This is a similar
* threshold, except it is used to suppress touchmove events from being delivered
* to content for NON-scrollable frames (or more precisely, for APZCs where
* ArePointerEventsConsumable returns false).\n
@@ -827,16 +835,22 @@ AsyncPanZoomController::IsDestroyed() co
}
/* static */ScreenCoord
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());
}
bool
AsyncPanZoomController::ArePointerEventsConsumable(TouchBlockState* aBlock, uint32_t aTouchPoints) {
if (aTouchPoints == 0) {
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -110,16 +110,22 @@ public:
* Constant describing the tolerance in distance we use, multiplied by the
* device DPI, before we start panning the screen. This is to prevent us from
* accidentally processing taps as touch moves, and from very short/accidental
* touches moving the screen.
* Note: It's an abuse of the 'Coord' class to use it to represent a 2D
* 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,
const RefPtr<InputQueue>& aInputQueue,
GeckoContentController* aController,
GestureBehavior aGestures = DEFAULT_GESTURES);
// --------------------------------------------------------------------------
--- a/gfx/layers/apz/src/GestureEventListener.cpp
+++ b/gfx/layers/apz/src/GestureEventListener.cpp
@@ -173,17 +173,30 @@ nsEventStatus GestureEventListener::Hand
mTouchStartPosition = mLastTouchInput.mTouches[0].mLocalScreenPoint;
if (sLongTapEnabled) {
CreateLongTapTimeoutTask();
}
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");
SetState(GESTURE_NONE);
break;
}
return nsEventStatus_eIgnore;
@@ -235,22 +248,35 @@ nsEventStatus GestureEventListener::Hand
NS_WARNING("Unhandled state upon multitouch start");
SetState(GESTURE_NONE);
break;
}
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()
{
nsEventStatus rv = nsEventStatus_eIgnore;
switch (mState) {
case GESTURE_NONE:
--- a/gfx/layers/apz/src/GestureEventListener.h
+++ b/gfx/layers/apz/src/GestureEventListener.h
@@ -140,17 +140,19 @@ private:
nsEventStatus HandleInputTouchEnd();
nsEventStatus HandleInputTouchMove();
nsEventStatus HandleInputTouchCancel();
void HandleInputTimeoutLongTap();
void HandleInputTimeoutMaxTap(bool aDuringFastFling);
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
* her finger down.
*/
ParentLayerCoord GetYSpanFromStartPoint();
/**
--- a/gfx/thebes/gfxPrefs.h
+++ b/gfx/thebes/gfxPrefs.h
@@ -328,16 +328,17 @@ private:
DECL_GFX_PREF(Live, "apz.overscroll.stop_distance_threshold", APZOverscrollStopDistanceThreshold, float, 5.0f);
DECL_GFX_PREF(Live, "apz.overscroll.stop_velocity_threshold", APZOverscrollStopVelocityThreshold, float, 0.01f);
DECL_GFX_PREF(Live, "apz.overscroll.stretch_factor", APZOverscrollStretchFactor, float, 0.5f);
DECL_GFX_PREF(Live, "apz.paint_skipping.enabled", APZPaintSkipping, bool, true);
DECL_GFX_PREF(Live, "apz.peek_messages.enabled", APZPeekMessages, bool, true);
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);
DECL_GFX_PREF(Live, "apz.touch_start_tolerance", APZTouchStartTolerance, float, 1.0f/4.5f);
DECL_GFX_PREF(Live, "apz.velocity_bias", APZVelocityBias, float, 0.0f);
DECL_GFX_PREF(Live, "apz.velocity_relevance_time_ms", APZVelocityRelevanceTime, uint32_t, 150);
DECL_GFX_PREF(Live, "apz.x_skate_highmem_adjust", APZXSkateHighMemAdjust, float, 0.0f);
DECL_GFX_PREF(Live, "apz.x_skate_size_multiplier", APZXSkateSizeMultiplier, float, 1.5f);
--- a/mobile/android/app/mobile.js
+++ b/mobile/android/app/mobile.js
@@ -557,16 +557,17 @@ pref("apz.fling_curve_function_y1", "0.4
pref("apz.fling_curve_function_x2", "0.05");
pref("apz.fling_curve_function_y2", "1.00");
pref("apz.fling_curve_threshold_inches_per_ms", "0.01");
// apz.fling_friction and apz.fling_stopped_threshold are currently ignored by Fennec.
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");
#ifdef NIGHTLY_BUILD
// Temporary fix of Bug 1390145 for Fennec Nightly
pref("apz.frame_delay.enabled", false);
#endif
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -737,16 +737,17 @@ pref("apz.popups.enabled", false);
// Whether to print the APZC tree for debugging
pref("apz.printtree", false);
#ifdef NIGHTLY_BUILD
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");
pref("apz.velocity_bias", "0.0");
pref("apz.velocity_relevance_time_ms", 150);
pref("apz.x_skate_highmem_adjust", "0.0");
pref("apz.y_skate_highmem_adjust", "0.0");
pref("apz.x_skate_size_multiplier", "1.25");