Bug 1454360 - Use "arrowscrollbox" in the "popup-scrollbars" binding. r=NeilDeakin
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Fri, 04 Jan 2019 15:07:12 +0000
changeset 510040 24b9b8cb6c615c78895f5007f4f81a06dbe5df06
parent 510039 55d05fe0a947fd42522299005df14b283d148d6d
child 510041 2d69841d2eb77fd7f296a09b979c96a9794ff6f6
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersNeilDeakin
bugs1454360
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1454360 - Use "arrowscrollbox" in the "popup-scrollbars" binding. r=NeilDeakin This prepares this binding for the unification with the "popup" binding, and removes the last consumer of the scrollByIndex method of XULScrollElement. Because some code paths in "arrowscrollbox" are optimized using requestAnimationFrame, the related scrolling tests are now asynchronous. Differential Revision: https://phabricator.services.mozilla.com/D15276
browser/base/content/tabbrowser.xml
browser/base/content/test/forms/browser_selectpopup.js
browser/base/content/test/tabs/browser_overflowScroll.js
layout/xul/nsMenuFrame.cpp
layout/xul/nsMenuPopupFrame.cpp
toolkit/content/tests/chrome/test_menulist.xul
toolkit/content/tests/chrome/test_mousescroll.xul
toolkit/content/tests/chrome/window_largemenu.xul
toolkit/content/widgets/popup.xml
toolkit/content/widgets/scrollbox.xml
toolkit/content/xul.css
toolkit/modules/SelectParentHelper.jsm
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -29,17 +29,17 @@
 
     <handlers>
       <handler event="underflow" phase="capturing"><![CDATA[
         // Ignore underflow events:
         // - from nested scrollable elements
         // - for vertical orientation
         // - corresponding to an overflow event that we ignored
         let tabs = document.getBindingParent(this);
-        if (event.originalTarget != this._scrollbox ||
+        if (event.originalTarget != this.scrollbox ||
             event.detail == 0 ||
             !tabs.hasAttribute("overflow")) {
           return;
         }
 
         tabs.removeAttribute("overflow");
 
         if (tabs._lastTabClosedByMouse) {
@@ -51,17 +51,17 @@
         }
 
         tabs._positionPinnedTabs();
       ]]></handler>
       <handler event="overflow"><![CDATA[
         // Ignore overflow events:
         // - from nested scrollable elements
         // - for vertical orientation
-        if (event.originalTarget != this._scrollbox ||
+        if (event.originalTarget != this.scrollbox ||
             event.detail == 0) {
           return;
         }
 
         var tabs = document.getBindingParent(this);
         tabs.setAttribute("overflow", "true");
         tabs._positionPinnedTabs();
         tabs._handleTabSelect(true);
@@ -629,17 +629,17 @@
           // Move the dragged tab based on the mouse position.
 
           let leftTab = tabs[0];
           let rightTab = tabs[tabs.length - 1];
           let rightMovingTabScreenX = movingTabs[movingTabs.length - 1].boxObject.screenX;
           let leftMovingTabScreenX = movingTabs[0].boxObject.screenX;
           let translateX = screenX - draggedTab._dragData.screenX;
           if (!pinned) {
-            translateX += this.arrowScrollbox._scrollbox.scrollLeft - draggedTab._dragData.scrollX;
+            translateX += this.arrowScrollbox.scrollbox.scrollLeft - draggedTab._dragData.scrollX;
           }
           let leftBound = leftTab.boxObject.screenX - leftMovingTabScreenX;
           let rightBound = (rightTab.boxObject.screenX + rightTab.boxObject.width) -
                            (rightMovingTabScreenX + tabWidth);
           translateX = Math.min(Math.max(translateX, leftBound), rightBound);
 
           for (let tab of movingTabs) {
             tab.style.transform = "translateX(" + translateX + "px)";
@@ -1550,17 +1550,17 @@
         // relative to the corner of the dragged tab.
         function clientX(ele) {
           return ele.getBoundingClientRect().left;
         }
         let tabOffsetX = clientX(tab) - clientX(this);
         tab._dragData = {
           offsetX: event.screenX - window.screenX - tabOffsetX,
           offsetY: event.screenY - window.screenY,
-          scrollX: this.arrowScrollbox._scrollbox.scrollLeft,
+          scrollX: this.arrowScrollbox.scrollbox.scrollLeft,
           screenX: event.screenX,
           movingTabs: (tab.multiselected ? gBrowser.selectedTabs : [tab])
                       .filter(t => t.pinned == tab.pinned),
         };
 
         event.stopPropagation();
       ]]></handler>
 
--- a/browser/base/content/test/forms/browser_selectpopup.js
+++ b/browser/base/content/test/forms/browser_selectpopup.js
@@ -457,61 +457,74 @@ async function performLargePopupTests(wi
   });
 
   let selectPopup = win.document.getElementById("ContentSelectDropdown").menupopup;
   let browserRect = browser.getBoundingClientRect();
 
   // Check if a drag-select works and scrolls the list.
   await openSelectPopup(selectPopup, "mousedown", "select", win);
 
-  let scrollPos = selectPopup.scrollBox.scrollTop;
+  let getScrollPos = () => selectPopup.scrollBox.scrollbox.scrollTop;
+  let scrollPos = getScrollPos();
   let popupRect = selectPopup.getBoundingClientRect();
 
   // First, check that scrolling does not occur when the mouse is moved over the
   // anchor button but not the popup yet.
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 5, popupRect.top - 10, { type: "mousemove" }, win);
-  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position after mousemove over button should not change");
+  is(getScrollPos(), scrollPos, "scroll position after mousemove over button should not change");
 
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top + 10, { type: "mousemove" }, win);
 
   // Dragging above the popup scrolls it up.
