Bug 918049 - deal with skipintoolbarset items better when dnd'ing, r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 02 Oct 2013 14:52:56 +0200
changeset 150755 52281082b7daa6affd086e9d27002adbeae08757
parent 150754 530dd4a6a9496467ede60886f5511b828775bc6e
child 150862 f4d76e89a820d545d1c2849e57449ebccfbe4909
push id493
push usergijskruitbosch@gmail.com
push dateWed, 09 Oct 2013 05:07:17 +0000
treeherderux@52281082b7da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs918049
milestone27.0a1
Bug 918049 - deal with skipintoolbarset items better when dnd'ing, r=jaws
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_918049_skipintoolbarset_dnd.js
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -906,47 +906,80 @@ CustomizeMode.prototype = {
       }
       return;
     }
 
     if (!CustomizableUI.canWidgetMoveToArea(aDraggedItemId, aTargetArea.id)) {
       return;
     }
 
+    // Skipintoolbarset items won't really be moved:
+    if (draggedItem.getAttribute("skipintoolbarset") == "true") {
+      // These items should never leave their area:
+      if (aTargetArea != aOriginArea) {
+        return;
+      }
+      this.unwrapToolbarItem(draggedItem.parentNode);
+      if (aTargetNode == aTargetArea.customizationTarget) {
+        aTargetArea.customizationTarget.appendChild(draggedItem);
+      } else {
+        this.unwrapToolbarItem(aTargetNode.parentNode);
+        aTargetArea.customizationTarget.insertBefore(draggedItem, aTargetNode);
+        this.wrapToolbarItem(aTargetNode);
+      }
+      this.wrapToolbarItem(draggedItem);
+      return;
+    }
+
     // Is the target the customization area itself? If so, we just add the
     // widget to the end of the area.
     if (aTargetNode == aTargetArea.customizationTarget) {
       CustomizableUI.addWidgetToArea(aDraggedItemId, aTargetArea.id);
       return;
     }
 
     // We need to determine the place that the widget is being dropped in
     // the target.
     let placement;
-    if (!aTargetNode.classList.contains(kPlaceholderClass)) {
-      let targetNodeId = (aTargetNode.nodeName == "toolbarpaletteitem") ?
-                            aTargetNode.firstChild && aTargetNode.firstChild.id :
-                            aTargetNode.id;
+    let itemForPlacement = aTargetNode;
+    // Skip the skipintoolbarset items when determining the place of the item:
+    while (itemForPlacement && itemForPlacement.getAttribute("skipintoolbarset") == "true" &&
+           itemForPlacement.parentNode &&
+           itemForPlacement.parentNode.nodeName == "toolbarpaletteitem") {
+      itemForPlacement = itemForPlacement.parentNode.nextSibling;
+      if (itemForPlacement && itemForPlacement.nodeName == "toolbarpaletteitem") {
+        itemForPlacement = itemForPlacement.firstChild;
+      }
+    }
+    if (itemForPlacement && !itemForPlacement.classList.contains(kPlaceholderClass)) {
+      let targetNodeId = (itemForPlacement.nodeName == "toolbarpaletteitem") ?
+                            itemForPlacement.firstChild && itemForPlacement.firstChild.id :
+                            itemForPlacement.id;
       placement = CustomizableUI.getPlacementOfWidget(targetNodeId);
     }
     if (!placement) {
-      LOG("Could not get a position for " + aTargetNode + "#" + aTargetNode.id + "." + aTargetNode.className);
+      LOG("Could not get a position for " + aTargetNode.nodeName + "#" + aTargetNode.id + "." + aTargetNode.className);
     }
     let position = placement ? placement.position : null;
 
 
     // Is the target area the same as the origin? Since we've already handled
     // the possibility that the target is the customization palette, we know
     // that the widget is moving within a customizable area.
     if (aTargetArea == aOriginArea) {
       CustomizableUI.moveWidgetWithinArea(aDraggedItemId, position);
-      return;
+    } else {
+      CustomizableUI.addWidgetToArea(aDraggedItemId, aTargetArea.id, position);
     }
-
-    CustomizableUI.addWidgetToArea(aDraggedItemId, aTargetArea.id, position);
+    // If we dropped onto a skipintoolbarset item, manually correct the drop location:
+    if (aTargetNode != itemForPlacement) {
+      let draggedWrapper = draggedItem.parentNode;
+      let container = draggedWrapper.parentNode;
+      container.insertBefore(draggedWrapper, aTargetNode.parentNode);
+    }
   },
 
   _onDragExit: function(aEvent) {
     if (this._isUnwantedDragDrop(aEvent)) {
       return;
     }
 
     __dumpDragData(aEvent);
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -13,10 +13,11 @@ support-files =
 [browser_886323_buildArea_removable_nodes.js]
 [browser_887438_currentset_shim.js]
 [browser_890262_destroyWidget_after_add_to_panel.js]
 [browser_892955_isWidgetRemovable_for_removed_widgets.js]
 [browser_892956_destroyWidget_defaultPlacements.js]
 [browser_888817_currentset_updating.js]
 [browser_913972_currentset_overflow.js]
 [browser_914863_disabled_help_quit_buttons.js]
+[browser_918049_skipintoolbarset_dnd.js]
 [browser_923857_customize_mode_event_wrapping_during_reset.js]
 
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_918049_skipintoolbarset_dnd.js
@@ -0,0 +1,46 @@
+/* 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/. */
+
+let navbar;
+let skippedItem;
+let gTests = [
+  {
+    desc: "Attempting to drag a skipintoolbarset item should work.",
+    setup: function() {
+      navbar = document.getElementById("nav-bar");
+      skippedItem = document.createElement("toolbarbutton");
+      skippedItem.id = "test-skipintoolbarset-item";
+      skippedItem.setAttribute("label", "Test");
+      skippedItem.setAttribute("skipintoolbarset", "true");
+      navbar.customizationTarget.appendChild(skippedItem);
+    },
+    run: function() {
+      let downloadsButton = document.getElementById("downloads-button");
+      yield startCustomizing();
+      ok(CustomizableUI.inDefaultState, "Should still be in default state");
+      simulateItemDrag(skippedItem, downloadsButton);
+      ok(CustomizableUI.inDefaultState, "Should still be in default state");
+      let skippedItemWrapper = skippedItem.parentNode;
+      is(skippedItemWrapper.nextSibling && skippedItemWrapper.nextSibling.id,
+         downloadsButton.parentNode.id, "Should be next to downloads button");
+      simulateItemDrag(downloadsButton, skippedItem);
+      let downloadWrapper = downloadsButton.parentNode;
+      is(downloadWrapper.nextSibling && downloadWrapper.nextSibling.id,
+         skippedItem.parentNode.id, "Should be next to skipintoolbarset item");
+      ok(CustomizableUI.inDefaultState, "Should still be in default state");
+    }
+  },
+];
+function asyncCleanup() {
+  yield endCustomizing();
+  skippedItem.remove();
+  Services.prefs.clearUserPref("browser.uiCustomization.skipSourceNodeCheck");
+  yield resetCustomization();
+}
+
+function test() {
+  Services.prefs.setBoolPref("browser.uiCustomization.skipSourceNodeCheck", true);
+  waitForExplicitFinish();
+  runTests(gTests, asyncCleanup);
+}