Bug 876944 - customizemode should deal with creating/destroying widgets, r=Unfocused
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 23 Oct 2013 13:03:55 +0200
changeset 155919 2669b682cd34b7b3c31858b55ae208db6825280c
parent 155918 75aeed86188a11c702ca0fb404d7986b096999aa
child 155920 93f754a03d3775ef012f184dbd654c0e3ad12197
push id25666
push userjwein@mozilla.com
push dateMon, 18 Nov 2013 15:56:58 +0000
treeherdermozilla-central@f2adb62d07eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersUnfocused
bugs876944
milestone27.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 876944 - customizemode should deal with creating/destroying widgets, r=Unfocused
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_876944_customize_mode_create_destroy.js
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -1553,16 +1553,17 @@ let CustomizableUIInternal = {
             this.addWidgetToArea(widget.id, widget.defaultArea);
           }
         }
 
         this.endBatchUpdate(true);
       }
     }
 
+    this.notifyListeners("onWidgetAfterCreation", widget.id, widget.currentArea);
     return widget.id;
   },
 
   createBuiltinWidget: function(aData) {
     // This should only ever be called on startup, before any windows are
     // opened - so we know there's no build areas to handle. Also, builtin
     // widgets are expected to be (mostly) static, so shouldn't affect the
     // current placement settings.
@@ -1972,17 +1973,17 @@ this.CustomizableUI = {
     if (!gAreas.has(aArea)) {
       throw new Error("Unknown customization area: " + aArea);
     }
     if (!gPlacements.has(aArea)) {
       throw new Error("Area not yet restored");
     }
 
     // We need to clone this, as we don't want to let consumers muck with placements
-    return [].concat(gPlacements.get(aArea));
+    return [...gPlacements.get(aArea)];
   },
   getWidgetsInArea: function(aArea) {
     return this.getWidgetIdsInArea(aArea).map(
       CustomizableUIInternal.wrapWidget,
       CustomizableUIInternal
     );
   },
   get areas() {
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -671,16 +671,41 @@ CustomizeMode.prototype = {
         this.visiblePalette.appendChild(paletteItem);
       }
     }
     if (aContainer.id == CustomizableUI.AREA_PANEL) {
       this._showPanelCustomizationPlaceholders();
     }
   },
 