+  let scrolledPromise = BrowserTestUtils.waitForEvent(selectPopup, "scroll", false,
+    () => getScrollPos() < scrollPos - 5);
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" }, win);
-  ok(selectPopup.scrollBox.scrollTop < scrollPos - 5, "scroll position at drag up");
+  await scrolledPromise;
+  ok(true, "scroll position at drag up");
 
   // Dragging below the popup scrolls it down.
-  scrollPos = selectPopup.scrollBox.scrollTop;
+  scrollPos = getScrollPos();
+  scrolledPromise = BrowserTestUtils.waitForEvent(selectPopup, "scroll", false,
+    () => getScrollPos() > scrollPos + 5);
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" }, win);
-  ok(selectPopup.scrollBox.scrollTop > scrollPos + 5, "scroll position at drag down");
+  await scrolledPromise;
+  ok(true, "scroll position at drag down");
 
   // Releasing the mouse button and moving the mouse does not change the scroll position.
-  scrollPos = selectPopup.scrollBox.scrollTop;
+  scrollPos = getScrollPos();
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" }, win);
-  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at mouseup should not change");
+  is(getScrollPos(), scrollPos, "scroll position at mouseup should not change");
 
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" }, win);
-  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at mousemove after mouseup should not change");
+  is(getScrollPos(), scrollPos, "scroll position at mousemove after mouseup should not change");
 
   // Now check dragging with a mousedown on an item
   let menuRect = selectPopup.children[51].getBoundingClientRect();
   EventUtils.synthesizeMouseAtPoint(menuRect.left + 5, menuRect.top + 5, { type: "mousedown" }, win);
 
   // Dragging below the popup scrolls it down.
+  scrolledPromise = BrowserTestUtils.waitForEvent(selectPopup, "scroll", false,
+    () => getScrollPos() > scrollPos + 5);
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" }, win);
-  ok(selectPopup.scrollBox.scrollTop > scrollPos + 5, "scroll position at drag down from option");
+  await scrolledPromise;
+  ok(true, "scroll position at drag down from option");
 
   // Dragging above the popup scrolls it up.
+  scrolledPromise = BrowserTestUtils.waitForEvent(selectPopup, "scroll", false,
+    () => getScrollPos() < scrollPos - 5);
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" }, win);
-  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at drag up from option");
+  await scrolledPromise;
+  ok(true, "scroll position at drag up from option");
 
-  scrollPos = selectPopup.scrollBox.scrollTop;
+  scrollPos = getScrollPos();
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" }, win);
-  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at mouseup from option should not change");
+  is(getScrollPos(), scrollPos, "scroll position at mouseup from option should not change");
 
   EventUtils.synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" }, win);
