From c2876c73556964a55ec6b7c3ac7706654252da37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A3o=20Gottwald?= <dao@mozilla.com>
Date: Sat, 5 Aug 2017 10:12:38 +0200
Subject: [PATCH] Bug 1387084 - Use instant scroll behavior when doing pixel
scrolling. r=Gijs
Touchmove and wheel events are sent frequently enough that smooth scroll behavior
prevents the expected pixel distance from being reached before the next event.
Also replace aSmoothScroll parameters with aInstant to better reflect how this
works: its purpose is to force instant scrolling, whereas omitting it falls back
to "auto" (which may mean instant or smooth depending on different factors).
The ensureElementIsVisible call from browser-customization.js can go away as
customize mode doesn't add padding around the window anymore.
Finally, remove the unused scrollPosition and scrollPaddingRect properties.
MozReview-Commit-ID: 3Ac7g6zZ0hW
--HG--
extra : rebase_source : e43d0bcab82c74e65d01a0fd79bfaec96952b35a
---
browser/base/content/browser-customization.js | 8 ---
browser/base/content/tabbrowser.xml | 10 +--
browser/components/places/content/menu.xml | 2 +-
toolkit/content/widgets/scrollbox.xml | 63 +++++++------------
4 files changed, 27 insertions(+), 56 deletions(-)
diff --git a/browser/base/content/browser-customization.js b/browser/base/content/browser-customization.js
index 5867564e49434..213271cd6a932 100644
--- a/browser/base/content/browser-customization.js
+++ b/browser/base/content/browser-customization.js
@@ -41,14 +41,6 @@ var CustomizationHandler = {
CombinedStopReload.uninit();
PlacesToolbarHelper.customizeStart();
DownloadsButton.customizeStart();
-
- // The additional padding on the sides of the browser
- // can cause the customize tab to get clipped.
- let tabContainer = gBrowser.tabContainer;
- if (tabContainer.getAttribute("overflow") == "true") {
- let tabstrip = tabContainer.mTabstrip;
- tabstrip.ensureElementIsVisible(gBrowser.selectedTab, true);
- }
},
_customizationEnding(aDetails) {
diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml
index 5f8fd159fb623..ce1b547f9c535 100644
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -3558,7 +3558,7 @@
this.showTab(tab);
}
- this.tabContainer._handleTabSelect(false);
+ this.tabContainer._handleTabSelect(true);
]]>
</body>
</method>
@@ -3768,7 +3768,7 @@
if (wasFocused)
this.mCurrentTab.focus();
- this.tabContainer._handleTabSelect(false);
+ this.tabContainer._handleTabSelect(true);
if (aTab.pinned)
this.tabContainer._positionPinnedTabs();
@@ -5896,7 +5896,7 @@
var tabs = document.getBindingParent(this);
tabs.setAttribute("overflow", "true");
tabs._positionPinnedTabs();
- tabs._handleTabSelect(false);
+ tabs._handleTabSelect(true);
]]></handler>
</handlers>
</binding>
@@ -6253,10 +6253,10 @@
</method>
<method name="_handleTabSelect">
- <parameter name="aSmoothScroll"/>
+ <parameter name="aInstant"/>
<body><![CDATA[
if (this.getAttribute("overflow") == "true")
- this.mTabstrip.ensureElementIsVisible(this.selectedItem, aSmoothScroll);
+ this.mTabstrip.ensureElementIsVisible(this.selectedItem, aInstant);
]]></body>
</method>
diff --git a/browser/components/places/content/menu.xml b/browser/components/places/content/menu.xml
index edc987236bdb8..acbd70def7d0a 100644
--- a/browser/components/places/content/menu.xml
+++ b/browser/components/places/content/menu.xml
@@ -420,7 +420,7 @@
scrollDir = 1;
}
if (scrollDir != 0) {
- this._scrollBox.scrollByIndex(scrollDir, false);
+ this._scrollBox.scrollByIndex(scrollDir, true);
}
// Check if we should hide the drop indicator for this target.
diff --git a/toolkit/content/widgets/scrollbox.xml b/toolkit/content/widgets/scrollbox.xml
index 50c3ade864a80..8fcb1b86333e9 100644
--- a/toolkit/content/widgets/scrollbox.xml
+++ b/toolkit/content/widgets/scrollbox.xml
@@ -165,33 +165,6 @@
]]></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;
- innerRect.bottom = innerRect.top + this._scrollbox.scrollHeight;
- return innerRect;
- ]]></getter>
- </property>
- <property name="scrollPosition">
- <getter><![CDATA[
- return this.orient == "vertical" ?
- this._scrollbox.scrollTop :
- this._scrollbox.scrollLeft;
- ]]></getter>
- <setter><![CDATA[
- if (this.orient == "vertical")
- this._scrollbox.scrollTop = val;
- else
- this._scrollbox.scrollLeft = val;
- return val;
- ]]></setter>
- </property>
-
<field name="_startEndProps"><![CDATA[
this.orient == "vertical" ? ["top", "bottom"] : ["left", "right"];
]]></field>
@@ -241,18 +214,18 @@
<method name="ensureElementIsVisible">
<parameter name="element"/>
- <parameter name="aSmoothScroll"/>
+ <parameter name="aInstant"/>
<body><![CDATA[
if (!this._canScrollToElement(element))
return;
- element.scrollIntoView({ behavior: aSmoothScroll == false ? "instant" : "auto" });
+ element.scrollIntoView({ behavior: aInstant ? "instant" : "auto" });
]]></body>
</method>
<method name="scrollByIndex">
<parameter name="index"/>
- <parameter name="aSmoothScroll"/>
+ <parameter name="aInstant"/>
<body><![CDATA[
if (index == 0)
return;
@@ -264,7 +237,7 @@
let elements = this._getScrollableElements();
if (this._scrollTarget != elements[0] &&
this._scrollTarget != elements[elements.length - 1])
- this.ensureElementIsVisible(this._scrollTarget, false);
+ this.ensureElementIsVisible(this._scrollTarget, true);
}
var rect = this.scrollClientRect;
@@ -292,13 +265,13 @@
if (!targetElement)
return;
- this.ensureElementIsVisible(targetElement, aSmoothScroll);
+ this.ensureElementIsVisible(targetElement, aInstant);
]]></body>
</method>
<method name="scrollByPage">
<parameter name="pageDelta"/>
- <parameter name="aSmoothScroll"/>
+ <parameter name="aInstant"/>
<body><![CDATA[
if (pageDelta == 0)
return;
@@ -309,7 +282,7 @@
let elements = this._getScrollableElements();
if (this._scrollTarget != elements[0] &&
this._scrollTarget != elements[elements.length - 1])
- this.ensureElementIsVisible(this._scrollTarget, false);
+ this.ensureElementIsVisible(this._scrollTarget, true);
}
var [start, end] = this._startEndProps;
@@ -346,7 +319,7 @@
if (!targetElement)
return;
- this.ensureElementIsVisible(targetElement, aSmoothScroll);
+ this.ensureElementIsVisible(targetElement, aInstant);
]]></body>
</method>
@@ -424,9 +397,12 @@
</method>
<method name="scrollByPixels">
- <parameter name="px"/>
+ <parameter name="aPixels"/>
+ <parameter name="aInstant"/>
<body><![CDATA[
- this.scrollPosition += px;
+ let scrollOptions = { behavior: aInstant ? "instant" : "auto" };
+ scrollOptions[this._startEndProps[0]] = aPixels;
+ this._scrollbox.scrollBy(scrollOptions);
]]></body>
</method>
@@ -502,6 +478,7 @@
<handlers>
<handler event="wheel"><![CDATA[
let doScroll = false;
+ let instant;
let scrollAmount = 0;
if (this.orient == "vertical") {
doScroll = true;
@@ -526,12 +503,14 @@
if (this._prevMouseScrolls.every(prev => prev == isVertical)) {
doScroll = true;
- if (event.deltaMode == event.DOM_DELTA_PIXEL)
+ if (event.deltaMode == event.DOM_DELTA_PIXEL) {
scrollAmount = scrollByDelta;
- else if (event.deltaMode == event.DOM_DELTA_PAGE)
+ instant = true;
+ } else if (event.deltaMode == event.DOM_DELTA_PAGE) {
scrollAmount = scrollByDelta * this.scrollClientSize;
- else
+ } else {
scrollAmount = scrollByDelta * this.lineScrollAmount;
+ }
}
if (this._prevMouseScrolls.length > 1)
@@ -540,7 +519,7 @@
}
if (doScroll) {
- this.scrollByPixels(scrollAmount);
+ this.scrollByPixels(scrollAmount, instant);
}
event.stopPropagation();
@@ -570,7 +549,7 @@
: event.touches[0].screenX);
var delta = this._touchStart - touchPoint;
if (Math.abs(delta) > 0) {
- this.scrollByPixels(delta);
+ this.scrollByPixels(delta, true);
this._touchStart = touchPoint;
}
event.preventDefault();
--
GitLab