Bug 1161838 - fix positioning of newly added widgets, r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 06 May 2015 17:01:30 +0100
changeset 274095 79ce48075b625afcff42001138ce0a081dbbad77
parent 274094 182674a080675c8873632e473fc19a8bb249aa78
child 274096 9d1cf0bd3626445bedd0981bb95a8d561a3d438e
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1161838
milestone40.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 1161838 - fix positioning of newly added widgets, r=jaws
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
@@ -340,16 +340,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;
@@ -420,16 +471,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();
+}
+