-  is(selectPopup.scrollBox.scrollTop, scrollPos, "scroll position at mousemove after mouseup should not change");
+  is(getScrollPos(), scrollPos, "scroll position at mousemove after mouseup should not change");
 
   await hideSelectPopup(selectPopup, "escape", win);
 
   let positions = [
     "margin-top: 300px;",
     "position: fixed; bottom: 200px;",
     "width: 100%; height: 9999px;",
   ];
--- a/browser/base/content/test/tabs/browser_overflowScroll.js
+++ b/browser/base/content/test/tabs/browser_overflowScroll.js
@@ -3,17 +3,17 @@
 requestLongerTimeout(2);
 
 /**
  * Tests that scrolling the tab strip via the scroll buttons scrolls the right
  * amount in non-smoothscroll mode.
  */
 add_task(async function() {
   let arrowScrollbox = gBrowser.tabContainer.arrowScrollbox;
-  let scrollbox = arrowScrollbox._scrollbox;
+  let scrollbox = arrowScrollbox.scrollbox;
   let originalSmoothScroll = arrowScrollbox.smoothScroll;
   let tabs = gBrowser.tabs;
   let tabMinWidth = parseInt(getComputedStyle(gBrowser.selectedTab, null).minWidth);
 
   let rect = ele => ele.getBoundingClientRect();
   let width = ele => rect(ele).width;
 
   let tabCountForOverflow = Math.ceil(width(arrowScrollbox) / tabMinWidth * 3);
--- a/layout/xul/nsMenuFrame.cpp
+++ b/layout/xul/nsMenuFrame.cpp
@@ -1203,18 +1203,17 @@ bool nsMenuFrame::SizeToPopup(nsBoxLayou
       //      border-padding
       //  (3) there's enough room in the popup for the content and its
       //      scrollbar
       nsMargin borderPadding;
       GetXULBorderAndPadding(borderPadding);
 
       // if there is a scroll frame, add the desired width of the scrollbar as
       // well
-      nsIScrollableFrame* scrollFrame =
-          do_QueryFrame(popupFrame->PrincipalChildList().FirstChild());
+      nsIScrollableFrame* scrollFrame = popupFrame->GetScrollFrame(popupFrame);
       nscoord scrollbarWidth = 0;
       if (scrollFrame) {
         scrollbarWidth =
             scrollFrame->GetDesiredScrollbarSizes(&aState).LeftRight();
       }
 
       aSize.width =
           tmpSize.width + std::max(borderPadding.LeftRight(), scrollbarWidth);
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -1043,18 +1043,17 @@ nsPoint nsMenuPopupFrame::AdjustPosition
 
     // Only adjust the popup if it just opened, otherwise the popup will move
     // around if its gets resized or the selection changed. Cache the value in
     // mPositionedOffset and use that instead for any future calculations.
     if (mIsOpenChanged || mReflowCallbackData.mIsOpenChanged) {
       nsIFrame* selectedItemFrame = GetSelectedItemForAlignment();
       if (selectedItemFrame) {
         int32_t scrolly = 0;
-        nsIScrollableFrame* scrollframe =
-            do_QueryFrame(nsBox::GetChildXULBox(this));
+        nsIScrollableFrame* scrollframe = GetScrollFrame(this);
         if (scrollframe) {
           scrolly = scrollframe->GetScrollPosition().y;
         }
 
         mPositionedOffset = originalAnchorRect.height +
                             selectedItemFrame->GetRect().y - scrolly;
       }
     }
--- a/toolkit/content/tests/chrome/test_menulist.xul
+++ b/toolkit/content/tests/chrome/test_menulist.xul
@@ -16,17 +16,17 @@
     <menupopup id="menulist-popup"/>
   </menulist>
   <button label="Two"/>
   <button label="Three"/>
 </vbox>
 <richlistbox id="scroller-in-listbox" style="overflow: auto" height="60">
   <richlistitem allowevents="true">
     <menulist id="menulist-in-listbox" onpopupshown="test_menulist_open(this, this.parentNode.parentNode)"
-              onpopuphidden="SimpleTest.executeSoon(checkScrollAndFinish)">
+              onpopuphidden="SimpleTest.executeSoon(() => checkScrollAndFinish().catch(ex => ok(false, ex)));">
       <menupopup id="menulist-in-listbox-popup">
         <menuitem label="One" value="one"/>
         <menuitem label="Two" value="two"/>
       </menupopup>
     </menulist>
   </richlistitem>
   <richlistitem><label value="Two"/></richlistitem>
   <richlistitem><label value="Three"/></richlistitem>
@@ -69,16 +69,28 @@
     <menuitem label="Seven" value="seven"/>
     <menuitem label="Eight" value="eight"/>
   </menupopup>
 </menulist>
 
 <script class="testbody" type="application/javascript">
 <![CDATA[
 
+function waitForEvent(subject, eventName, checkFn) {
+  return new Promise(resolve => {
+    subject.addEventListener(eventName, function listener(event) {
+      if (checkFn && !checkFn(event)) {
+        return;
+      }
+      subject.removeEventListener(eventName, listener);
+      SimpleTest.executeSoon(() => resolve(event));
+    });
+  });
+}
+
 SimpleTest.waitForExplicitFinish();
 
 function testtag_menulists()
 {
   testtag_menulist_UI_start($("menulist"), false);
 }
 
 function testtag_menulist_UI_start(element)
@@ -220,102 +232,115 @@ function test_menulist_open(element, scr
   synthesizeMouse(element.menupopup.childNodes[1], 6, 6, { type: "mousemove" });
   is(element.activeChild, item, "activeChild after menu highlight " + element.id);
   is(element.selectedIndex, 0, "selectedIndex after menu highlight " + element.id);
   is(scroller.scrollTop, 0, "scroll position after menu highlight " + element.id);
 
   element.open = false;
 }
 
-function checkScrollAndFinish()
+async function checkScrollAndFinish()
 {
   is($("scroller").scrollTop, 0, "mousewheel on menulist does not scroll vbox parent");
   is($("scroller-in-listbox").scrollTop, 0, "mousewheel on menulist does not scroll listbox parent");
 
   let menulist = $("menulist-size");
-  menulist.addEventListener("popupshown", function testAltClose() {
-    menulist.removeEventListener("popupshown", testAltClose);
+  let shownPromise = waitForEvent(menulist, "popupshown");
+  menulist.open = true;
+  await shownPromise;
 
-    sendKey("ALT");
-    is(menulist.menupopup.state, "open", "alt doesn't close menulist");
-    menulist.open = false;
+  sendKey("ALT");
+  is(menulist.menupopup.state, "open", "alt doesn't close menulist");
+  menulist.open = false;
 
-    dragScroll();
-  });
-
-  menulist.open = true;
+  await dragScroll();
 }
 
-function dragScroll()
+async function dragScroll()
 {
   let menulist = $("menulist-clipped");
-  menulist.addEventListener("popupshown", function testDragScroll() {
-    menulist.removeEventListener("popupshown", testDragScroll);
 
-    let popup = menulist.menupopup;
-    let scrollPos = popup.scrollBox.scrollTop;
-    let popupRect = popup.getBoundingClientRect();
+  let shownPromise = waitForEvent(menulist, "popupshown");
+  menulist.open = true;
+  await shownPromise;
 
-    // First, check that scrolling does not occur when the mouse is moved over the
-    // anchor button but not the popup yet.
-    synthesizeMouseAtPoint(popupRect.left + 5, popupRect.top - 10, { type: "mousemove" });
-    is(popup.scrollBox.scrollTop, scrollPos, "scroll position after mousemove over button should not change");
+  let popup = menulist.menupopup;
+  let getScrollPos = () => popup.scrollBox.scrollbox.scrollTop;
+  let scrollPos = getScrollPos();
+  let popupRect = popup.getBoundingClientRect();
 
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top + 10, { type: "mousemove" });
+  // First, check that scrolling does not occur when the mouse is moved over the
+  // anchor button but not the popup yet.
+  synthesizeMouseAtPoint(popupRect.left + 5, popupRect.top - 10, { type: "mousemove" });
+  is(getScrollPos(), scrollPos, "scroll position after mousemove over button should not change");
+
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top + 10, { type: "mousemove" });
 
-    // Dragging above the popup scrolls it up.
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" });
-    ok(popup.scrollBox.scrollTop < scrollPos - 5, "scroll position at drag up");
+  // Dragging above the popup scrolls it up.
+  let scrolledPromise = waitForEvent(popup, "scroll", false,
+    () => getScrollPos() < scrollPos - 5);
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" });
+  await scrolledPromise;
+  ok(true, "scroll position at drag up");
 
-    // Dragging below the popup scrolls it down.
-    scrollPos = popup.scrollBox.scrollTop;
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
-    ok(popup.scrollBox.scrollTop > scrollPos + 5, "scroll position at drag down");
+  // Dragging below the popup scrolls it down.
+  scrollPos = getScrollPos();
+  scrolledPromise = waitForEvent(popup, "scroll", false,
+    () => getScrollPos() > scrollPos + 5);
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
+  await scrolledPromise;
+  ok(true, "scroll position at drag down");
 
-    // Releasing the mouse button and moving the mouse does not change the scroll position.
-    scrollPos = popup.scrollBox.scrollTop;
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" });
-    is(popup.scrollBox.scrollTop, scrollPos, "scroll position at mouseup should not change");
+  // Releasing the mouse button and moving the mouse does not change the scroll position.
+  scrollPos = getScrollPos();
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" });
+  is(getScrollPos(), scrollPos, "scroll position at mouseup should not change");
 
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
-    is(popup.scrollBox.scrollTop, scrollPos, "scroll position at mousemove after mouseup should not change");
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
+  is(getScrollPos(), scrollPos, "scroll position at mousemove after mouseup should not change");
 
-    // Now check dragging with a mousedown on an item
-    let menuRect = popup.childNodes[4].getBoundingClientRect();
-    synthesizeMouseAtPoint(menuRect.left + 5, menuRect.top + 5, { type: "mousedown" });
-
-    // Dragging below the popup scrolls it down.
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
-    ok(popup.scrollBox.scrollTop > scrollPos + 5, "scroll position at drag down from item");
+  // Now check dragging with a mousedown on an item. Make sure the element is
+  // visible, as the asynchronous scrolling may have moved it out of view.
+  popup.childNodes[4].scrollIntoView({ block: "nearest", behavior: "instant" });
+  let menuRect = popup.childNodes[4].getBoundingClientRect();
+  synthesizeMouseAtPoint(menuRect.left + 5, menuRect.top + 5, { type: "mousedown" });
 
-    // Dragging above the popup scrolls it up.
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" });
-    is(popup.scrollBox.scrollTop, scrollPos, "scroll position at drag up from item");
-
-    scrollPos = popup.scrollBox.scrollTop;
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" });
-    is(popup.scrollBox.scrollTop, scrollPos, "scroll position at mouseup should not change");
+  // Dragging below the popup scrolls it down.
+  scrolledPromise = waitForEvent(popup, "scroll", false,
+    () => getScrollPos() > scrollPos + 5);
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
+  await scrolledPromise;
+  ok(true, "scroll position at drag down from item");
 
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
-    is(popup.scrollBox.scrollTop, scrollPos, "scroll position at mousemove after mouseup should not change");
+  // Dragging above the popup scrolls it up.
+  scrollPos = getScrollPos();
+  scrolledPromise = waitForEvent(popup, "scroll", false,
+    () => getScrollPos() < scrollPos - 5);
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.top - 20, { type: "mousemove" });
+  await scrolledPromise;
+  ok(true, "scroll position at drag up from item");
 
-    menulist.open = false;
+  scrollPos = getScrollPos();
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 25, { type: "mouseup" });
+  is(getScrollPos(), scrollPos, "scroll position at mouseup should not change");
 
-    let mouseMoveTarget = null;
-    popup.childNodes[4].click();
-    addEventListener("mousemove", function checkMouseMove(event) {
-      mouseMoveTarget = event.target;
-    }, {once: true});
-    synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
-    isnot(mouseMoveTarget, popup, "clicking on item when popup closed doesn't start dragging");
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
+  is(getScrollPos(), scrollPos, "scroll position at mousemove after mouseup should not change");
+
+  menulist.open = false;
 
-    SimpleTest.finish();
-  });
+  let mouseMoveTarget = null;
+  popup.childNodes[4].click();
+  addEventListener("mousemove", function checkMouseMove(event) {
+    mouseMoveTarget = event.target;
+  }, {once: true});
+  synthesizeMouseAtPoint(popupRect.left + 20, popupRect.bottom + 20, { type: "mousemove" });
+  isnot(mouseMoveTarget, popup, "clicking on item when popup closed doesn't start dragging");
 
-  menulist.open = true;
+  SimpleTest.finish();
 }
 
 ]]>
 </script>
 
 <body xmlns="http://www.w3.org/1999/xhtml">
 <p id="display">
 </p>
