Bug 995164 - registerArea should work correctly with customize mode, r=mconley
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 14 Apr 2014 00:42:24 +0100
changeset 196954 f4338beb6e1241dd5dd12557d579194ee2e21cdf
parent 196953 bba21dfbd38137a80d118dc1e3a7abd831765878
child 196955 87a8e8c24bf9ac58dfca02cbbba13cddda59728b
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs995164
milestone31.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 995164 - registerArea should work correctly with customize mode, r=mconley
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/src/DragPositionManager.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_993322_widget_notoolbar.js
browser/components/customizableui/test/browser_995164_registerArea_during_customize_mode.js
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -381,16 +381,23 @@ let CustomizableUIInternal = {
       if (aDestroyPlacements) {
         gPlacements.delete(aName);
       } else {
         // Otherwise we need to re-set them, as removeFromArea will have emptied
         // them out:
         gPlacements.set(aName, placements);
       }
       gFuturePlacements.delete(aName);
+      let existingAreaNodes = gBuildAreas.get(aName);
+      if (existingAreaNodes) {
+        for (let areaNode of existingAreaNodes) {
+          this.notifyListeners("onAreaNodeUnregistered", aName, areaNode.customizationTarget,
+                               CustomizableUI.REASON_AREA_UNREGISTERED);
+        }
+      }
       gBuildAreas.delete(aName);
     } finally {
       this.endBatchUpdate(true);
     }
   },
 
   registerToolbarNode: function(aToolbar, aExistingChildren) {
     let area = aToolbar.id;
@@ -453,16 +460,17 @@ let CustomizableUIInternal = {
       // 2) The number of children of the toolbar does not match the length of
       //    the placements array for that area.
       //
       // This notion of being "dirty" is stored in a cache which is persisted
       // in the saved state.
       if (gDirtyAreaCache.has(area)) {
         this.buildArea(area, placements, aToolbar);
       }
+      this.notifyListeners("onAreaNodeRegistered", area, aToolbar.customizationTarget);
       aToolbar.setAttribute("currentset", placements.join(","));
     } finally {
       this.endBatchUpdate();
     }
   },
 
   buildArea: function(aArea, aPlacements, aAreaNode) {
     let document = aAreaNode.ownerDocument;
@@ -694,16 +702,18 @@ let CustomizableUIInternal = {
 
     aPanelContents.toolbox = document.getElementById("navigator-toolbox");
     aPanelContents.customizationTarget = aPanelContents;
 
     this.addPanelCloseListeners(this._getPanelForNode(aPanelContents));
 
     let placements = gPlacements.get(CustomizableUI.AREA_PANEL);
     this.buildArea(CustomizableUI.AREA_PANEL, placements, aPanelContents);
+    this.notifyListeners("onAreaNodeRegistered", CustomizableUI.AREA_PANEL, aPanelContents);
+
     for (let child of aPanelContents.children) {
       if (child.localName != "toolbarbutton") {
         if (child.localName == "toolbaritem") {
           this.ensureButtonContextMenu(child, aPanelContents);
         }
         continue;
       }
       this.ensureButtonContextMenu(child, aPanelContents);
@@ -834,16 +844,18 @@ let CustomizableUIInternal = {
     gBuildWindows.delete(aWindow);
     gSingleWrapperCache.delete(aWindow);
     let document = aWindow.document;
 
     for (let [areaId, areaNodes] of gBuildAreas) {
       let areaProperties = gAreas.get(areaId);
       for (let node of areaNodes) {
         if (node.ownerDocument == document) {
+          this.notifyListeners("onAreaNodeUnregistered", areaId, node.customizationTarget,
+                               CustomizableUI.REASON_WINDOW_CLOSED);
           if (areaProperties.has("overflowable")) {
             node.overflowable.uninit();
             node.overflowable = null;
           }
           areaNodes.delete(node);
         }
       }
     }
@@ -2498,16 +2510,27 @@ this.CustomizableUI = {
    */
   get WIDE_PANEL_CLASS() "panel-wide-item",
   /**
    * The (constant) number of columns in the menu panel.
    */
   get PANEL_COLUMN_COUNT() 3,
 
   /**
+   * Constant indicating the reason the event was fired was a window closing
+   */
+  get REASON_WINDOW_CLOSED() "window-closed",
+  /**
+   * Constant indicating the reason the event was fired was an area being
+   * unregistered separately from window closing mechanics.
+   */
+  get REASON_AREA_UNREGISTERED() "area-unregistered",
+
+
+  /**
    * An iteratable property of windows managed by CustomizableUI.
    * Note that this can *only* be used as an iterator. ie:
    *     for (let window of CustomizableUI.windows) { ... }
    */
   windows: {
     "@@iterator": function*() {
       for (let [window,] of gBuildWindows)
         yield window;
@@ -2598,16 +2621,25 @@ this.CustomizableUI = {
    *     Fired when a widget's DOM node is *not* overflowing its container, a
    *     toolbar, anymore.
    *   - onWindowOpened(aWindow)
    *     Fired when a window has been opened that is managed by CustomizableUI,
    *     once all of the prerequisite setup has been done.
    *   - onWindowClosed(aWindow)
    *     Fired when a window that has been managed by CustomizableUI has been
    *     closed.
+   *   - onAreaNodeRegistered(aArea, aContainer)
+   *     Fired after an area node is first built when it is registered. This
+   *     is often when the window has opened, but in the case of add-ons,
+   *     could fire when the node has just been registered with CustomizableUI
+   *     after an add-on update or disable/enable sequence.
+   *   - onAreaNodeUnregistered(aArea, aContainer, aReason)
+   *     Fired when an area node is explicitly unregistered by an API caller,
+   *     or by a window closing. The aReason parameter indicates which of
+   *     these is the case.
    */
   addListener: function(aListener) {
     CustomizableUIInternal.addListener(aListener);
   },
   /**
    * Remove a listener added with addListener
    * @param aListener the listener object to remove
    */
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -231,21 +231,17 @@ CustomizeMode.prototype = {
         mainView.removeAttribute("context");
       }
 
       this._showPanelCustomizationPlaceholders();
 
       yield this._wrapToolbarItems();
       this.populatePalette();
 
-      this.visiblePalette.addEventListener("dragstart", this, true);
-      this.visiblePalette.addEventListener("dragover", this, true);
-      this.visiblePalette.addEventListener("dragexit", this, true);
-      this.visiblePalette.addEventListener("drop", this, true);
-      this.visiblePalette.addEventListener("dragend", this, true);
+      this._addDragHandlers(this.visiblePalette);
 
       window.gNavToolbox.addEventListener("toolbarvisibilitychange", this);
 
       document.getElementById("PanelUI-help").setAttribute("disabled", true);
       document.getElementById("PanelUI-quit").setAttribute("disabled", true);
 
       this._updateResetButton();
       this._updateUndoResetButton();
@@ -397,21 +393,17 @@ CustomizeMode.prototype = {
       }
       browser.parentNode.selectedPanel = browser;
       let customizer = document.getElementById("customization-container");
       customizer.hidden = true;
 
       window.gNavToolbox.removeEventListener("toolbarvisibilitychange", this);
 
       DragPositionManager.stop();
-      this.visiblePalette.removeEventListener("dragstart", this, true);
-      this.visiblePalette.removeEventListener("dragover", this, true);
-      this.visiblePalette.removeEventListener("dragexit", this, true);
-      this.visiblePalette.removeEventListener("drop", this, true);
-      this.visiblePalette.removeEventListener("dragend", this, true);
+      this._removeDragHandlers(this.visiblePalette);
 
       yield this._unwrapToolbarItems();
 
       if (this._changed) {
         // XXXmconley: At first, it seems strange to also persist the old way with
         //             currentset - but this might actually be useful for switching
         //             to old builds. We might want to keep this around for a little
         //             bit.
@@ -895,60 +887,68 @@ CustomizeMode.prototype = {
 
   _wrapToolbarItems: function() {
     let window = this.window;
     // Add drag-and-drop event handlers to all of the customizable areas.
     return Task.spawn(function() {
       this.areas = [];
       for (let area of CustomizableUI.areas) {
         let target = CustomizableUI.getCustomizeTargetForArea(area, window);
-        target.addEventListener("dragstart", this, true);
-        target.addEventListener("dragover", this, true);
-        target.addEventListener("dragexit", this, true);
-        target.addEventListener("drop", this, true);
-        target.addEventListener("dragend", this, true);
+        this._addDragHandlers(target);
         for (let child of target.children) {
           if (this.isCustomizableItem(child)) {
             yield this.deferredWrapToolbarItem(child, CustomizableUI.getPlaceForItem(child));
           }
         }
         this.areas.push(target);
       }
     }.bind(this)).then(null, ERROR);
   },
 
+  _addDragHandlers: function(aTarget) {
+    aTarget.addEventListener("dragstart", this, true);
+    aTarget.addEventListener("dragover", this, true);
+    aTarget.addEventListener("dragexit", this, true);
+    aTarget.addEventListener("drop", this, true);
+    aTarget.addEventListener("dragend", this, true);
+  },
+
   _wrapItemsInArea: function(target) {
     for (let child of target.children) {
       if (this.isCustomizableItem(child)) {
         this.wrapToolbarItem(child, CustomizableUI.getPlaceForItem(child));
       }
     }
   },
 
+  _removeDragHandlers: function(aTarget) {
+    aTarget.removeEventListener("dragstart", this, true);
+    aTarget.removeEventListener("dragover", this, true);
+    aTarget.removeEventListener("dragexit", this, true);
+    aTarget.removeEventListener("drop", this, true);
+    aTarget.removeEventListener("dragend", this, true);
+  },
+
   _unwrapItemsInArea: function(target) {
     for (let toolbarItem of target.children) {
       if (this.isWrappedToolbarItem(toolbarItem)) {
         this.unwrapToolbarItem(toolbarItem);
       }
     }
   },
 
   _unwrapToolbarItems: function() {
     return Task.spawn(function() {
       for (let target of this.areas) {
         for (let toolbarItem of target.children) {
           if (this.isWrappedToolbarItem(toolbarItem)) {
             yield this.deferredUnwrapToolbarItem(toolbarItem);
           }
         }
-        target.removeEventListener("dragstart", this, true);
-        target.removeEventListener("dragover", this, true);
-        target.removeEventListener("dragexit", this, true);
-        target.removeEventListener("drop", this, true);
-        target.removeEventListener("dragend", this, true);
+        this._removeDragHandlers(target);
       }
     }.bind(this)).then(null, ERROR);
   },
 
   _removeExtraToolbarsIfEmpty: function() {
     let toolbox = this.window.gNavToolbox;
     for (let child of toolbox.children) {
       if (child.hasAttribute("customindex")) {
@@ -1113,16 +1113,35 @@ CustomizeMode.prototype = {
         this.wrapToolbarItem(widgetNode, "palette");
       } else {
         let widget = CustomizableUI.getWidget(aWidgetId);
         this.visiblePalette.appendChild(this.makePaletteItem(widget, "palette"));
       }
     }
   },
 
+  onAreaNodeRegistered: function(aArea, aContainer) {
+    if (aContainer.ownerDocument == this.document) {
+      this._wrapItemsInArea(aContainer);
+      this._addDragHandlers(aContainer);
+      DragPositionManager.add(this.window, aArea, aContainer);
+      this.areas.push(aContainer);
+    }
+  },
+
+  onAreaNodeUnregistered: function(aArea, aContainer, aReason) {
+    if (aContainer.ownerDocument == this.document && aReason == CustomizableUI.REASON_AREA_UNREGISTERED) {
+      this._unwrapItemsInArea(aContainer);
+      this._removeDragHandlers(aContainer);
+      DragPositionManager.remove(this.window, aArea, aContainer);
+      let index = this.areas.indexOf(aContainer);
+      this.areas.splice(index, 1);
+    }
+  },
+
   _onUIChange: function() {
     this._changed = true;
     if (!this.resetting) {
       this._updateResetButton();
       this._updateUndoResetButton();
       this._updateEmptyPaletteNotice();
     }
     CustomizableUI.dispatchToolboxEvent("customizationchange");
--- a/browser/components/customizableui/src/DragPositionManager.jsm
+++ b/browser/components/customizableui/src/DragPositionManager.jsm
@@ -388,16 +388,32 @@ let DragPositionManager = {
       if (positionManager) {
         positionManager.update(areaNode);
       } else {
         gManagers.set(areaNode, new AreaPositionManager(areaNode));
       }
     }
   },
 
+  add: function(aWindow, aArea, aContainer) {
+    if (CustomizableUI.getAreaType(aArea) != "toolbar") {
+      return;
+    }
+
+    gManagers.set(aContainer, new AreaPositionManager(aContainer));
+  },
+
+  remove: function(aWindow, aArea, aContainer) {
+    if (CustomizableUI.getAreaType(aArea) != "toolbar") {
+      return;
+    }
+
+    gManagers.delete(aContainer);
+  },
+
   stop: function() {
     gManagers.clear();
   },
 
   getManagerForArea: function(aArea) {
     return gManagers.get(aArea);
   }
 };
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -99,10 +99,11 @@ skip-if = os == "linux"
 
 [browser_985815_propagate_setToolbarVisibility.js]
 [browser_981305_separator_insertion.js]
 [browser_987177_destroyWidget_xul.js]
 [browser_987177_xul_wrapper_updating.js]
 [browser_987492_window_api.js]
 [browser_992747_toggle_noncustomizable_toolbar.js]
 [browser_993322_widget_notoolbar.js]
+[browser_995164_registerArea_during_customize_mode.js]
 [browser_bootstrapped_custom_toolbar.js]
 [browser_panel_toggle.js]
--- a/browser/components/customizableui/test/browser_993322_widget_notoolbar.js
+++ b/browser/components/customizableui/test/browser_993322_widget_notoolbar.js
@@ -26,9 +26,11 @@ add_task(function*() {
     is(buttonNode.parentNode.getAttribute("place"), "palette", "Node should be in palette");
     is(buttonNode, gNavToolbox.palette.querySelector("#" + BUTTONID), "Node should really be in palette.");
   }
   is(currentWidget.forWindow(window).node, buttonNode, "Should have the same node for customize mode");
   yield endCustomizing();
 
   CustomizableUI.destroyWidget(BUTTONID);
   CustomizableUI.unregisterArea(TOOLBARID, true);
+  toolbar.remove();
+  gAddedToolbars.clear();
 });
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_995164_registerArea_during_customize_mode.js
@@ -0,0 +1,113 @@
+/* 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/. */
+
+"use strict";
+
+const TOOLBARID = "test-toolbar-added-during-customize-mode";
+
+add_task(function*() {
+  yield startCustomizing();
+  let toolbar = createToolbarWithPlacements(TOOLBARID, []);
+  CustomizableUI.addWidgetToArea("sync-button", TOOLBARID);
+  let syncButton = document.getElementById("sync-button");
+  ok(syncButton, "Sync button should exist.");
+  is(syncButton.parentNode.localName, "toolbarpaletteitem", "Sync button's parent node should be a wrapper.");
+
+  simulateItemDrag(syncButton, gNavToolbox.palette);
+  ok(!CustomizableUI.getPlacementOfWidget("sync-button"), "Button moved to the palette");
+  ok(gNavToolbox.palette.querySelector("#sync-button"), "Sync button really is in palette.");
+
+  simulateItemDrag(syncButton, toolbar);
+  ok(CustomizableUI.getPlacementOfWidget("sync-button"), "Button moved out of palette");
+  is(CustomizableUI.getPlacementOfWidget("sync-button").area, TOOLBARID, "Button's back on toolbar");
+  ok(toolbar.querySelector("#sync-button"), "Sync button really is on toolbar.");
+
+  yield endCustomizing();
+  isnot(syncButton.parentNode.localName, "toolbarpaletteitem", "Sync button's parent node should not be a wrapper outside customize mode.");
+  yield startCustomizing();
+
+  is(syncButton.parentNode.localName, "toolbarpaletteitem", "Sync button's parent node should be a wrapper back in customize mode.");
+
+  simulateItemDrag(syncButton, gNavToolbox.palette);
+  ok(!CustomizableUI.getPlacementOfWidget("sync-button"), "Button moved to the palette");
+  ok(gNavToolbox.palette.querySelector("#sync-button"), "Sync button really is in palette.");
+
+  ok(!CustomizableUI.inDefaultState, "Not in default state while toolbar is not collapsed yet.");
+  setToolbarVisibility(toolbar, false);
+  ok(CustomizableUI.inDefaultState, "In default state while toolbar is collapsed.");
+
+  setToolbarVisibility(toolbar, true);
+
+  info("Check that removing the area registration from within customize mode works");
+  CustomizableUI.unregisterArea(TOOLBARID);
+  ok(CustomizableUI.inDefaultState, "Now that the toolbar is no longer registered, should be in default state.");
+  ok(!(new Set(gCustomizeMode.areas)).has(toolbar), "Toolbar shouldn't be known to customize mode.");
+
+  CustomizableUI.registerArea(TOOLBARID, {legacy: true, defaultPlacements: []});
+  CustomizableUI.registerToolbarNode(toolbar, []);
+  ok(!CustomizableUI.inDefaultState, "Now that the toolbar is registered again, should no longer be in default state.");
+  ok((new Set(gCustomizeMode.areas)).has(toolbar), "Toolbar should be known to customize mode again.");
+
+  simulateItemDrag(syncButton, toolbar);
+  ok(CustomizableUI.getPlacementOfWidget("sync-button"), "Button moved out of palette");
+  is(CustomizableUI.getPlacementOfWidget("sync-button").area, TOOLBARID, "Button's back on toolbar");
+  ok(toolbar.querySelector("#sync-button"), "Sync button really is on toolbar.");
+
+  let otherWin = yield openAndLoadWindow({}, true);
+  let otherTB = otherWin.document.createElementNS(kNSXUL, "toolbar");
+  otherTB.id = TOOLBARID;
+  otherTB.setAttribute("customizable", "true");
+  otherWin.gNavToolbox.appendChild(otherTB);
+  ok(otherTB.querySelector("#sync-button"), "Sync button is on other toolbar, too.");
+
+  simulateItemDrag(syncButton, gNavToolbox.palette);
+  ok(!CustomizableUI.getPlacementOfWidget("sync-button"), "Button moved to the palette");
+  ok(gNavToolbox.palette.querySelector("#sync-button"), "Sync button really is in palette.");
+  ok(!otherTB.querySelector("#sync-button"), "Sync button is in palette in other window, too.");
+
+  simulateItemDrag(syncButton, toolbar);
+  ok(CustomizableUI.getPlacementOfWidget("sync-button"), "Button moved out of palette");
+  is(CustomizableUI.getPlacementOfWidget("sync-button").area, TOOLBARID, "Button's back on toolbar");
+  ok(toolbar.querySelector("#sync-button"), "Sync button really is on toolbar.");
+  ok(otherTB.querySelector("#sync-button"), "Sync button is on other toolbar, too.");
+
+  let wasInformedCorrectlyOfAreaDisappearing = false;
+  let listener = {
+    onAreaNodeUnregistered: function(aArea, aNode, aReason) {
+      if (aArea == TOOLBARID) {
+        is(aNode, otherTB, "Should be informed about other toolbar");
+        is(aReason, CustomizableUI.REASON_WINDOW_CLOSED, "Reason should be correct.");
+        wasInformedCorrectlyOfAreaDisappearing = (aReason === CustomizableUI.REASON_WINDOW_CLOSED);
+      }
+    }
+  };
+  CustomizableUI.addListener(listener);
+  yield promiseWindowClosed(otherWin);
+
+  ok(wasInformedCorrectlyOfAreaDisappearing, "Should be told about window closing.");
+  CustomizableUI.removeListener(listener);
+  // Closing the other window should not be counted against this window's customize mode:
+  is(syncButton.parentNode.localName, "toolbarpaletteitem", "Sync button's parent node should still be a wrapper.");
+  isnot(gCustomizeMode.areas.indexOf(toolbar), -1, "Toolbar should still be a customizable area for this customize mode instance.");
+
+  yield gCustomizeMode.reset();
+
+  yield endCustomizing();
+
+  wasInformedCorrectlyOfAreaDisappearing = false;
+  listener = {
+    onAreaNodeUnregistered: function(aArea, aNode, aReason) {
+      if (aArea == TOOLBARID) {
+        is(aNode, toolbar, "Should be informed about this window's toolbar");
+        is(aReason, CustomizableUI.REASON_AREA_UNREGISTERED, "Reason for final removal should be correct.");
+        wasInformedCorrectlyOfAreaDisappearing = (aReason === CustomizableUI.REASON_AREA_UNREGISTERED);
+      }
+    },
+  }
+  CustomizableUI.addListener(listener);
+  removeCustomToolbars();
+  ok(wasInformedCorrectlyOfAreaDisappearing, "Should be told about area being unregistered.");
+  CustomizableUI.removeListener(listener);
+  ok(CustomizableUI.inDefaultState, "Should be fine after exiting customize mode.");
+});