Bug 1462911 - allow for new overflown items to be the first item in the overflow list, r=jaws a=pascalc
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 26 Mar 2019 19:14:27 +0000
changeset 525823 cb8b50a3dc6addd0e25b0de62802bbbcbaa8bd8a
parent 525822 381133904780a376d4991883a605457b2faecc86
child 525824 e0b94caff888c14d80461bfa414a90ac11515184
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, pascalc
bugs1462911
milestone67.0
Bug 1462911 - allow for new overflown items to be the first item in the overflow list, r=jaws a=pascalc Differential Revision: https://phabricator.services.mozilla.com/D24944
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/test/browser_914138_widget_API_overflowable_toolbar.js
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -4653,22 +4653,25 @@ OverflowableToolbar.prototype = {
     }
 
     let nowInBar = aNode.parentNode == aContainer;
     let nowOverflowed = aNode.parentNode == this._list;
     let wasOverflowed = this._collapsed.has(aNode.id);
 
     // If this wasn't overflowed before...
     if (!wasOverflowed) {
-      // ... but it is now, then we added to the overflow panel. Exciting stuff:
+      // ... but it is now, then we added to the overflow panel.
       if (nowOverflowed) {
-        // NB: we're guaranteed that it has a previousSibling, because if it didn't,
-        // we would have added it to the toolbar instead. See getOverflowedNextNode.
-        let prevId = aNode.previousElementSibling.id;
-        let minSize = this._collapsed.get(prevId);
+        // We could be the first item in the overflow panel if we're being inserted
+        // before the previous first item in it. We can't assume the minimum
+        // size is the same (because the other item might be much wider), so if
+        // there is no previous item, just allow this item to be put back in the
+        // toolbar immediately by specifying a very low minimum size.
+        let sourceOfMinSize = aNode.previousElementSibling;
+        let minSize = sourceOfMinSize ? this._collapsed.get(sourceOfMinSize.id) : 1;
         this._collapsed.set(aNode.id, minSize);
         aNode.setAttribute("cui-anchorid", this._chevron.id);
         aNode.setAttribute("overflowedItem", true);
         CustomizableUIInternal.ensureButtonContextMenu(aNode, aContainer, true);
         CustomizableUIInternal.notifyListeners("onWidgetOverflow", aNode, this._target);
       } else if (!nowInBar) {
         // If it is not overflowed and not in the toolbar, and was not overflowed
         // either, it moved out of the toolbar. That means there's now space in there!
--- a/browser/components/customizableui/test/browser_914138_widget_API_overflowable_toolbar.js
+++ b/browser/components/customizableui/test/browser_914138_widget_API_overflowable_toolbar.js
@@ -5,31 +5,37 @@
 "use strict";
 
 var navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
 var overflowList = document.getElementById(navbar.getAttribute("overflowtarget"));
 
 const kTestBtn1 = "test-addWidgetToArea-overflow";
 const kTestBtn2 = "test-removeWidgetFromArea-overflow";
 const kTestBtn3 = "test-createWidget-overflow";
+const kTestBtn4 = "test-createWidget-overflow-first-item";
+const kTestBtn5 = "test-addWidgetToArea-overflow-first-item";
 const kSidebarBtn = "sidebar-button";
+const kLibraryButton = "library-button";
 const kDownloadsBtn = "downloads-button";
 const kSearchBox = "search-container";
 
 var originalWindowWidth;
 
 // Adding a widget should add it next to the widget it's being inserted next to.
 add_task(async function() {
   originalWindowWidth = window.outerWidth;
   createDummyXULButton(kTestBtn1, "Test");
   ok(!navbar.hasAttribute("overflowing"), "Should start with a non-overflowing toolbar.");
   ok(CustomizableUI.inDefaultState, "Should start in default state.");
 
   window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
-  await TestUtils.waitForCondition(() => navbar.hasAttribute("overflowing"));
+  await TestUtils.waitForCondition(() => {
+    return navbar.hasAttribute("overflowing") &&
+      document.getElementById(kSidebarBtn).getAttribute("overflowedItem") == "true";
+  });
   ok(navbar.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
   ok(!navbar.querySelector("#" + kSidebarBtn), "Sidebar button should no longer be in the navbar");
   let sidebarBtnNode = overflowList.querySelector("#" + kSidebarBtn);
   ok(sidebarBtnNode, "Sidebar button should be overflowing");
   ok(sidebarBtnNode && sidebarBtnNode.getAttribute("overflowedItem") == "true", "Sidebar button should have overflowedItem attribute");
 
   let placementOfSidebarButton = CustomizableUI.getWidgetIdsInArea(navbar.id).indexOf(kSidebarBtn);
   CustomizableUI.addWidgetToArea(kTestBtn1, navbar.id, placementOfSidebarButton);
@@ -50,16 +56,17 @@ add_task(async function() {
   ok(!overflowList.querySelector("#" + kTestBtn1), "Test button should no longer be overflowing");
   ok(newButtonNode && (newButtonNode.getAttribute("overflowedItem") != "true"), "New button should no longer have overflowedItem attribute");
   let el = document.getElementById(kTestBtn1);
   if (el) {
     CustomizableUI.removeWidgetFromArea(kTestBtn1);
     el.remove();
   }
   window.resizeTo(originalWindowWidth, window.outerHeight);
+  await TestUtils.waitForCondition(() => !navbar.hasAttribute("overflowing"));
 });
 
 // Removing a widget should remove it from the overflow list if that is where it is, and update it accordingly.
 add_task(async function() {
   createDummyXULButton(kTestBtn2, "Test");
   ok(!navbar.hasAttribute("overflowing"), "Should start with a non-overflowing toolbar.");
   ok(CustomizableUI.inDefaultState, "Should start in default state.");
   CustomizableUI.addWidgetToArea(kTestBtn2, navbar.id);
@@ -81,26 +88,30 @@ add_task(async function() {
   await TestUtils.waitForCondition(() => !navbar.hasAttribute("overflowing"));
   ok(!navbar.hasAttribute("overflowing"), "Should not have an overflowing toolbar.");
   let el = document.getElementById(kTestBtn2);
   if (el) {
     CustomizableUI.removeWidgetFromArea(kTestBtn2);
     el.remove();
   }
   window.resizeTo(originalWindowWidth, window.outerHeight);
+  await TestUtils.waitForCondition(() => !navbar.hasAttribute("overflowing"));
 });
 
 // Constructing a widget while overflown should set the right class on it.
 add_task(async function() {
   originalWindowWidth = window.outerWidth;
   ok(!navbar.hasAttribute("overflowing"), "Should start with a non-overflowing toolbar.");
   ok(CustomizableUI.inDefaultState, "Should start in default state.");
 
   window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
-  await TestUtils.waitForCondition(() => navbar.hasAttribute("overflowing"));
+  await TestUtils.waitForCondition(() => {
+    return navbar.hasAttribute("overflowing") &&
+      document.getElementById(kSidebarBtn).getAttribute("overflowedItem") == "true";
+  });
   ok(navbar.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
   ok(!navbar.querySelector("#" + kSidebarBtn), "Sidebar button should no longer be in the navbar");
   let sidebarBtnNode = overflowList.querySelector("#" + kSidebarBtn);
   ok(sidebarBtnNode, "Sidebar button should be overflowing");
   ok(sidebarBtnNode && sidebarBtnNode.getAttribute("overflowedItem") == "true", "Sidebar button should have overflowedItem class");
 
   let testBtnSpec = {id: kTestBtn3, label: "Overflowable widget test", defaultArea: "nav-bar"};
   CustomizableUI.createWidget(testBtnSpec);
@@ -117,14 +128,66 @@ add_task(async function() {
   ok(testNode, "Test button should be overflowing");
   ok(testNode && testNode.getAttribute("overflowedItem") == "true", "Test button should have overflowedItem class");
 
   CustomizableUI.removeWidgetFromArea(kTestBtn3);
   testNode = document.getElementById(kTestBtn3);
   ok(!testNode, "Test button should be gone");
   CustomizableUI.destroyWidget(kTestBtn3);
   window.resizeTo(originalWindowWidth, window.outerHeight);
+  await TestUtils.waitForCondition(() => !navbar.hasAttribute("overflowing"));
 });
 
-add_task(async function asyncCleanup() {
+add_task(async function insertBeforeFirstItemInOverflow() {
+  originalWindowWidth = window.outerWidth;
+
+  ok(!navbar.hasAttribute("overflowing"), "Should start with a non-overflowing toolbar.");
+  ok(CustomizableUI.inDefaultState, "Should start in default state.");
+  // Ensure nothing flexes to make the resize predictable:
+  navbar.querySelectorAll("toolbarspring").forEach(s => CustomizableUI.removeWidgetFromArea(s.id));
+  let urlbar = document.getElementById("urlbar-container");
+  urlbar.style.minWidth = urlbar.getBoundingClientRect().width + "px";
+  let libraryButton = document.getElementById(kLibraryButton);
+  // Negative number to make the window smaller by the difference between the left side of
+  // the item next to the library button and left side of the hamburger one.
+  // The width of the overflow button that needs to appear will then be enough to
+  // also hide the library button.
+  let resizeWidthToMakeLibraryLast =
+    libraryButton.nextElementSibling.getBoundingClientRect().left -
+    PanelUI.menuButton.parentNode.getBoundingClientRect().left +
+    10; // Leave some margin for the margins between buttons etc.;
+  window.resizeBy(resizeWidthToMakeLibraryLast, 0);
+  await TestUtils.waitForCondition(() => {
+    return libraryButton.getAttribute("overflowedItem") == "true" &&
+           !libraryButton.previousElementSibling;
+  });
+
+  let testBtnSpec = {id: kTestBtn4, label: "Overflowable widget test"};
+  let placementOfLibraryButton = CustomizableUI.getWidgetIdsInArea(navbar.id).indexOf(kLibraryButton);
+  CustomizableUI.createWidget(testBtnSpec);
+  CustomizableUI.addWidgetToArea(kTestBtn4, "nav-bar", placementOfLibraryButton);
+  let testNode = overflowList.querySelector("#" + kTestBtn4);
+  ok(testNode, "Test button should be overflowing");
+  ok(testNode && testNode.getAttribute("overflowedItem") == "true", "Test button should have overflowedItem class");
+  CustomizableUI.destroyWidget(kTestBtn4);
+  testNode = document.getElementById(kTestBtn4);
+  ok(!testNode, "Test button should be gone");
+
+  createDummyXULButton(kTestBtn5, "Test");
+  CustomizableUI.addWidgetToArea(kTestBtn5, "nav-bar", placementOfLibraryButton);
+  testNode = overflowList.querySelector("#" + kTestBtn5);
+  ok(testNode, "Test button should be overflowing");
+  ok(testNode && testNode.getAttribute("overflowedItem") == "true", "Test button should have overflowedItem class");
+  CustomizableUI.removeWidgetFromArea(kTestBtn5);
+  testNode && testNode.remove();
+
+  urlbar.style.removeProperty("min-width");
   window.resizeTo(originalWindowWidth, window.outerHeight);
+  await TestUtils.waitForCondition(() => !navbar.hasAttribute("overflowing"));
   await resetCustomization();
 });
+
+registerCleanupFunction(async function asyncCleanup() {
+  document.getElementById("urlbar-container").style.removeProperty("min-width");
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+  await TestUtils.waitForCondition(() => !navbar.hasAttribute("overflowing"));
+  await resetCustomization();
+});