--- a/toolkit/content/tests/chrome/test_mousescroll.xul
+++ b/toolkit/content/tests/chrome/test_mousescroll.xul
@@ -144,17 +144,17 @@ function* testRichListbox(id)
     yield* helper(5, -delta,  0, deltaModes[i]);
     yield* helper(5,  delta,  1, deltaModes[i]);
     yield* helper(5,  delta,  0, deltaModes[i]);
   }
 }
 
 function* testArrowScrollbox(id)
 {
-  var scrollbox = document.getElementById(id)._scrollbox;
+  var scrollbox = document.getElementById(id).scrollbox;
   var orient = scrollbox.getAttribute("orient");
 
   function* helper(aStart, aDelta, aDeltaMode, aExpected)
   {
     var lineOrPageDelta = (aDeltaMode == WheelEvent.DOM_DELTA_PIXEL) ? aDelta / 10 : aDelta;
     var orientIsHorizontal = (orient == "horizontal");
 
     scrollbox.scrollTo(aStart, aStart);
--- a/toolkit/content/tests/chrome/window_largemenu.xul
+++ b/toolkit/content/tests/chrome/window_largemenu.xul
@@ -98,17 +98,17 @@ function popupShown()
   if (gTests[gTestIndex] == "menu movement")
     return testPopupMovement();
 
   if (gContextMenuTests)
     return contextMenuPopupShown();
 
   var popup = document.getElementById("popup");
   var rect = popup.getBoundingClientRect();
-  var scrollbox = document.getAnonymousNodes(popup)[0]._scrollbox;
+  var scrollbox = document.getAnonymousNodes(popup)[0].scrollbox;
   var expectedScrollPos = 0;
 
   if (gTestIndex == 0) {
     // the popup should be in the center of the screen
     // note that if the height is odd, the y-offset will have been rounded
     // down when we pass the fractional value to openPopupAtScreen above.
     is(Math.round(rect.top) + gScreenY, Math.floor(screen.height / 2),
                               gTests[gTestIndex] + " top");
--- a/toolkit/content/widgets/popup.xml
+++ b/toolkit/content/widgets/popup.xml
@@ -260,19 +260,20 @@
       <handler event="popuppositioned" phase="target">
         this.adjustArrowPosition();
       </handler>
     </handlers>
   </binding>
 
   <binding id="popup-scrollbars" extends="chrome://global/content/bindings/popup.xml#popup">
     <content>
-      <xul:scrollbox class="popup-internal-box" flex="1" orient="vertical" style="overflow: auto;">
+      <xul:arrowscrollbox class="popup-internal-box" flex="1" orient="vertical"
+                          smoothscroll="false">
         <children/>
-      </xul:scrollbox>
+      </xul:arrowscrollbox>
     </content>
     <implementation>
       <field name="AUTOSCROLL_INTERVAL">25</field>
       <field name="NOT_DRAGGING">0</field>
       <field name="DRAG_OVER_BUTTON">-1</field>
       <field name="DRAG_OVER_POPUP">1</field>
 
       <field name="_draggingState">this.NOT_DRAGGING</field>
@@ -363,21 +364,21 @@
             if (event.screenY > popupRect.top && event.screenY < popupRect.bottom) {
               this._draggingState = this.DRAG_OVER_POPUP;
             }
           }
 
           if (this._draggingState == this.DRAG_OVER_POPUP &&
               (event.screenY <= popupRect.top || event.screenY >= popupRect.bottom)) {
             let scrollAmount = event.screenY <= popupRect.top ? -1 : 1;
-            this.scrollBox.scrollByIndex(scrollAmount);
+            this.scrollBox.scrollByIndex(scrollAmount, true);
 
             let win = this.ownerGlobal;
             this._scrollTimer = win.setInterval(() => {
-              this.scrollBox.scrollByIndex(scrollAmount);
+              this.scrollBox.scrollByIndex(scrollAmount, true);
             }, this.AUTOSCROLL_INTERVAL);
           }
         }
       ]]>
       </handler>
     </handlers>
   </binding>
 
