Bug 1397447 - ensure the button is in the navbar by default, r?mak draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 08 Sep 2017 14:07:05 +0100
changeset 663081 10f163485600bb7c8e002b34cbfda16fd75635db
parent 663080 cd5ac2d2413a3ff6fd1c0990a350f658efbfdfe1
child 663082 83827656291a3a43711c2dcdff72ce08e3ef252f
push id79307
push userbmo:gijskruitbosch+bugs@gmail.com
push dateTue, 12 Sep 2017 15:18:02 +0000
reviewersmak
bugs1397447
milestone57.0a1
Bug 1397447 - ensure the button is in the navbar by default, r?mak MozReview-Commit-ID: H6r3dAEg4r1
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/test/browser_1042100_default_placements_update.js
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -53,17 +53,17 @@ const kSubviewEvents = [
   "ViewShowing",
   "ViewHiding"
 ];
 
 /**
  * The current version. We can use this to auto-add new default widgets as necessary.
  * (would be const but isn't because of testing purposes)
  */
-var kVersion = 10;
+var kVersion = 11;
 
 /**
  * Buttons removed from built-ins by version they were removed. kVersion must be
  * bumped any time a new id is added to this. Use the button id as key, and
  * version the button is removed in as the value.  e.g. "pocket-button": 5
  */
 var ObsoleteBuiltinButtons = {
 };
@@ -313,17 +313,17 @@ var CustomizableUIInternal = {
       // Nuke the old 'loop-call-button' out of orbit.
       CustomizableUI.removeWidgetFromArea("loop-call-button");
     }
 
     if (currentVersion < 4) {
       CustomizableUI.removeWidgetFromArea("loop-button-throttled");
     }
 
-    if (currentVersion < 7 && gSavedState && gSavedState.placements &&
+    if (currentVersion < 7 && gSavedState.placements &&
         gSavedState.placements[CustomizableUI.AREA_NAVBAR]) {
       let placements = gSavedState.placements[CustomizableUI.AREA_NAVBAR];
       let newPlacements = ["back-button", "forward-button", "stop-reload-button", "home-button"];
       for (let button of placements) {
         if (!newPlacements.includes(button)) {
           newPlacements.push(button);
         }
       }
@@ -403,24 +403,55 @@ var CustomizableUIInternal = {
         let bmbIndex = placements.indexOf("bookmarks-menu-button");
         placements.splice(bmbIndex, 1);
         let downloadButtonIndex = placements.indexOf("downloads-button");
         let libraryIndex = downloadButtonIndex == -1 ? bmbIndex : (downloadButtonIndex + 1);
         placements.splice(libraryIndex, 0, "library-button");
       }
     }
 
-    if (currentVersion < 10 && gSavedState && gSavedState.placements) {
+    if (currentVersion < 10 && gSavedState.placements) {
       for (let placements of Object.values(gSavedState.placements)) {
         if (placements.includes("webcompat-reporter-button")) {
           placements.splice(placements.indexOf("webcompat-reporter-button"), 1);
           break;
         }
       }
     }
+
+    // Move the downloads button to the default position in the navbar if it's
+    // not there already.
+    if (currentVersion < 11 && gSavedState.placements) {
+      let navbarPlacements = gSavedState.placements[CustomizableUI.AREA_NAVBAR];
+      // First remove from wherever it currently lives, if anywhere:
+      for (let placements of Object.values(gSavedState.placements)) {
+        let existingIndex = placements.indexOf("downloads-button");
+        if (existingIndex != -1) {
+          placements.splice(existingIndex, 1);
+          break; // It can only be in 1 place, so no point looking elsewhere.
+        }
+      }
+
+      // Now put the button in the navbar in the correct spot:
+      if (navbarPlacements) {
+        let insertionPoint = navbarPlacements.indexOf("urlbar-container");
+        // Deliberately iterate to 1 past the end of the array to insert at the
+        // end if need be.
+        while (++insertionPoint < navbarPlacements.length) {
+          let widget = navbarPlacements[insertionPoint];
+          // If we find a non-searchbar, non-spacer node, break out of the loop:
+          if (widget != "search-container" && !this.matchingSpecials(widget, "spring")) {
+            break;
+          }
+        }
+        // We either found the right spot, or reached the end of the
+        // placements, so insert here:
+        navbarPlacements.splice(insertionPoint, 0, "downloads-button");
+      }
+    }
   },
 
   /**
    * _markObsoleteBuiltinButtonsSeen
    * when upgrading, ensure obsoleted buttons are in seen state.
    */
   _markObsoleteBuiltinButtonsSeen() {
     if (!gSavedState)
--- a/browser/components/customizableui/test/browser_1042100_default_placements_update.js
+++ b/browser/components/customizableui/test/browser_1042100_default_placements_update.js
@@ -91,18 +91,20 @@ function test() {
       }
       placements.splice(i, 1);
     }
 
     is(placements.length, 1, "Should have 1 newly placed widget in nav-bar");
     is(placements[0], testWidgetNew.id, "Should have our test widget to be placed in nav-bar");
   }
 
