Bug 1368208 - Don't flush layout to determine whether scrollbox scroll buttons should be enabled/disabled
MozReview-Commit-ID: HcOQ4W0Z4YP
--- a/layout/reftests/bugs/508816-1-ref.xul
+++ b/layout/reftests/bugs/508816-1-ref.xul
@@ -1,52 +1,40 @@
<?xml version="1.0"?>
<!DOCTYPE window>
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" title="RTL overflow test" width="640px" height="480px" style="direction: rtl" onload="load()" class="reftest-wait">
+<style type="text/css" xmlns="http://www.w3.org/1999/xhtml"><![CDATA[
+button {
+ margin: 0;
+}
+]]></style>
+
<hbox style="max-width: 200px; border: 1px solid red;">
-<scrollbox id="sb" align="start" pack="start" orient="horizontal" flex="1">
+<scrollbox align="start" pack="start" orient="horizontal" flex="1">
<button id="sb1" label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
</scrollbox>
</hbox>
<hbox style="max-width: 200px; border: 1px solid blue">
-<arrowscrollbox id="asb" align="start" pack="start" orient="horizontal" flex="1">
+<arrowscrollbox align="start" pack="start" orient="horizontal" flex="1">
<button id="asb1" label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
</arrowscrollbox>
</hbox>
<hbox style="max-width: 200px; border: 1px solid black">
<hbox align="end" pack="end" flex="1">
<button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
</hbox>
</hbox>
<script type="text/javascript">
<![CDATA[
function load() {
- // arrowscrollbox provides a helper ensureElementIsVisible method
- document.getElementById("asb").ensureElementIsVisible(document.getElementById("asb1"), false);
-
- // scrollbox doesn't provide that, so we have to do the heavy lifting directly
- var sb = document.getElementById("sb");
- var sb1 = document.getElementById("sb1");
- var rect = sb.getBoundingClientRect();
- var containerStart = rect.left;
- var containerEnd = rect.right;
- rect = sb1.getBoundingClientRect();
- var elementStart = rect.left;
- var elementEnd = rect.right;
-
- var amountToScroll = 0;
- if (elementStart < containerStart) {
- amountToScroll = elementStart - containerStart;
- } else if (elementEnd > containerEnd) {
- amountToScroll = elementEnd - containerEnd;
- }
- sb.scrollLeft += amountToScroll;
+ document.getElementById("sb1").scrollIntoView({ behavior: "instant" });
+ document.getElementById("asb1").scrollIntoView({ behavior: "instant" });
document.documentElement.removeAttribute("class");
}
]]>
</script>
</window>
--- a/layout/reftests/bugs/508816-1.xul
+++ b/layout/reftests/bugs/508816-1.xul
@@ -1,23 +1,29 @@
-<?xml version="1.0"?>
-<!DOCTYPE window>
-<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" title="RTL overflow test" width="640px" height="480px" style="direction: rtl">
-
-<hbox style="max-width: 200px; border: 1px solid red;">
-<scrollbox align="start" pack="start" orient="horizontal" flex="1">
- <button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
-</scrollbox>
-</hbox>
-
-<hbox style="max-width: 200px; border: 1px solid blue">
-<arrowscrollbox align="start" pack="start" orient="horizontal" flex="1">
- <button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
-</arrowscrollbox>
-</hbox>
-
-<hbox style="max-width: 200px; border: 1px solid black">
-<hbox align="end" pack="end" flex="1">
- <button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
-</hbox>
-</hbox>
-
-</window>
+<?xml version="1.0"?>
+<!DOCTYPE window>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" title="RTL overflow test" width="640px" height="480px" style="direction: rtl">
+
+<style type="text/css" xmlns="http://www.w3.org/1999/xhtml"><![CDATA[
+button {
+ margin: 0;
+}
+]]></style>
+
+<hbox style="max-width: 200px; border: 1px solid red;">
+<scrollbox align="start" pack="start" orient="horizontal" flex="1">
+ <button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
+</scrollbox>
+</hbox>
+
+<hbox style="max-width: 200px; border: 1px solid blue">
+<arrowscrollbox align="start" pack="start" orient="horizontal" flex="1">
+ <button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
+</arrowscrollbox>
+</hbox>
+
+<hbox style="max-width: 200px; border: 1px solid black">
+<hbox align="end" pack="end" flex="1">
+ <button label="test1"/><button label="test2"/><button label="test3"/><button label="test4"/><button label="test5"/>
+</hbox>
+</hbox>
+
+</window>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1406,17 +1406,17 @@ needs-focus fails-if(!styloVsGecko) == 5
fuzzy-if(Android,5,2800) == 506481-1.html 506481-1-ref.html
== 507187-1.html 507187-1-ref.html
== 507487-1.html 507487-1-ref.html
== 507487-2.xhtml 507487-2-ref.xhtml
== 507762-1.html 507762-1-ref.html
== 507762-2.html 507762-2-ref.html
== 507762-3.html 507762-1-ref.html
== 507762-4.html 507762-2-ref.html
-random-if(cocoaWidget) == 508816-1.xul 508816-1-ref.xul # Bug 631982
+random-if(cocoaWidget||(gtkWidget&&webrender)||winWidget) == 508816-1.xul 508816-1-ref.xul # Bug 631982 and bug 1375012
== 508816-2.html 508816-2-ref.html
== 508908-1.xul 508908-1-ref.xul
== 508919-1.xhtml 508919-1-ref.xhtml
== 509155-1.xhtml 509155-1-ref.xhtml
fuzzy-if(Android,5,1656) fuzzy-if(skiaContent,1,1200) == 512410.html 512410-ref.html
== 512631-1.html 512631-1-ref.html
== 513153-1a.html 513153-1-ref.html
== 513153-1b.html 513153-1-ref.html
--- a/toolkit/content/widgets/scrollbox.xml
+++ b/toolkit/content/widgets/scrollbox.xml
@@ -185,16 +185,21 @@
return innerRect;
]]></getter>
</property>
<field name="scrollboxPaddingStart"><![CDATA[
parseFloat(window.getComputedStyle(this._scrollbox)[
this._isRTLScrollbox ? "paddingRight" : "paddingLeft"
]);
]]></field>
+ <field name="scrollboxPaddingEnd"><![CDATA[
+ parseFloat(window.getComputedStyle(this._scrollbox)[
+ this._isRTLScrollbox ? "paddingLeft" : "paddingRight"
+ ]);
+ ]]></field>
<property name="scrollPosition">
<getter><![CDATA[
return this.orient == "vertical" ?
this._scrollbox.scrollTop :
this._scrollbox.scrollLeft;
]]></getter>
<setter><![CDATA[
if (this.orient == "vertical")
@@ -629,50 +634,80 @@
if (this._isScrolling) {
this._scrollAnim.stop();
this._isScrolling = 0;
this._scrollTarget = null;
}
]]></body>
</method>
+ <field name="_scrollButtonUpdatePending">false</field>
<method name="_updateScrollButtonsDisabledState">
- <parameter name="aScrolling"/>
<body><![CDATA[
- let scrolledToStart;
- let scrolledToEnd;
-
- // Avoid flushing layout when not overflowing or when scrolling.
if (this.hasAttribute("notoverflowing")) {
- scrolledToStart = true;
- scrolledToEnd = true;
- } else if (aScrolling) {
- scrolledToStart = false;
- scrolledToEnd = false;
- } else if (this.scrollPosition == 0) {
- // In the RTL case, this means the _last_ element in the
- // scrollbox is visible
- scrolledToEnd = this._isRTLScrollbox;
- scrolledToStart = !this._isRTLScrollbox;
- } else if (this.scrollClientSize + this.scrollPosition == this.scrollSize) {
- // In the RTL case, this means the _first_ element in the
- // scrollbox is visible
- scrolledToStart = this._isRTLScrollbox;
- scrolledToEnd = !this._isRTLScrollbox;
+ this.setAttribute("scrolledtoend", "true");
+ this.setAttribute("scrolledtostart", "true");
+ return;
}
- if (scrolledToEnd)
- this.setAttribute("scrolledtoend", "true");
- else
- this.removeAttribute("scrolledtoend");
+ if (this._scrollButtonUpdatePending) {
+ return;
+ }
+ this._scrollButtonUpdatePending = true;
+
+ // Wait until after the next paint to get current layout data from
+ // getBoundsWithoutFlushing.
+ window.requestAnimationFrame(() => {
+ setTimeout(() => {
+ this._scrollButtonUpdatePending = false;
+
+ let scrolledToStart = false;
+ let scrolledToEnd = false;
+
+ if (this.hasAttribute("notoverflowing")) {
+ scrolledToStart = true;
+ scrolledToEnd = true;
+ } else {
+ let scrollboxPaddingStart = Math.round(this.scrollboxPaddingStart);
+ let scrollboxPaddingEnd = Math.round(this.scrollboxPaddingEnd);
+ let [start, end] = this._startEndProps;
+ if (this._isRTLScrollbox) {
+ [start, end] = [end, start];
+ scrollboxPaddingStart = -scrollboxPaddingStart;
+ scrollboxPaddingEnd = -scrollboxPaddingEnd;
+ }
- if (scrolledToStart)
- this.setAttribute("scrolledtostart", "true");
- else
- this.removeAttribute("scrolledtostart");
+ let scrollboxRect = this._boundsWithoutFlushing(this._scrollbox);
+ let elements = this._getScrollableElements();
+ let [firstElement, lastElement] = [elements[0], elements[elements.length - 1]];
+
+ if (firstElement &&
+ Math.round(this._boundsWithoutFlushing(firstElement)[start]) - scrollboxPaddingStart ==
+ Math.round(scrollboxRect[start])) {
+ scrolledToStart = true;
+ } else if (lastElement &&
+ Math.round(this._boundsWithoutFlushing(lastElement)[end]) + scrollboxPaddingEnd ==
+ Math.round(scrollboxRect[end])) {
+ scrolledToEnd = true;
+ }
+ }
+
+ if (scrolledToEnd) {
+ this.setAttribute("scrolledtoend", "true");
+ } else {
+ this.removeAttribute("scrolledtoend");
+ }
+
+ if (scrolledToStart) {
+ this.setAttribute("scrolledtostart", "true");
+ } else {
+ this.removeAttribute("scrolledtostart");
+ }
+ }, 0);
+ });
]]></body>
</method>
</implementation>
<handlers>
<handler event="wheel"><![CDATA[
let doScroll = false;
let useSmoothScroll = event.deltaMode != event.DOM_DELTA_PIXEL && this.smoothScroll;
@@ -815,37 +850,17 @@
// as the whole overflow event should not be happening in that case.
this._updateScrollButtonsDisabledState();
} catch (e) {
this.setAttribute("notoverflowing", "true");
}
]]></handler>
<handler event="scroll"><![CDATA[
- if (!this._delayedUpdateScrollButtonsTimer) {
- // This is the beginning of a scrolling animation. We need to update
- // scroll buttons now in case we were scrolled to the start or to the
- // end before we started scrolling.
- this._updateScrollButtonsDisabledState(true);
- } else {
- // We're in the middle of the scrolling animation. We'll restart the
- // delayed update request so that we only update the scroll buttons
- // a second time once we're done scrolling.
- window.clearTimeout(this._delayedUpdateScrollButtonsTimer);
- }
-
- // Try to detect the end of the scrolling animation to update the
- // scroll buttons again. To avoid false positives, this timeout needs
- // to be big enough to account for intermediate frames that don't move
- // the scroll position in case we're scrolling slowly.
- this._delayedUpdateScrollButtonsTimer = setTimeout(() => {
- // Scrolling animation has finished.
- this._delayedUpdateScrollButtonsTimer = 0;
- this._updateScrollButtonsDisabledState();
- }, 200);
+ this._updateScrollButtonsDisabledState();
]]></handler>
</handlers>
</binding>
<binding id="autorepeatbutton" extends="chrome://global/content/bindings/scrollbox.xml#scrollbox-base">
<content repeat="hover">
<xul:image class="autorepeatbutton-icon"/>
</content>