--- a/toolkit/content/widgets/scrollbox.xml
+++ b/toolkit/content/widgets/scrollbox.xml
@@ -61,17 +61,17 @@
       <destructor><![CDATA[
         // Release timer to avoid reference cycles.
         if (this._scrollTimer) {
           this._scrollTimer.cancel();
           this._scrollTimer = null;
         }
       ]]></destructor>
 
-      <field name="_scrollbox">
+      <field name="scrollbox">
         document.getAnonymousElementByAttribute(this, "anonid", "scrollbox");
       </field>
       <field name="_scrollButtonUp">
         document.getAnonymousElementByAttribute(this, "anonid", "scrollbutton-up");
       </field>
       <field name="_scrollButtonDown">
         document.getAnonymousElementByAttribute(this, "anonid", "scrollbutton-down");
       </field>
@@ -148,39 +148,39 @@
         <setter><![CDATA[
           this.setAttribute("smoothscroll", !!val);
           return val;
         ]]></setter>
       </property>
 
       <property name="scrollBoxObject" readonly="true">
         <getter><![CDATA[
-          return this._scrollbox.boxObject;
+          return this.scrollbox.boxObject;
         ]]></getter>
       </property>
 
       <property name="scrollClientRect" readonly="true">
         <getter><![CDATA[
-          return this._scrollbox.getBoundingClientRect();
+          return this.scrollbox.getBoundingClientRect();
         ]]></getter>
       </property>
 
       <property name="scrollClientSize" readonly="true">
         <getter><![CDATA[
           return this.orient == "vertical" ?
-                 this._scrollbox.clientHeight :
-                 this._scrollbox.clientWidth;
+                 this.scrollbox.clientHeight :
+                 this.scrollbox.clientWidth;
         ]]></getter>
       </property>
 
       <property name="scrollSize" readonly="true">
         <getter><![CDATA[
           return this.orient == "vertical" ?
-                 this._scrollbox.scrollHeight :
-                 this._scrollbox.scrollWidth;
+                 this.scrollbox.scrollHeight :
+                 this.scrollbox.scrollWidth;
         ]]></getter>
       </property>
 
       <property name="lineScrollAmount" readonly="true">
         <getter><![CDATA[
           // line scroll amout should be the width (at horizontal scrollbox) or
           // the height (at vertical scrollbox) of the scrolled elements.
           // However, the elements may have different width or height.  So,
@@ -188,28 +188,28 @@
           var elements = this._getScrollableElements();
           return elements.length && (this.scrollSize / elements.length);
         ]]></getter>
       </property>
 
       <property name="scrollPosition" readonly="true">
         <getter><![CDATA[
           return this.orient == "vertical" ?
-                 this._scrollbox.scrollTop :
-                 this._scrollbox.scrollLeft;
+                 this.scrollbox.scrollTop :
+                 this.scrollbox.scrollLeft;
         ]]></getter>
       </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";
