Bug 1708829 - Make sure that delta* getters account for scroll speed …

…override correctly. r=masayuki Depends on D114052 Differential Revision: https://phabricator.services.mozilla.com/D114737
dothq · May 14, 2021 · 2ec499c · 2ec499c
1 parent e595057
commit 2ec499c
Show file tree

Hide file tree

Showing 15 changed files with 50 additions and 48 deletions.
diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
@@ -6046,6 +6046,7 @@ void EventStateManager::DeltaAccumulator::InitLineOrPageDelta(
   // If it's handling neither a device that does not provide line or page deltas
   // nor delta values multiplied by prefs, we must not modify lineOrPageDelta
   // values.
+  // TODO(emilio): Does this care about overridden scroll speed?
   if (!mIsNoLineOrPageDeltaDevice &&
       !EventStateManager::WheelPrefs::GetInstance()
            ->NeedToComputeLineOrPageDelta(aEvent)) {
@@ -6105,13 +6106,7 @@ EventStateManager::DeltaAccumulator::ComputeScrollAmountForDefaultAction(
     WidgetWheelEvent* aEvent, const nsIntSize& aScrollAmountInDevPixels) {
   MOZ_ASSERT(aEvent);
 
-  // If the wheel event is line scroll and the delta value is computed from
-  // system settings, allow to override the system speed.
-  bool allowScrollSpeedOverride =
-      (!aEvent->mCustomizedByUserPrefs &&
-       aEvent->mDeltaMode == WheelEvent_Binding::DOM_DELTA_LINE);
-  DeltaValues acceleratedDelta =
-      WheelTransaction::AccelerateWheelDelta(aEvent, allowScrollSpeedOverride);
+  DeltaValues acceleratedDelta = WheelTransaction::AccelerateWheelDelta(aEvent);
 
   nsIntPoint result(0, 0);
   if (aEvent->mDeltaMode == WheelEvent_Binding::DOM_DELTA_PIXEL) {

diff --git a/dom/events/WheelEvent.cpp b/dom/events/WheelEvent.cpp
@@ -57,6 +57,7 @@ void WheelEvent::InitWheelEvent(
   wheelEvent->mDeltaY = aDeltaY;
   wheelEvent->mDeltaZ = aDeltaZ;
   wheelEvent->mDeltaMode = aDeltaMode;
+  wheelEvent->mAllowToOverrideSystemScrollSpeed = false;
 }
 
 int32_t WheelEvent::WheelDeltaX(CallerType aCallerType) {
@@ -68,8 +69,8 @@ int32_t WheelEvent::WheelDeltaX(CallerType aCallerType) {
     // We always return pixels regardless of the checking-state.
     double pixelDelta =
         ev->mDeltaMode == WheelEvent_Binding::DOM_DELTA_PIXEL
-            ? DevToCssPixels(ev->mDeltaX)
-            : ev->mDeltaX *
+            ? DevToCssPixels(ev->OverriddenDeltaX())
+            : ev->OverriddenDeltaX() *
                   CSSPixel::FromAppUnits(ev->mScrollAmount.width).Rounded();
     return int32_t(-std::round(pixelDelta * kTrustedDeltaToWheelDelta));
   }
@@ -85,8 +86,8 @@ int32_t WheelEvent::WheelDeltaY(CallerType aCallerType) {
   if (IsTrusted()) {
     double pixelDelta =
         ev->mDeltaMode == WheelEvent_Binding::DOM_DELTA_PIXEL
-            ? DevToCssPixels(ev->mDeltaY)
-            : ev->mDeltaY *
+            ? DevToCssPixels(ev->OverriddenDeltaY())
+            : ev->OverriddenDeltaY() *
                   CSSPixel::FromAppUnits(ev->mScrollAmount.height).Rounded();
     return int32_t(-std::round(pixelDelta * kTrustedDeltaToWheelDelta));
   }
@@ -113,14 +114,14 @@ double WheelEvent::ToWebExposedDelta(WidgetWheelEvent& aWidgetEvent,
 
 double WheelEvent::DeltaX(CallerType aCallerType) {
   WidgetWheelEvent* ev = mEvent->AsWheelEvent();
-  return ToWebExposedDelta(*ev, ev->mDeltaX, ev->mScrollAmount.width,
+  return ToWebExposedDelta(*ev, ev->OverriddenDeltaX(), ev->mScrollAmount.width,
                            aCallerType);
 }
 
 double WheelEvent::DeltaY(CallerType aCallerType) {
   WidgetWheelEvent* ev = mEvent->AsWheelEvent();
-  return ToWebExposedDelta(*ev, ev->mDeltaY, ev->mScrollAmount.height,
-                           aCallerType);
+  return ToWebExposedDelta(*ev, ev->OverriddenDeltaY(),
+                           ev->mScrollAmount.height, aCallerType);
 }
 
 double WheelEvent::DeltaZ(CallerType aCallerType) {

diff --git a/dom/events/WheelHandlingHelper.cpp b/dom/events/WheelHandlingHelper.cpp
@@ -357,19 +357,14 @@ LayoutDeviceIntPoint WheelTransaction::GetScreenPoint(WidgetGUIEvent* aEvent) {
 }
 
 /* static */
-DeltaValues WheelTransaction::AccelerateWheelDelta(
-    WidgetWheelEvent* aEvent, bool aAllowScrollSpeedOverride) {
-  DeltaValues result(aEvent);
+DeltaValues WheelTransaction::AccelerateWheelDelta(WidgetWheelEvent* aEvent) {
+  DeltaValues result = OverrideSystemScrollSpeed(aEvent);
 
   // Don't accelerate the delta values if the event isn't line scrolling.
   if (aEvent->mDeltaMode != dom::WheelEvent_Binding::DOM_DELTA_LINE) {
     return result;
   }
 
-  if (aAllowScrollSpeedOverride) {
-    result = OverrideSystemScrollSpeed(aEvent);
-  }
-
   // Accelerate by the sScrollSeriesCounter
   int32_t start = StaticPrefs::mousewheel_acceleration_start();
   if (start >= 0 && sScrollSeriesCounter >= start) {
@@ -394,7 +389,6 @@ double WheelTransaction::ComputeAcceleratedWheelDelta(double aDelta,
 DeltaValues WheelTransaction::OverrideSystemScrollSpeed(
     WidgetWheelEvent* aEvent) {
   MOZ_ASSERT(sTargetFrame, "We don't have mouse scrolling transaction");
-  MOZ_ASSERT(aEvent->mDeltaMode == dom::WheelEvent_Binding::DOM_DELTA_LINE);
 
   // If the event doesn't scroll to both X and Y, we don't need to do anything
   // here.

diff --git a/dom/events/WheelHandlingHelper.h b/dom/events/WheelHandlingHelper.h
@@ -139,8 +139,7 @@ class WheelTransaction {
 
   static void OwnScrollbars(bool aOwn);
 
-  static DeltaValues AccelerateWheelDelta(WidgetWheelEvent* aEvent,
-                                          bool aAllowScrollSpeedOverride);
+  static DeltaValues AccelerateWheelDelta(WidgetWheelEvent* aEvent);
 
  protected:
   static void BeginTransaction(nsIFrame* aTargetFrame,

diff --git a/dom/events/test/test_bug422132.html b/dom/events/test/test_bug422132.html
@@ -30,7 +30,7 @@
   SpecialPowers.pushPrefEnv({
     "set":[["general.smoothScroll", false],
            ["mousewheel.min_line_scroll_amount", 1],
-           ["mousewheel.system_scroll_override_on_root_content.enabled", false],
+           ["mousewheel.system_scroll_override.enabled", false],
            ["mousewheel.transaction.timeout", 100000]]}, runTests)}, window);
 
 function runTests()

diff --git a/dom/events/test/test_bug574663.html b/dom/events/test/test_bug574663.html
@@ -180,7 +180,7 @@
   SpecialPowers.pushPrefEnv({
     "set":[["general.smoothScroll", false],
            ["mousewheel.acceleration.start", -1],
-           ["mousewheel.system_scroll_override_on_root_content.enabled", false],
+           ["mousewheel.system_scroll_override.enabled", false],
            ["mousewheel.with_control.action", 3]]}, runTest);
 }
 

diff --git a/dom/events/test/test_bug607464.html b/dom/events/test/test_bug607464.html
@@ -71,7 +71,7 @@
   SpecialPowers.pushPrefEnv({
     "set":[["general.smoothScroll", true],
            ["mousewheel.acceleration.start", -1],
-           ["mousewheel.system_scroll_override_on_root_content.enabled", false]]}, runTest);
+           ["mousewheel.system_scroll_override.enabled", false]]}, runTest);
 }
 
 SimpleTest.waitForExplicitFinish();

diff --git a/dom/events/test/test_bug946632.html b/dom/events/test/test_bug946632.html
@@ -41,7 +41,7 @@
 SimpleTest.waitForFocus(function() {
   SpecialPowers.pushPrefEnv({
     "set":[["general.smoothScroll", false],
-           ["mousewheel.system_scroll_override_on_root_content.enabled", false]]
+           ["mousewheel.system_scroll_override.enabled", false]]
     }, runTests)
   }, window);
 

diff --git a/dom/events/test/test_continuous_wheel_events.html b/dom/events/test/test_continuous_wheel_events.html
@@ -3263,6 +3263,8 @@
 
     ["dom.event.wheel-deltaMode-lines.disabled", true],
 
+    ["mousewheel.system_scroll_override.enabled", false],
+
     ["mousewheel.transaction.timeout", 100000],
     ["mousewheel.default.delta_multiplier_x", 100],
     ["mousewheel.default.delta_multiplier_y", 100],

diff --git a/dom/events/test/test_dom_wheel_event.html b/dom/events/test/test_dom_wheel_event.html
@@ -216,6 +216,13 @@
     throw new Error("Unknown delta mode");
   }
 
+  function defaultScrollMultiplier(horizontal) {
+    if (!SpecialPowers.getBoolPref("mousewheel.system_scroll_override.enabled")) {
+      return 1;
+    }
+    return SpecialPowers.getIntPref(`mousewheel.system_scroll_override.${horizontal ? "horizontal" : "vertical"}.factor`) / 100;
+  }
+
   function wheelEventHandler(aEvent) {
     calledHandlers.wheel = true;
 
@@ -235,6 +242,9 @@
           expectedDeltaZ *= currentMultiplier;
           break;
       }
+    } else if (aEvent.isTrusted && aEvent.deltaMode == WheelEvent.DOM_DELTA_LINE) {
+      expectedDeltaX *= defaultScrollMultiplier(true);
+      expectedDeltaY *= defaultScrollMultiplier(false);
     }
     is(aEvent.deltaMode, currentEvent.deltaMode, description + "deltaMode (" + currentEvent.deltaMode + ") was invalid");
     is(aEvent.deltaX, expectedDeltaX, description + "deltaX (" + currentEvent.deltaX + ") was invalid");

diff --git a/dom/events/test/test_wheel_default_action.html b/dom/events/test/test_wheel_default_action.html
@@ -17,7 +17,7 @@
 SimpleTest.waitForExplicitFinish();
 SimpleTest.requestFlakyTimeout("untriaged");
 SpecialPowers.pushPrefEnv({"set": [
-    ["mousewheel.system_scroll_override_on_root_content.enabled", false]
+    ["mousewheel.system_scroll_override.enabled", false]
 ]}, runTest);
 
 var subWin = null;

diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -1924,11 +1924,10 @@ ParentLayerPoint AsyncPanZoomController::GetScrollWheelDelta(
   delta.y *= aMultiplierY;
 
   // For the conditions under which we allow system scroll overrides, see
-  // EventStateManager::DeltaAccumulator::ComputeScrollAmountForDefaultAction
-  // and WheelTransaction::OverrideSystemScrollSpeed. Note that we do *not*
-  // restrict this to the root content, see bug 1217715 for discussion on this.
-  if (StaticPrefs::
-          mousewheel_system_scroll_override_on_root_content_enabled() &&
+  // WidgetWheelEvent::OverriddenDelta{X,Y}.
+  // Note that we do *not* restrict this to the root content, see bug 1217715
+  // for discussion on this.
+  if (StaticPrefs::mousewheel_system_scroll_override_enabled() &&
       !aEvent.IsCustomizedByUserPrefs() &&
       aEvent.mDeltaType == ScrollWheelInput::SCROLLDELTA_LINE &&
       aEvent.mAllowToOverrideSystemScrollSpeed) {

diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -8706,7 +8706,7 @@
   value: false
   mirror: always
 
-- name: mousewheel.system_scroll_override_on_root_content.enabled
+- name: mousewheel.system_scroll_override.enabled
   type: RelaxedAtomicBool
 #ifdef XP_WIN
   value: true
@@ -8717,18 +8717,18 @@
 
 # Prefs for overriding the system mouse wheel scrolling speed on
 # content of the web pages.  When
-# "mousewheel.system_scroll_override_on_root_content.enabled" is true and the
+# "mousewheel.system_scroll_override.enabled" is true and the
 # system scrolling speed isn't customized by the user, the content scrolling
 # speed is multiplied by the following factors.  The value will be used as
 # 1/100.  E.g., 200 means 2.00.
-# NOTE: Even if "mousewheel.system_scroll_override_on_root_content.enabled" is
+# NOTE: Even if "mousewheel.system_scroll_override.enabled" is
 # true, when Gecko detects the user customized the system scrolling speed
 # settings, the override isn't executed.
-- name: mousewheel.system_scroll_override_on_root_content.horizontal.factor
+- name: mousewheel.system_scroll_override.horizontal.factor
   type: RelaxedAtomicInt32
   value: 200
   mirror: always
-- name: mousewheel.system_scroll_override_on_root_content.vertical.factor
+- name: mousewheel.system_scroll_override.vertical.factor
   type: RelaxedAtomicInt32
   value: 200
   mirror: always

diff --git a/widget/WidgetEventImpl.cpp b/widget/WidgetEventImpl.cpp
@@ -18,6 +18,7 @@
 #include "mozilla/TouchEvents.h"
 #include "mozilla/WritingModes.h"
 #include "mozilla/dom/KeyboardEventBinding.h"
+#include "mozilla/dom/WheelEventBinding.h"
 #include "nsCommandParams.h"
 #include "nsContentUtils.h"
 #include "nsIContent.h"
@@ -729,16 +730,13 @@ void WidgetDragEvent::InitDropEffectForTests() {
 /* static */
 double WidgetWheelEvent::ComputeOverriddenDelta(double aDelta,
                                                 bool aIsForVertical) {
-  if (!StaticPrefs::
-          mousewheel_system_scroll_override_on_root_content_enabled()) {
+  if (!StaticPrefs::mousewheel_system_scroll_override_enabled()) {
     return aDelta;
   }
   int32_t intFactor =
       aIsForVertical
-          ? StaticPrefs::
-                mousewheel_system_scroll_override_on_root_content_vertical_factor()
-          : StaticPrefs::
-                mousewheel_system_scroll_override_on_root_content_horizontal_factor();
+          ? StaticPrefs::mousewheel_system_scroll_override_vertical_factor()
+          : StaticPrefs::mousewheel_system_scroll_override_horizontal_factor();
   // Making the scroll speed slower doesn't make sense. So, ignore odd factor
   // which is less than 1.0.
   if (intFactor <= 100) {
@@ -749,14 +747,18 @@ double WidgetWheelEvent::ComputeOverriddenDelta(double aDelta,
 }
 
 double WidgetWheelEvent::OverriddenDeltaX() const {
-  if (!mAllowToOverrideSystemScrollSpeed) {
+  if (!mAllowToOverrideSystemScrollSpeed ||
+      mDeltaMode != dom::WheelEvent_Binding::DOM_DELTA_LINE ||
+      mCustomizedByUserPrefs) {
     return mDeltaX;
   }
   return ComputeOverriddenDelta(mDeltaX, false);
 }
 
 double WidgetWheelEvent::OverriddenDeltaY() const {
-  if (!mAllowToOverrideSystemScrollSpeed) {
+  if (!mAllowToOverrideSystemScrollSpeed ||
+      mDeltaMode != dom::WheelEvent_Binding::DOM_DELTA_LINE ||
+      mCustomizedByUserPrefs) {
     return mDeltaY;
   }
   return ComputeOverriddenDelta(mDeltaY, true);

diff --git a/widget/tests/window_mouse_scroll_win.html b/widget/tests/window_mouse_scroll_win.html
@@ -45,7 +45,7 @@
 
 const DOM_PAGE_SCROLL_DELTA = 32768;
 
-const kSystemScrollSpeedOverridePref = "mousewheel.system_scroll_override_on_root_content.enabled";
+const kSystemScrollSpeedOverridePref = "mousewheel.system_scroll_override.enabled";
 
 const kAltKeyActionPref         = "mousewheel.with_alt.action";
 const kCtrlKeyActionPref        = "mousewheel.with_control.action";