Bug 940013 - fix Australis' CustomizableUI to have better interaction between registerToolbarNode and registerArea, r=Unfocused
☠☠ backed out by e970d7fcd33c ☠ ☠
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 29 Nov 2013 00:20:34 +0100
changeset 173150 78e12d1f3aa23128910a5a32e69808f3a669a126
parent 173149 d1e4fed327fe09619cabb204e19274d3e42eb615
child 173151 bac5ac014c63754ff362d96905091337524aefeb
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersUnfocused
bugs940013
milestone28.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 940013 - fix Australis' CustomizableUI to have better interaction between registerToolbarNode and registerArea, r=Unfocused
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_877178_unregisterArea.js
browser/components/customizableui/test/browser_927717_customize_drag_empty_toolbar.js
browser/components/customizableui/test/browser_940013_registerToolbarNode_calls_registerArea.js
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -92,16 +92,23 @@ let gSeenWidgets = new Set();
 /**
  * gDirtyAreaCache is a set of area IDs for areas where items have been added,
  * moved or removed at least once. This set is persisted, and is used to
  * optimize building of toolbars in the default case where no toolbars should
  * be "dirty".
  */
 let gDirtyAreaCache = new Set();
 
+/**
+ * gPendingBuildAreas is a map from area IDs to map from build nodes to their
+ * existing children at the time of node registration, that are waiting
+ * for the area to be registered
+ */
+let gPendingBuildAreas = new Map();
+
 let gSavedState = null;
 let gRestoring = false;
 let gDirty = false;
 let gInBatchStack = 0;
 let gResetting = false;
 
 /**
  * gBuildAreas maps area IDs to actual area nodes within browser windows.
@@ -258,38 +265,66 @@ let CustomizableUIInternal = {
     gGroupWrapperCache.set(aWidgetId, wrapper);
     return wrapper;
   },
 
   registerArea: function(aName, aProperties) {
     if (typeof aName != "string" || !/^[a-z0-9-_]{1,}$/i.test(aName)) {
       throw new Error("Invalid area name");
     }
-    if (gAreas.has(aName)) {
-      throw new Error("Area already registered");
-    }
-
-    let props = new Map();
+
+    let areaIsKnown = gAreas.has(aName);
+    let props = areaIsKnown ? gAreas.get(aName) : new Map();
     for (let key in aProperties) {
       //XXXgijs for special items, we need to make sure they have an appropriate ID
       // so we aren't perpetually in a non-default state:
       if (key == "defaultPlacements" && Array.isArray(aProperties[key])) {
         props.set(key, aProperties[key].map(x => this.isSpecialWidget(x) ? this.ensureSpecialWidgetId(x) : x ));
       } else {
         props.set(key, aProperties[key]);
       }
     }
-    gAreas.set(aName, props);
-
-    if (props.get("legacy")) {
-      // Guarantee this area exists in gFuturePlacements, to avoid checking it in
-      // various places elsewhere.
-      gFuturePlacements.set(aName, new Set());
-    } else {
-      this.restoreStateForArea(aName);
+    // Default to a toolbar:
+    if (!props.has("type")) {
+      props.set("type", CustomizableUI.TYPE_TOOLBAR);
+    }
+    // Sanity check type:
+    let allTypes = [CustomizableUI.TYPE_TOOLBAR, CustomizableUI.TYPE_MENU_PANEL];
+    if (allTypes.indexOf(props.get("type")) == -1) {
+      throw new Error("Invalid area type " + props.get("type"));
+    }
+
+    // And to no placements:
+    if (!props.has("defaultPlacements")) {
+      props.set("defaultPlacements", []);
+    }
+    // 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);
+
+      if (props.get("legacy")) {
+        // Guarantee this area exists in gFuturePlacements, to avoid checking it in
+        // various places elsewhere.
+        gFuturePlacements.set(aName, new Set());
+      } else {
+        this.restoreStateForArea(aName);
+      }
+
+      // If we have pending build area nodes, register all of them
+      if (gPendingBuildAreas.has(aName)) {
+        let pendingNodes = gPendingBuildAreas.get(aName);
+        for (let [pendingNode, existingChildren] of pendingNodes) {
+          this.registerToolbarNode(pendingNode, existingChildren);
+        }
+        gPendingBuildAreas.delete(aName);
+      }
     }
   },
 
   unregisterArea: function(aName, aDestroyPlacements) {
     if (typeof aName != "string" || !/^[a-z0-9-_]{1,}$/i.test(aName)) {
       throw new Error("Invalid area name");
     }
     if (!gAreas.has(aName) && !gPlacements.has(aName)) {
@@ -326,18 +361,33 @@ let CustomizableUIInternal = {
   registerToolbarNode: function(aToolbar, aExistingChildren) {
     let area = aToolbar.id;
     if (gBuildAreas.has(area) && gBuildAreas.get(area).has(aToolbar)) {
       return;
     }
     let document = aToolbar.ownerDocument;
     let areaProperties = gAreas.get(area);
 
+    // If this area is not registered, try to do it automatically:
     if (!areaProperties) {
-      throw new Error("Unknown customization area: " + area);
+      // If there's no default set attribute at all, we assume that we should
+      // wait for registerArea to be called:
+      if (!aToolbar.hasAttribute("defaultset")) {
+        if (!gPendingBuildAreas.has(area)) {
+          gPendingBuildAreas.set(area, new Map());
+        }
+        let pendingNodes = gPendingBuildAreas.get(area);
+        pendingNodes.set(aToolbar, aExistingChildren);
+        return;
+      }
+      let props = {type: CustomizableUI.TYPE_TOOLBAR, legacy: true};
+      let defaultsetAttribute = aToolbar.getAttribute("defaultset");
+      props.defaultPlacements = defaultsetAttribute.split(',').filter(s => s);
+      this.registerArea(area, props);
+      areaProperties = gAreas.get(area);
     }
 
     this.beginBatchUpdate();
     try {
       let placements = gPlacements.get(area);
       if (!placements && areaProperties.has("legacy")) {
         let legacyState = aToolbar.getAttribute("currentset");
         if (legacyState) {
@@ -725,16 +775,28 @@ let CustomizableUIInternal = {
         }
       }
     }
 
     for (let [,widget] of gPalette) {
       widget.instances.delete(document);
       this.notifyListeners("onWidgetInstanceRemoved", widget.id, document);
     }
+
+    for (let [area, areaMap] of gPendingBuildAreas) {
+      let toDelete = [];
+      for (let [areaNode, ] of areaMap) {
+        if (areaNode.ownerDocument == document) {
+          toDelete.push(areaNode);
+        }
+      }
+      for (let areaNode of toDelete) {
+        areaMap.delete(toDelete);
+      }
+    }
   },
 
   setLocationAttributes: function(aNode, aArea) {
     let props = gAreas.get(aArea);
     if (!props) {
       throw new Error("Expected area " + aArea + " to have a properties Map " +
                       "associated with it.");
     }
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -33,12 +33,13 @@ skip-if = true
 [browser_927717_customize_drag_empty_toolbar.js]
 
 [browser_934113_menubar_removable.js]
 # Because this test is about the menubar, it can't be run on mac
 skip-if = os == "mac"
 
 [browser_938980_navbar_collapsed.js]
 [browser_938995_indefaultstate_nonremovable.js]
+[browser_940013_registerToolbarNode_calls_registerArea.js]
 [browser_940946_removable_from_navbar_customizemode.js]
 [browser_941083_invalidate_wrapper_cache_createWidget.js]
 [browser_942581_unregisterArea_keeps_placements.js]
 [browser_panel_toggle.js]
--- a/browser/components/customizableui/test/browser_877178_unregisterArea.js
+++ b/browser/components/customizableui/test/browser_877178_unregisterArea.js
@@ -7,19 +7,16 @@ let gTests = [
     desc: "Sanity checks",
     run: function() {
       SimpleTest.doesThrow(function() CustomizableUI.registerArea("@foo"),
                            "Registering areas with an invalid ID should throw.");
 
       SimpleTest.doesThrow(function() CustomizableUI.registerArea([]),
                            "Registering areas with an invalid ID should throw.");
 
-      SimpleTest.doesThrow(function() CustomizableUI.registerArea(CustomizableUI.AREA_NAVBAR),
-                           "Registering an area with an ID that's already registered should throw.");
-
       SimpleTest.doesThrow(function() CustomizableUI.unregisterArea("@foo"),
                            "Unregistering areas with an invalid ID should throw.");
 
       SimpleTest.doesThrow(function() CustomizableUI.unregisterArea([]),
                            "Unregistering areas with an invalid ID should throw.");
 
       SimpleTest.doesThrow(function() CustomizableUI.unregisterArea("unknown"),
                            "Unregistering an area that's not registered should throw.");
--- a/browser/components/customizableui/test/browser_927717_customize_drag_empty_toolbar.js
+++ b/browser/components/customizableui/test/browser_927717_customize_drag_empty_toolbar.js
@@ -2,17 +2,17 @@
  * 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 kTestToolbarId = "test-empty-drag";
 let gTests = [
   {
     desc: "Attempting to drag an item to an empty container should work.",
     setup: function() {
-      createToolbarWithPlacements(kTestToolbarId, "");
+      createToolbarWithPlacements(kTestToolbarId, []);
     },
     run: function() {
       yield startCustomizing();
       let downloadButton = document.getElementById("downloads-button");
       let customToolbar = document.getElementById(kTestToolbarId);
       simulateItemDrag(downloadButton, customToolbar);
       assertAreaPlacements(kTestToolbarId, ["downloads-button"]);
       ok(downloadButton.parentNode && downloadButton.parentNode.parentNode == customToolbar,
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_940013_registerToolbarNode_calls_registerArea.js
@@ -0,0 +1,77 @@
+/* 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 kToolbarId = "test-registerToolbarNode-toolbar";
+const kButtonId = "test-registerToolbarNode-button";
+let gTests = [
+  {
+    desc: "Registering a toolbar with defaultset attribute should work",
+    run: function() {
+      let btn = createDummyXULButton(kButtonId);
+      let toolbar = document.createElement("toolbar");
+      toolbar.id = kToolbarId;
+      toolbar.setAttribute("customizable", true);
+      toolbar.setAttribute("defaultset", kButtonId);
+      gNavToolbox.appendChild(toolbar);
+      ok(CustomizableUI.areas.indexOf(kToolbarId) != -1,
+         "Toolbar should have been registered automatically.");
+      is(CustomizableUI.getAreaType(kToolbarId), CustomizableUI.TYPE_TOOLBAR,
+         "Area should be registered as toolbar");
+      assertAreaPlacements(kToolbarId, [kButtonId]);
+      ok(CustomizableUI.inDefaultState, "Everything should be in its default state.");
+      CustomizableUI.unregisterArea(kToolbarId, true);
+      toolbar.remove();
+      ok(CustomizableUI.inDefaultState, "Everything should be in its default state.");
+      btn.remove();
+    }
+  },
+  {
+    desc: "Registering a toolbar without a defaultset attribute should " +
+          "wait for the registerArea call",
+    run: function() {
+      let btn = createDummyXULButton(kButtonId);
+      let toolbar = document.createElement("toolbar");
+      toolbar.id = kToolbarId;
+      toolbar.setAttribute("customizable", true);
+      gNavToolbox.appendChild(toolbar);
+      ok(CustomizableUI.areas.indexOf(kToolbarId) == -1,
+         "Toolbar should not yet have been registered automatically.");
+      CustomizableUI.registerArea(kToolbarId, {defaultPlacements: [kButtonId]});
+      ok(CustomizableUI.areas.indexOf(kToolbarId) != -1,
+         "Toolbar should have been registered now.");
+      is(CustomizableUI.getAreaType(kToolbarId), CustomizableUI.TYPE_TOOLBAR,
+         "Area should be registered as toolbar");
+      assertAreaPlacements(kToolbarId, [kButtonId]);
+      ok(CustomizableUI.inDefaultState, "Everything should be in its default state.");
+      CustomizableUI.unregisterArea(kToolbarId, true);
+      toolbar.remove();
+      ok(CustomizableUI.inDefaultState, "Everything should be in its default state.");
+      btn.remove();
+    }
+  }
+];
+
+function asyncCleanup() {
+  yield resetCustomization();
+}
+
+function cleanup() {
+  let toolbar = document.getElementById(kToolbarId);
+  if (toolbar) {
+    toolbar.remove();
+  }
+  let btn = document.getElementById(kButtonId) ||
+            gNavToolbox.querySelector("#" + kButtonId);
+  if (btn) {
+    btn.remove();
+  }
+}
+
+function test() {
+  waitForExplicitFinish();
+  registerCleanupFunction(cleanup);
+  runTests(gTests, asyncCleanup);
+}
+
+