+        document.defaultView.getComputedStyle(this.scrollbox).direction == "rtl";
       ]]></field>
 
       <method name="_onButtonClick">
         <parameter name="event"/>
         <body><![CDATA[
           if (this._clickToScroll) {
             this._distanceScroll(event);
           }
@@ -293,17 +293,18 @@
         <body><![CDATA[
           if (!this._canScrollToElement(element))
             return;
 
           if (this._ensureElementIsVisibleAnimationFrame) {
             window.cancelAnimationFrame(this._ensureElementIsVisibleAnimationFrame);
           }
           this._ensureElementIsVisibleAnimationFrame = window.requestAnimationFrame(() => {
-            element.scrollIntoView({ behavior: aInstant ? "instant" : "auto" });
+            element.scrollIntoView({ block: "nearest",
+                                     behavior: aInstant ? "instant" : "auto" });
             this._ensureElementIsVisibleAnimationFrame = 0;
           });
         ]]></body>
       </method>
 
       <method name="scrollByIndex">
         <parameter name="index"/>
         <parameter name="aInstant"/>
@@ -533,17 +534,17 @@
       </method>
 
       <method name="scrollByPixels">
         <parameter name="aPixels"/>
         <parameter name="aInstant"/>
         <body><![CDATA[
           let scrollOptions = { behavior: aInstant ? "instant" : "auto" };
           scrollOptions[this._startEndProps[0]] = aPixels;
-          this._scrollbox.scrollBy(scrollOptions);
+          this.scrollbox.scrollBy(scrollOptions);
         ]]></body>
       </method>
 
       <field name="_prevMouseScrolls">[null, null]</field>
 
       <field name="_touchStart">-1</field>
 
       <field name="_scrollButtonUpdatePending">false</field>
@@ -559,16 +560,21 @@
             return;
           }
           this._scrollButtonUpdatePending = true;
 
           // Wait until after the next paint to get current layout data from
           // getBoundsWithoutFlushing.
           window.requestAnimationFrame(() => {
             setTimeout(() => {
+              if (!this._startEndProps) {
+                // We've been destroyed in the meantime.
+                return;
+              }
+
               this._scrollButtonUpdatePending = false;
 
               let scrolledToStart = false;
               let scrolledToEnd = false;
 
               if (this.hasAttribute("notoverflowing")) {
                 scrolledToStart = true;
                 scrolledToEnd = true;
@@ -579,21 +585,21 @@
 
                 let elements = this._getScrollableElements();
                 let [leftOrTopElement, rightOrBottomElement] = [elements[0], elements[elements.length - 1]];
                 if (this._isRTLScrollbox) {
                   [leftOrTopElement, rightOrBottomElement] = [rightOrBottomElement, leftOrTopElement];
                 }
 
                 if (leftOrTopElement &&
-                    leftOrTopEdge(leftOrTopElement) >= leftOrTopEdge(this._scrollbox)) {
+                    leftOrTopEdge(leftOrTopElement) >= leftOrTopEdge(this.scrollbox)) {
                   scrolledToStart = !this._isRTLScrollbox;
                   scrolledToEnd = this._isRTLScrollbox;
                 } else if (rightOrBottomElement &&
-                           rightOrBottomEdge(rightOrBottomElement) <= rightOrBottomEdge(this._scrollbox)) {
+                           rightOrBottomEdge(rightOrBottomElement) <= rightOrBottomEdge(this.scrollbox)) {
                   scrolledToStart = this._isRTLScrollbox;
                   scrolledToEnd = !this._isRTLScrollbox;
                 }
               }
 
               if (scrolledToEnd) {
                 this.setAttribute("scrolledtoend", "true");
               } else {
--- a/toolkit/content/xul.css
+++ b/toolkit/content/xul.css
@@ -631,16 +631,27 @@ menulist[popuponly="true"] {
   min-height: 0 !important;
   border: 0 !important;
 }
 
 menulist > menupopup > menuitem {
   -moz-binding: url("chrome://global/content/bindings/menu.xml#menuitem-iconic-noaccel");
 }
 
+menulist > menupopup > .popup-internal-box > .scrollbutton-up,
+menulist > menupopup > .popup-internal-box > .arrowscrollbox-overflow-start-indicator,
+menulist > menupopup > .popup-internal-box > .arrowscrollbox-overflow-end-indicator,
+menulist > menupopup > .popup-internal-box > .scrollbutton-down {
+  display: none;
+}
+
+menulist > menupopup > .popup-internal-box > .arrowscrollbox-scrollbox {
+  overflow: auto;
+}
+
 dropmarker > .dropmarker-icon {
   pointer-events: none;
 }
 
 /********** splitter **********/
 
 .tree-splitter {
   width: 0px;
--- a/toolkit/modules/SelectParentHelper.jsm
+++ b/toolkit/modules/SelectParentHelper.jsm
@@ -240,17 +240,17 @@ var SelectParentHelper = {
 
     if (msg.name == "Forms:UpdateDropDown") {
       // Sanity check - we'd better know what the currently
       // opened menulist is, and what browser it belongs to...
       if (!currentMenulist) {
         return;
       }
 
-      let scrollBox = currentMenulist.menupopup.scrollBox;
+      let scrollBox = currentMenulist.menupopup.scrollBox.scrollbox;
       let scrollTop = scrollBox.scrollTop;
 
       let options = msg.data.options;
       let selectedIndex = msg.data.selectedIndex;
       let uaSelectBackgroundColor = msg.data.uaSelectBackgroundColor;
       let uaSelectColor = msg.data.uaSelectColor;
       let selectBackgroundColor = msg.data.selectBackgroundColor;
       let selectColor = msg.data.selectColor;