+  onWidgetDestroyed: function(aWidgetId) {
+    let wrapper = this.document.getElementById("wrapper-" + aWidgetId);
+    if (wrapper) {
+      let wasInPanel = wrapper.parentNode == this.panelUIContents;
+      wrapper.remove();
+      if (wasInPanel) {
+        this._showPanelCustomizationPlaceholders();
+      }
+    }
+  },
+
+  onWidgetAfterCreation: function(aWidgetId, aArea) {
+    // If the node was added to an area, we would have gotten an onWidgetAdded notification,
+    // plus associated DOM change notifications, so only do stuff for the palette:
+    if (!aArea) {
+      let widgetNode = this.document.getElementById(aWidgetId);
+      if (widgetNode) {
+        this.wrapToolbarItem(widgetNode, "palette");
+      } else {
+        let widget = CustomizableUI.getWidget(aWidgetId);
+        this.visiblePalette.appendChild(this.makePaletteItem(widget, "palette"));
+      }
+    }
+  },
+
   _onUIChange: function() {
     this._changed = true;
     this._updateResetButton();
     this.dispatchToolboxEvent("customizationchange");
   },
 
   _updateResetButton: function() {
     let btn = this.document.getElementById("customization-reset-button");
@@ -949,25 +974,26 @@ CustomizeMode.prototype = {
     }
 
     // Skipintoolbarset items won't really be moved:
     if (draggedItem.getAttribute("skipintoolbarset") == "true") {
       // These items should never leave their area:
       if (aTargetArea != aOriginArea) {
         return;
       }
+      let place = draggedItem.parentNode.getAttribute("place");
       this.unwrapToolbarItem(draggedItem.parentNode);
       if (aTargetNode == aTargetArea.customizationTarget) {
         aTargetArea.customizationTarget.appendChild(draggedItem);
       } else {
         this.unwrapToolbarItem(aTargetNode.parentNode);
         aTargetArea.customizationTarget.insertBefore(draggedItem, aTargetNode);
-        this.wrapToolbarItem(aTargetNode);
+        this.wrapToolbarItem(aTargetNode, place);
       }
-      this.wrapToolbarItem(draggedItem);
+      this.wrapToolbarItem(draggedItem, place);
       return;
     }
 
     // Is the target the customization area itself? If so, we just add the
     // widget to the end of the area.
     if (aTargetNode == aTargetArea.customizationTarget) {
       CustomizableUI.addWidgetToArea(aDraggedItemId, aTargetArea.id);
       return;
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -1,14 +1,15 @@
 [DEFAULT]
 support-files =
   head.js
 
 [browser_873501_handle_specials.js]
 [browser_876926_customize_mode_wrapping.js]
+[browser_876944_customize_mode_create_destroy.js]
 [browser_877006_missing_view.js]
 [browser_877178_unregisterArea.js]
 [browser_877447_skip_missing_ids.js]
 [browser_878452_drag_to_panel.js]
 [browser_880382_drag_wide_widgets_in_panel.js]
 [browser_885530_showInPrivateBrowsing.js]
 [browser_886323_buildArea_removable_nodes.js]
 [browser_887438_currentset_shim.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_876944_customize_mode_create_destroy.js
@@ -0,0 +1,65 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const kTestWidget1 = "test-customize-mode-create-destroy1";
+const kTestWidget2 = "test-customize-mode-create-destroy2";
+
+let gTests = [
+  {
+    desc: "Creating and destroying a widget should correctly wrap/unwrap stuff",
+    setup: startCustomizing,
+    run: function() {
+      CustomizableUI.createWidget({id: kTestWidget1, label: 'Pretty label', tooltiptext: 'Pretty tooltip'});
+      let elem = document.getElementById(kTestWidget1);
+      let wrapper = document.getElementById("wrapper-" + kTestWidget1);
+      ok(elem, "There should be an item");
+      ok(wrapper, "There should be a wrapper");
+      is(wrapper.firstChild.id, kTestWidget1, "Wrapper should have test widget");
+      is(wrapper.parentNode.id, "customization-palette", "Wrapper should be in palette");
+      CustomizableUI.destroyWidget(kTestWidget1);
+      wrapper = document.getElementById("wrapper-" + kTestWidget1);
+      ok(!wrapper, "There should be a wrapper");
+      let item = document.getElementById(kTestWidget1);
+      ok(!item, "There should no longer be an item");
+    },
+  },
+  {
+    desc: "Creating and destroying a widget should correctly deal with panel placeholders",
+    run: function() {
+      let panel = document.getElementById(CustomizableUI.AREA_PANEL);
+      is(panel.querySelectorAll(".panel-customization-placeholder").length, 3, "The number of placeholders should be correct.");
+      CustomizableUI.createWidget({id: kTestWidget2, label: 'Pretty label', tooltiptext: 'Pretty tooltip', defaultArea: CustomizableUI.AREA_PANEL});
+      let elem = document.getElementById(kTestWidget2);
+      let wrapper = document.getElementById("wrapper-" + kTestWidget2);
+      ok(elem, "There should be an item");
+      ok(wrapper, "There should be a wrapper");
+      is(wrapper.firstChild.id, kTestWidget2, "Wrapper should have test widget");
+      is(wrapper.parentNode, panel, "Wrapper should be in panel");
+      is(panel.querySelectorAll(".panel-customization-placeholder").length, 2, "The number of placeholders should be correct.");
+      CustomizableUI.destroyWidget(kTestWidget2);
+      wrapper = document.getElementById("wrapper-" + kTestWidget2);
+      ok(!wrapper, "There should be a wrapper");
+      let item = document.getElementById(kTestWidget2);
+      ok(!item, "There should no longer be an item");
+    },
+    teardown: endCustomizing
+  },
+];
+
+function asyncCleanup() {
+  yield endCustomizing();
+  try {
+    CustomizableUI.destroyWidget(kTestWidget1);
+  } catch (ex) {}
+  try {
+    CustomizableUI.destroyWidget(kTestWidget2);
+  } catch (ex) {}
+  yield resetCustomization();
+}
+
+
+function test() {
+  waitForExplicitFinish();
+  runTests(gTests, asyncCleanup);
+}