author | Gijs Kruitbosch <gijskruitbosch@gmail.com> |
Tue, 11 Mar 2014 12:29:25 +0000 | |
changeset 173141 | d6869ad04b24a84eba21acce157ba3967622df65 |
parent 173140 | 8d6720456275408b376b92879863236319ee9337 |
child 173142 | 03e7402f6fd34d3958e664744480cf18b42e1348 |
push id | 26391 |
push user | cbook@mozilla.com |
push date | Wed, 12 Mar 2014 11:20:34 +0000 |
treeherder | mozilla-central@a56837cfc67c [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | jaws |
bugs | 982027 |
milestone | 30.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
|
--- 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."); +});