Bug 1316505 part.1 The "wheel" event handler of <scrollbox> should use |.scrollByPixels()| for respecting wheel event's scroll speed and scrolling smoother r?enndeakin draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Sun, 13 Nov 2016 19:17:07 +0900
changeset 438105 608cd1cd5dd8073b92b75a32d584cd228677bee2
parent 438104 3c166172ce2be913b29aee47fd5ae2407b06ade2
child 438106 fd9e668c039aa3d0e9e2e97fca081218201e36ea
push id35619
push usermasayuki@d-toybox.com
push dateSun, 13 Nov 2016 11:10:23 +0000
reviewersenndeakin
bugs1316505
milestone52.0a1
Bug 1316505 part.1 The "wheel" event handler of <scrollbox> should use |.scrollByPixels()| for respecting wheel event's scroll speed and scrolling smoother r?enndeakin Currently, ths "wheel" event handler of <scrollbox> uses |.scrollByIndex()|. However, it scrolls too fast when "wheel" event has high resolution delta value when its deltaMode is DOM_DELTA_LINE. For respecting the delta value, it should use |.scrollByPixels()|. Therefore, this patch implements new readonly property, |.lineScrollAmount|, which returns font size of the |.scrollbox| and makes "wheel" event handler multiplies the delta value and |.lineScrollAmount|. MozReview-Commit-ID: KzIvJyxqrn5
browser/base/content/test/general/browser_overflowScroll.js
toolkit/content/widgets/scrollbox.xml
--- a/browser/base/content/test/general/browser_overflowScroll.js
+++ b/browser/base/content/test/general/browser_overflowScroll.js
@@ -70,22 +70,14 @@ function runOverflowTests(aEvent) {
   EventUtils.synthesizeMouse(upButton, 1, 1, {clickCount: 2});
   isLeft(element, "Scrolled one page of tabs with a double click");
 
   EventUtils.synthesizeMouse(upButton, 1, 1, {clickCount: 3});
   var firstScrollableLeft = left(firstScrollable());
   ok(left(scrollbox) <= firstScrollableLeft, "Scrolled to the start with a triple click " +
      "(" + left(scrollbox) + " <= " + firstScrollableLeft + ")");
 
-  for (var i = 2; i; i--)
-    EventUtils.synthesizeWheel(scrollbox, 1, 1, { deltaX: -1.0, deltaMode: WheelEvent.DOM_DELTA_LINE });
-  is(left(firstScrollable()), firstScrollableLeft, "Remained at the start with the mouse wheel");
-
-  element = nextRightElement();
-  EventUtils.synthesizeWheel(scrollbox, 1, 1, { deltaX: 1.0, deltaMode: WheelEvent.DOM_DELTA_LINE});
-  isRight(element, "Scrolled one tab to the right with the mouse wheel");
-
   while (tabs.length > 1)
     gBrowser.removeTab(tabs[0]);
 
   tabstrip.smoothScroll = originalSmoothScroll;
   finish();
 }
--- a/toolkit/content/widgets/scrollbox.xml
+++ b/toolkit/content/widgets/scrollbox.xml
@@ -142,16 +142,33 @@
 
       <property name="scrollSize" readonly="true">
         <getter><![CDATA[
           return this.orient == "vertical" ?
                  this._scrollbox.scrollHeight :
                  this._scrollbox.scrollWidth;
         ]]></getter>
       </property>
+
+      <property name="lineScrollAmount" readonly="true">
+        <getter><![CDATA[
+          // If inherited element wants to customize line scroll amount by
+          // wheel events, it should override this.
+          var px = document.defaultView
+                           .getComputedStyle(this._scrollbox, "")
+                           .getPropertyValue("font-size");
+          if (px.endsWith("px")) {
+            return parseInt(px);
+          }
+          // XXX Is this possible case?  For the last resote, let's return
+          //     default font-size of web contents.
+          return 16;
+        ]]></getter>
+      </property>
+
       <property name="scrollPaddingRect" readonly="true">
         <getter><![CDATA[
           // This assumes that this._scrollbox doesn't have any border.
           var outerRect = this.scrollClientRect;
           var innerRect = {};
           innerRect.left = outerRect.left - this._scrollbox.scrollLeft;
           innerRect.top = outerRect.top - this._scrollbox.scrollTop;
           innerRect.right = innerRect.left + this._scrollbox.scrollWidth;
@@ -555,17 +572,17 @@
     <handlers>
       <handler event="wheel"><![CDATA[
         if (this.orient == "vertical") {
           if (event.deltaMode == event.DOM_DELTA_PIXEL)
             this.scrollByPixels(event.deltaY);
           else if (event.deltaMode == event.DOM_DELTA_PAGE)
             this.scrollByPage(event.deltaY);
           else
-            this.scrollByIndex(event.deltaY);
+            this.scrollByPixels(event.deltaY * this.lineScrollAmount);
         }
         // We allow vertical scrolling to scroll a horizontal scrollbox
         // because many users have a vertical scroll wheel but no
         // horizontal support.
         // Because of this, we need to avoid scrolling chaos on trackpads
         // and mouse wheels that support simultaneous scrolling in both axes.
         // We do this by scrolling only when the last two scroll events were
         // on the same axis as the current scroll event.
@@ -576,17 +593,17 @@
           let scrollByDelta = isVertical && this._isRTLScrollbox ? -delta : delta;
 
           if (this._prevMouseScrolls.every(prev => prev == isVertical)) {
             if (event.deltaMode == event.DOM_DELTA_PIXEL)
               this.scrollByPixels(scrollByDelta);
             else if (event.deltaMode == event.DOM_DELTA_PAGE)
               this.scrollByPage(scrollByDelta);
             else
-              this.scrollByIndex(scrollByDelta);
+              this.scrollByPixels(scrollByDelta * this.lineScrollAmount);
           }
 
           if (this._prevMouseScrolls.length > 1)
             this._prevMouseScrolls.shift();
           this._prevMouseScrolls.push(isVertical);
         }
 
         event.stopPropagation();