Bug 995164 - registerArea should work correctly with customize mode, r=mconley

browser/components/customizableui/src/CustomizableUI.jsm

@@ -386,6 +386,13 @@ let CustomizableUIInternal = {
         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);
@@ -458,6 +465,7 @@ let CustomizableUIInternal = {
       if (gDirtyAreaCache.has(area)) {
         this.buildArea(area, placements, aToolbar);
       }
+      this.notifyListeners("onAreaNodeRegistered", area, aToolbar.customizationTarget);
       aToolbar.setAttribute("currentset", placements.join(","));
     } finally {
       this.endBatchUpdate();
@@ -699,6 +707,8 @@ let CustomizableUIInternal = {
 
     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") {
@@ -839,6 +849,8 @@ let CustomizableUIInternal = {
       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;
@@ -2502,6 +2514,17 @@ this.CustomizableUI = {
    */
   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:
@@ -2603,6 +2626,15 @@ this.CustomizableUI = {
    *   - 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);

browser/components/customizableui/src/CustomizeMode.jsm

@@ -236,11 +236,7 @@ CustomizeMode.prototype = {
       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);
 
@@ -402,11 +398,7 @@ CustomizeMode.prototype = {
       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();
 
@@ -900,11 +892,7 @@ CustomizeMode.prototype = {
       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));
@@ -915,6 +903,14 @@ CustomizeMode.prototype = {
     }.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)) {
@@ -923,6 +919,14 @@ CustomizeMode.prototype = {
     }
   },
 
+  _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)) {
@@ -939,11 +943,7 @@ CustomizeMode.prototype = {
             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);
   },
@@ -1118,6 +1118,25 @@ CustomizeMode.prototype = {
     }
   },
 
+  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) {

browser/components/customizableui/src/DragPositionManager.jsm

@@ -393,6 +393,22 @@ let DragPositionManager = {
     }
   },
 
+  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();
   },

browser/components/customizableui/test/browser.ini

@@ -104,5 +104,6 @@ skip-if = os == "linux"
 [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]

browser/components/customizableui/test/browser_993322_widget_notoolbar.js

@@ -31,4 +31,6 @@ add_task(function*() {
 
   CustomizableUI.destroyWidget(BUTTONID);
   CustomizableUI.unregisterArea(TOOLBARID, true);
+  toolbar.remove();
+  gAddedToolbars.clear();
 });

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.");
+});