+  // Reset kVersion
+  CustomizableUIBSPass.kVersion--;
+
   // Now test that the builtin photon migrations work:
-  CustomizableUIBSPass.kVersion--;
 
   CustomizableUIBSPass.gSavedState = {
     currentVersion: 6,
     placements: {
       "nav-bar": ["urlbar-container", "bookmarks-menu-button"],
       "PanelUI-contents": ["panic-button", "edit-controls"],
     },
   };
@@ -111,20 +113,68 @@ function test() {
   let springs = navbarPlacements.filter(id => id.includes("spring"));
   is(springs.length, 2, "Should have 2 toolbarsprings in placements now");
   navbarPlacements = navbarPlacements.filter(id => !id.includes("spring"));
   is(navbarPlacements[0], "back-button", "Back button is in the right place.");
   is(navbarPlacements[1], "forward-button", "Fwd button is in the right place.");
   is(navbarPlacements[2], "stop-reload-button", "Stop/reload button is in the right place.");
   is(navbarPlacements[3], "home-button", "Home button is in the right place.");
   is(navbarPlacements[4], "urlbar-container", "URL bar is in the right place.");
-  is(navbarPlacements[5], "library-button", "Library button is in the right place.");
-  is(navbarPlacements[6], "sidebar-button", "Sidebar button is in the right place.");
-  is(navbarPlacements.length, 7, "Should have 7 items");
+  is(navbarPlacements[5], "downloads-button", "Downloads button is in the right place.");
+  is(navbarPlacements[6], "library-button", "Library button is in the right place.");
+  is(navbarPlacements[7], "sidebar-button", "Sidebar button is in the right place.");
+  is(navbarPlacements.length, 8, "Should have 8 items");
 
   let overflowPlacements = CustomizableUIBSPass.gSavedState.placements["widget-overflow-fixed-list"];
   Assert.deepEqual(overflowPlacements, ["panic-button"]);
 
+  // Finally test that the downloads migration works:
+  let oldNavbarPlacements = [
+    "urlbar-container", "customizableui-special-spring3", "search-container",
+  ];
+  CustomizableUIBSPass.gSavedState = {
+    currentVersion: 10,
+    placements: {
+      "nav-bar": Array.from(oldNavbarPlacements),
+      "widget-overflow-fixed-list": ["downloads-button"],
+    },
+  };
+  CustomizableUIInternal._updateForNewVersion();
+  navbarPlacements = CustomizableUIBSPass.gSavedState.placements["nav-bar"];
+  Assert.deepEqual(navbarPlacements, oldNavbarPlacements.concat(["downloads-button"]),
+                   "Downloads button inserted in navbar");
+  Assert.deepEqual(CustomizableUIBSPass.gSavedState.placements["widget-overflow-fixed-list"], [],
+                   "Overflow panel is empty");
+
+  CustomizableUIBSPass.gSavedState = {
+    currentVersion: 10,
+    placements: {
+      "nav-bar": ["downloads-button"].concat(oldNavbarPlacements),
+    },
+  };
+  CustomizableUIInternal._updateForNewVersion();
+  navbarPlacements = CustomizableUIBSPass.gSavedState.placements["nav-bar"];
+  Assert.deepEqual(navbarPlacements, oldNavbarPlacements.concat(["downloads-button"]),
+                   "Downloads button reinserted in navbar");
+
+  oldNavbarPlacements = [
+    "urlbar-container", "customizableui-special-spring3", "search-container", "other-widget",
+  ];
+  CustomizableUIBSPass.gSavedState = {
+    currentVersion: 10,
+    placements: {
+      "nav-bar": Array.from(oldNavbarPlacements),
+    },
+  };
+  CustomizableUIInternal._updateForNewVersion();
+  navbarPlacements = CustomizableUIBSPass.gSavedState.placements["nav-bar"];
+  let expectedNavbarPlacements = [
+    "urlbar-container", "customizableui-special-spring3", "search-container",
+    "downloads-button", "other-widget",
+  ];
+  Assert.deepEqual(navbarPlacements, expectedNavbarPlacements,
+                   "Downloads button inserted in navbar before other widgets");
+
   gFuturePlacements.delete(CustomizableUI.AREA_NAVBAR);
   gPalette.delete(testWidgetNew.id);
   gPalette.delete(testWidgetOld.id);
 }