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
--- a/browser/base/content/browser-customization.js
+++ b/browser/base/content/browser-customization.js
@@ -36,24 +36,16 @@ var CustomizationHandler = {
let cmd = document.getElementById("cmd_CustomizeToolbars");
cmd.setAttribute("disabled", "true");
UpdateUrlbarSearchSplitterState();
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) {
// Update global UI elements that may have been added or removed
if (aDetails.changed) {
gURLBar = document.getElementById("urlbar");
gHomeButton.updateTooltip();
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -3553,17 +3553,17 @@
<![CDATA[
for (let tab of this.tabs) {
if (aTabs.indexOf(tab) == -1)
this.hideTab(tab);
else
this.showTab(tab);
}
- this.tabContainer._handleTabSelect(false);
+ this.tabContainer._handleTabSelect(true);
]]>
</body>
</method>
<method name="showTab">
<parameter name="aTab"/>
<body>
<![CDATA[
@@ -3763,17 +3763,17 @@
// correct, and if we are in the midst of a tab switch, then the async
// tab switcher will set the visually selected tab once the tab switch
// has completed.
this.mCurrentTab._selected = true;
if (wasFocused)
this.mCurrentTab.focus();
- this.tabContainer._handleTabSelect(false);
+ this.tabContainer._handleTabSelect(true);
if (aTab.pinned)
this.tabContainer._positionPinnedTabs();
this.tabContainer._setPositionalAttributes();
var evt = document.createEvent("UIEvents");
evt.initUIEvent("TabMove", true, false, window, oldPosition);
@@ -5891,17 +5891,17 @@
// Ignore vertical events
if (event.detail == 0)
return;
var tabs = document.getBindingParent(this);
tabs.setAttribute("overflow", "true");
tabs._positionPinnedTabs();
- tabs._handleTabSelect(false);
+ tabs._handleTabSelect(true);
]]></handler>
</handlers>
</binding>
<binding id="tabbrowser-tabs"
extends="chrome://global/content/bindings/tabbox.xml#tabs">
<resources>
<stylesheet src="chrome://browser/content/tabbrowser.css"/>
@@ -6248,20 +6248,20 @@
this.removeAttribute("closebuttons");
}
});
});
]]></body>
</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>
<field name="_closingTabsSpacer">
document.getAnonymousElementByAttribute(this, "anonid", "closing-tabs-spacer");
</field>
<field name="_tabDefaultMaxWidth">NaN</field>
--- a/browser/components/places/content/menu.xml
+++ b/browser/components/places/content/menu.xml
@@ -415,17 +415,17 @@
let anonid = event.originalTarget.getAttribute("anonid");
let scrollDir = 0;
if (anonid == "scrollbutton-up") {
scrollDir = -1;
} else if (anonid == "scrollbutton-down") {
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.
if (dropPoint.folderElt || this._hideDropIndicator(event)) {
this._indicatorBar.hidden = true;
event.preventDefault();
event.stopPropagation();
return;
--- a/toolkit/content/widgets/scrollbox.xml
+++ b/toolkit/content/widgets/scrollbox.xml
@@ -160,43 +160,16 @@
var high = elements.length - 1;
// XXX If the total width is 0, do we need something more?
var totalWidth =
elements[high].getBoundingClientRect()[end] - elements[low].getBoundingClientRect()[start];
return totalWidth / elements.length;
]]></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>
<field name="_isRTLScrollbox"><![CDATA[
this.orient != "vertical" &&
document.defaultView.getComputedStyle(this._scrollbox).direction == "rtl";
]]></field>
@@ -236,40 +209,40 @@
// here which means that on the fly changes aren't fully supported.
let rect = this._boundsWithoutFlushing(element);
return !!(rect.top || rect.left || rect.width || rect.height);
]]></body>
</method>
<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;
// Each scrollByIndex call is expected to scroll the given number of
// items. If a previous call is still in progress because of smooth
// scrolling, we need to complete it before starting a new one.
if (this._scrollTarget) {
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;
var [start, end] = this._startEndProps;
var x = index > 0 ? rect[end] + 1 : rect[start] - 1;
var nextElement = this._elementFromPoint(x, index);
if (!nextElement)
return;
@@ -287,34 +260,34 @@
if (this._canScrollToElement(nextElement))
targetElement = nextElement;
nextElement = nextElement.nextSibling;
index--;
}
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;
// If a previous call is still in progress because of smooth
// scrolling, we need to complete it before starting a new one.
if (this._scrollTarget) {
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;
var rect = this.scrollClientRect;
var containerEdge = pageDelta > 0 ? rect[end] + 1 : rect[start] - 1;
var pixelDelta = pageDelta * (rect[end] - rect[start]);
var destinationPosition = containerEdge + pixelDelta;
var nextElement = this._elementFromPoint(containerEdge, pageDelta);
@@ -341,17 +314,17 @@
if (this._canScrollToElement(nextElement))
targetElement = nextElement;
nextElement = advance(nextElement);
} while (nextElement && !extendsPastTarget(nextElement));
if (!targetElement)
return;
- this.ensureElementIsVisible(targetElement, aSmoothScroll);
+ this.ensureElementIsVisible(targetElement, aInstant);
]]></body>
</method>
<method name="_getScrollableElements">
<body><![CDATA[
var nodes = this.childNodes;
if (nodes.length == 1 &&
nodes[0].localName == "children" &&
@@ -419,19 +392,22 @@
this.scrollByPixels(this.scrollIncrement * dir);
event.stopPropagation();
]]></body>
</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>
<field name="_prevMouseScrolls">[null, null]</field>
<field name="_touchStart">-1</field>
<field name="_scrollButtonUpdatePending">false</field>
@@ -497,16 +473,17 @@
});
]]></body>
</method>
</implementation>
<handlers>
<handler event="wheel"><![CDATA[
let doScroll = false;
+ let instant;
let scrollAmount = 0;
if (this.orient == "vertical") {
doScroll = true;
if (event.deltaMode == event.DOM_DELTA_PIXEL)
scrollAmount = event.deltaY;
else if (event.deltaMode == event.DOM_DELTA_PAGE)
scrollAmount = event.deltaY * this.scrollClientSize;
else
@@ -521,31 +498,33 @@
// on the same axis as the current scroll event.
// For diagonal scroll events we only respect the dominant axis.
let isVertical = Math.abs(event.deltaY) > Math.abs(event.deltaX);
let delta = isVertical ? event.deltaY : event.deltaX;
let scrollByDelta = isVertical && this._isRTLScrollbox ? -delta : delta;
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)
this._prevMouseScrolls.shift();
this._prevMouseScrolls.push(isVertical);
}
if (doScroll) {
- this.scrollByPixels(scrollAmount);
+ this.scrollByPixels(scrollAmount, instant);
}
event.stopPropagation();
event.preventDefault();
]]></handler>
<handler event="touchstart"><![CDATA[
if (event.touches.length > 1) {
@@ -565,17 +544,17 @@
<handler event="touchmove"><![CDATA[
if (event.touches.length == 1 &&
this._touchStart >= 0) {
var touchPoint = (this.orient == "vertical"
? event.touches[0].screenY
: 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();
}
]]></handler>
<handler event="touchend"><