Bug 1161838 - fix positioning of newly added widgets, r=jaws a=dolske
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 06 May 2015 17:01:30 +0100
changeset 260440 2eeb61f35995
parent 260439 257c096c7673
child 260441 ccec3836123c
push id782
push userjdolske@mozilla.com
push date2015-05-09 00:18 +0000
treeherdermozilla-release@067c9c7a5e75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, dolske
bugs1161838
milestone38.0
Bug 1161838 - fix positioning of newly added widgets, r=jaws a=dolske
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_1161838_inserted_new_default_buttons.js
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -344,16 +344,67 @@ let CustomizableUIInternal = {
       CustomizableUI.removeWidgetFromArea("loop-call-button");
     }
 
     if (currentVersion < 4) {
       CustomizableUI.removeWidgetFromArea("loop-button-throttled");
     }
   },
 
+  _placeNewDefaultWidgetsInArea: function(aArea) {
+    let futurePlacedWidgets = gFuturePlacements.get(aArea);
+    let savedPlacements = gSavedState && gSavedState.placements && gSavedState.placements[aArea];
+    let defaultPlacements = gAreas.get(aArea).get("defaultPlacements");
+    if (!savedPlacements || !savedPlacements.length || !futurePlacedWidgets || !defaultPlacements ||
+        !defaultPlacements.length) {
+      return;
+    }
+    let defaultWidgetIndex = -1;
+
+    for (let widgetId of futurePlacedWidgets) {
+      let widget = gPalette.get(widgetId);
+      if (!widget || widget.source !== CustomizableUI.SOURCE_BUILTIN ||
+          !widget.defaultArea || !widget._introducedInVersion ||
+          savedPlacements.indexOf(widget.id) !== -1) {
+        continue;
+      }
+      defaultWidgetIndex = defaultPlacements.indexOf(widget.id);
+      if (defaultWidgetIndex === -1) {
+        continue;
+      }
+      // Now we know that this widget should be here by default, was newly introduced,
+      // and we have a saved state to insert into, and a default state to work off of.
+      // Try introducing after widgets that come before it in the default placements:
+      for (let i = defaultWidgetIndex; i >= 0; i--) {
+        // Special case: if the defaults list this widget as coming first, insert at the beginning:
+        if (i === 0 && i === defaultWidgetIndex) {
+          savedPlacements.splice(0, 0, widget.id);
+          // Before you ask, yes, deleting things inside a let x of y loop where y is a Set is
+          // safe, and we won't skip any items.
+          futurePlacedWidgets.delete(widget.id);
+          break;
+        }
+        // Otherwise, if we're somewhere other than the beginning, check if the previous
+        // widget is in the saved placements.
+        if (i) {
+          let previousWidget = defaultPlacements[i - 1];
+          let previousWidgetIndex = savedPlacements.indexOf(previousWidget);
+          if (previousWidgetIndex != -1) {
+            savedPlacements.splice(previousWidgetIndex + 1, 0, widget.id);
+            futurePlacedWidgets.delete(widget.id);
+            break;
+          }
+        }
+      }
+      // The loop above either inserts the item or doesn't - either way, we can get away
+      // with doing nothing else now; if the item remains in gFuturePlacements, we'll
+      // add it at the end in restoreStateForArea.
+    }
+  },
+
   wrapWidget: function(aWidgetId) {
     if (gGroupWrapperCache.has(aWidgetId)) {
       return gGroupWrapperCache.get(aWidgetId);
     }
 
     let provider = this.getWidgetProvider(aWidgetId);
     if (!provider) {
       return null;
@@ -424,16 +475,19 @@ let CustomizableUIInternal = {
     // Sanity check default placements array:
     if (!Array.isArray(props.get("defaultPlacements"))) {
       throw new Error("Should provide an array of default placements");
     }
 
     if (!areaIsKnown) {
       gAreas.set(aName, props);
 
+      // Reconcile new default widgets. Have to do this before we start restoring things.
+      this._placeNewDefaultWidgetsInArea(aName);
+
       if (props.get("legacy") && !gPlacements.has(aName)) {
         // Guarantee this area exists in gFuturePlacements, to avoid checking it in
         // various places elsewhere.
         if (!gFuturePlacements.has(aName)) {
           gFuturePlacements.set(aName, new Set());
         }
       } else {
         this.restoreStateForArea(aName);
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -145,10 +145,11 @@ skip-if = os == "linux" && e10s # Bug 11
 [browser_1008559_anchor_undo_restore.js]
 [browser_1042100_default_placements_update.js]
 [browser_1058573_showToolbarsDropdown.js]
 [browser_1087303_button_fullscreen.js]
 skip-if = os == "mac"
 [browser_1087303_button_preferences.js]
 [browser_1089591_still_customizable_after_reset.js]
 [browser_1096763_seen_widgets_post_reset.js]
+[browser_1161838_inserted_new_default_buttons.js]
 [browser_bootstrapped_custom_toolbar.js]
 [browser_panel_toggle.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_1161838_inserted_new_default_buttons.js
@@ -0,0 +1,82 @@
+"use strict";
+
+// NB: This uses some ugly hacks to get into the CUI module from elsewhere...
+// don't try this at home, kids.
+function test() {
+  // Customize something to make sure stuff changed:
+  CustomizableUI.addWidgetToArea("feed-button", CustomizableUI.AREA_NAVBAR);
+
+  let CustomizableUIBSPass = Cu.import("resource:///modules/CustomizableUI.jsm", {});
+
+  is(CustomizableUIBSPass.gFuturePlacements.size, 0,
+     "All future placements should be dealt with by now.");
+
+  let {CustomizableUIInternal, gFuturePlacements, gPalette} = CustomizableUIBSPass;
+
+  // Force us to have a saved state:
+  CustomizableUIInternal.saveState();
+  CustomizableUIInternal.loadSavedState();
+
+  CustomizableUIInternal._introduceNewBuiltinWidgets();
+  is(gFuturePlacements.size, 0,
+     "No change to future placements initially.");
+
+  let currentVersion = CustomizableUIBSPass.kVersion;
+
+
+  // Add our widget to the defaults:
+  let testWidgetNew = {
+    id: "test-messing-with-default-placements-new-pref",
+    label: "Test messing with default placements - pref-based",
+    defaultArea: CustomizableUI.AREA_NAVBAR,
+    introducedInVersion: "pref",
+  };
+
+  let normalizedWidget = CustomizableUIInternal.normalizeWidget(testWidgetNew,
+                                                                CustomizableUI.SOURCE_BUILTIN);
+  ok(normalizedWidget, "Widget should be normalizable");
+  if (!normalizedWidget) {
+    return;
+  }
+  CustomizableUIBSPass.gPalette.set(testWidgetNew.id, normalizedWidget);
+
+  // Now adjust default placements for area:
+  let navbarArea = CustomizableUIBSPass.gAreas.get(CustomizableUI.AREA_NAVBAR);
+  let navbarPlacements = navbarArea.get("defaultPlacements");
+  navbarPlacements.splice(navbarPlacements.indexOf("bookmarks-menu-button") + 1, 0, testWidgetNew.id);
+
+  let savedPlacements = CustomizableUIBSPass.gSavedState.placements[CustomizableUI.AREA_NAVBAR];
+  // Then call the re-init routine so we re-add the builtin widgets
+  CustomizableUIInternal._introduceNewBuiltinWidgets();
+  is(gFuturePlacements.size, 1,
+     "Should have 1 more future placement");
+  let futureNavbarPlacements = gFuturePlacements.get(CustomizableUI.AREA_NAVBAR);
+  ok(futureNavbarPlacements, "Should have placements for nav-bar");
+  if (futureNavbarPlacements) {
+    ok(futureNavbarPlacements.has(testWidgetNew.id), "widget should be in future placements");
+  }
+  CustomizableUIInternal._placeNewDefaultWidgetsInArea(CustomizableUI.AREA_NAVBAR);
+
+  let indexInSavedPlacements = savedPlacements.indexOf(testWidgetNew.id);
+  info("Saved placements: " + savedPlacements.join(', '));
+  isnot(indexInSavedPlacements, -1, "Widget should have been inserted");
+  is(indexInSavedPlacements, savedPlacements.indexOf("bookmarks-menu-button") + 1,
+     "Widget should be in the right place.");
+
+  if (futureNavbarPlacements) {
+    ok(!futureNavbarPlacements.has(testWidgetNew.id), "widget should be out of future placements");
+  }
+
+  if (indexInSavedPlacements != -1) {
+    savedPlacements.splice(indexInSavedPlacements, 1);
+  }
+
+  gFuturePlacements.delete(CustomizableUI.AREA_NAVBAR);
+  let indexInDefaultPlacements = navbarPlacements.indexOf(testWidgetNew.id);
+  if (indexInDefaultPlacements != -1) {
+    navbarPlacements.splice(indexInDefaultPlacements, 1);
+  }
+  gPalette.delete(testWidgetNew.id);
+  CustomizableUI.reset();
+}
+