Bug 982027 - moving item around in Australis' customize mode shouldn't remove custom context menu, r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 11 Mar 2014 12:29:25 +0000
changeset 191357 d6869ad04b24a84eba21acce157ba3967622df65
parent 191356 8d6720456275408b376b92879863236319ee9337
child 191358 03e7402f6fd34d3958e664744480cf18b42e1348
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs982027
milestone30.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 982027 - moving item around in Australis' customize mode shouldn't remove custom context menu, r=jaws
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/test/browser_880164_customization_context_menus.js
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -661,17 +661,17 @@ CustomizeMode.prototype = {
   },
 
   //XXXunf Maybe this should use -moz-element instead of wrapping the node?
   //       Would ensure no weird interactions/event handling from original node,
   //       and makes it possible to put this in a lazy-loaded iframe/real tab
   //       while still getting rid of the need for overlays.
   makePaletteItem: function(aWidget, aPlace) {
     let widgetNode = aWidget.forWindow(this.window).node;
-    let wrapper = this.createWrapper(widgetNode, aPlace);
+    let wrapper = this.createOrUpdateWrapper(widgetNode, aPlace);
     wrapper.appendChild(widgetNode);
     return wrapper;
   },
 
   depopulatePalette: function() {
     return Task.spawn(function() {
       this.visiblePalette.hidden = true;
       let paletteChild = this.visiblePalette.firstChild;
@@ -723,36 +723,43 @@ CustomizeMode.prototype = {
 
     return deferred.promise;
   },
 
   wrapToolbarItem: function(aNode, aPlace) {
     if (!this.isCustomizableItem(aNode)) {
       return aNode;
     }
-    let wrapper = this.createWrapper(aNode, aPlace);
+    let wrapper = this.createOrUpdateWrapper(aNode, aPlace);
+
     // It's possible that this toolbar node is "mid-flight" and doesn't have
     // a parent, in which case we skip replacing it. This can happen if a
     // toolbar item has been dragged into the palette. In that case, we tell
     // CustomizableUI to remove the widget from its area before putting the
     // widget in the palette - so the node will have no parent.
     if (aNode.parentNode) {
       aNode = aNode.parentNode.replaceChild(wrapper, aNode);
     }
     wrapper.appendChild(aNode);
     return wrapper;
   },
 
-  createWrapper: function(aNode, aPlace) {
-    let wrapper = this.document.createElement("toolbarpaletteitem");
+  createOrUpdateWrapper: function(aNode, aPlace, aIsUpdate) {
+    let wrapper;
+    if (aIsUpdate && aNode.parentNode && aNode.parentNode.localName == "toolbarpaletteitem") {
+      wrapper = aNode.parentNode;
+      aPlace = wrapper.getAttribute("place");
+    } else {
+      wrapper = this.document.createElement("toolbarpaletteitem");
+      // "place" is used by toolkit to add the toolbarpaletteitem-palette
+      // binding to a toolbarpaletteitem, which gives it a label node for when
+      // it's sitting in the palette.
+      wrapper.setAttribute("place", aPlace);
+    }
 
-    // "place" is used by toolkit to add the toolbarpaletteitem-palette
-    // binding to a toolbarpaletteitem, which gives it a label node for when
-    // it's sitting in the palette.
-    wrapper.setAttribute("place", aPlace);
 
     // Ensure the wrapped item doesn't look like it's in any special state, and
     // can't be interactved with when in the customization palette.
     if (aNode.hasAttribute("command")) {
       wrapper.setAttribute("itemcommand", aNode.getAttribute("command"));
       aNode.removeAttribute("command");
     }
 
@@ -797,18 +804,21 @@ CustomizeMode.prototype = {
         currentContextMenu != contextMenuForPlace) {
       aNode.setAttribute("wrapped-context", currentContextMenu);
       aNode.setAttribute("wrapped-contextAttrName", contextMenuAttrName)
       aNode.removeAttribute(contextMenuAttrName);
     } else if (currentContextMenu == contextMenuForPlace) {
       aNode.removeAttribute(contextMenuAttrName);
     }
 
-    wrapper.addEventListener("mousedown", this);
-    wrapper.addEventListener("mouseup", this);
+    // Only add listeners for newly created wrappers:
+    if (!aIsUpdate) {
+      wrapper.addEventListener("mousedown", this);
+      wrapper.addEventListener("mouseup", this);
+    }
 
     return wrapper;
   },
 
   deferredUnwrapToolbarItem: function(aWrapper) {
     let deferred = Promise.defer();
     dispatchFunction(function() {
       let item = null;
@@ -1740,27 +1750,25 @@ CustomizeMode.prototype = {
     let rect = aDraggedItem.parentNode.getBoundingClientRect();
     size = {width: rect.width, height: rect.height};
     // Cache the found value of size for this target.
     itemMap.set(targetArea, size);
 
     if (targetArea != currentArea) {
       this.unwrapToolbarItem(aDraggedItem.parentNode);
       // Put the item back into its previous position.
-      if (currentSibling)
-        currentParent.insertBefore(aDraggedItem, currentSibling);
-      else
-        currentParent.appendChild(aDraggedItem);
+      currentParent.insertBefore(aDraggedItem, currentSibling);
       // restore the areaType
       if (areaType) {
         if (currentType === false)
           aDraggedItem.removeAttribute(kAreaType);
         else
           aDraggedItem.setAttribute(kAreaType, currentType);
       }
+      this.createOrUpdateWrapper(aDraggedItem, null, true);
       CustomizableUI.onWidgetDrag(aDraggedItem.id);
     } else {
       aDraggedItem.parentNode.hidden = true;
     }
     return size;
   },
 
   _getCustomizableParent: function(aElement) {
--- a/browser/components/customizableui/test/browser_880164_customization_context_menus.js
+++ b/browser/components/customizableui/test/browser_880164_customization_context_menus.js
@@ -335,8 +335,37 @@ add_task(function() {
   contextMenu.hidePopup();
   yield hiddenContextPromise;
 
   let hiddenPromise = promisePanelHidden(window);
   PanelUI.hide();
   yield hiddenPromise;
 });
 
+
+// Bug 982027 - moving icon around removes custom context menu.
+add_task(function() {
+  let widgetId = "custom-context-menu-toolbarbutton";
+  let expectedContext = "myfancycontext";
+  let widget = createDummyXULButton(widgetId, "Test ctxt menu");
+  widget.setAttribute("context", expectedContext);
+  CustomizableUI.addWidgetToArea(widgetId, CustomizableUI.AREA_NAVBAR);
+  is(widget.getAttribute("context"), expectedContext, "Should have context menu when added to the toolbar.");
+
+  yield startCustomizing();
+  is(widget.getAttribute("context"), "", "Should not have own context menu in the toolbar now that we're customizing.");
+  is(widget.getAttribute("wrapped-context"), expectedContext, "Should keep own context menu wrapped when in toolbar.");
+
+  let panel = PanelUI.contents;
+  simulateItemDrag(widget, panel);
+  is(widget.getAttribute("context"), "", "Should not have own context menu when in the panel.");
+  is(widget.getAttribute("wrapped-context"), expectedContext, "Should keep own context menu wrapped now that we're in the panel.");
+
+  simulateItemDrag(widget, document.getElementById("nav-bar").customizationTarget);
+  is(widget.getAttribute("context"), "", "Should not have own context menu when back in toolbar because we're still customizing.");
+  is(widget.getAttribute("wrapped-context"), expectedContext, "Should keep own context menu wrapped now that we're back in the toolbar.");
+
+  yield endCustomizing();
+  is(widget.getAttribute("context"), expectedContext, "Should have context menu again now that we're out of customize mode.");
+  CustomizableUI.removeWidgetFromArea(widgetId);
+  widget.remove();
+  ok(CustomizableUI.inDefaultState, "Should be in default state after removing button.");
+});