Bug 1396423 - make it possible to drop items in what visually looks like the palette but isn't, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 29 Sep 2017 15:40:48 +0100
changeset 672645 dad3be1d4bd9e4e43d990f1d3e6b9d47718b1b37
parent 672593 cd9c8c48e4b3ded47a776f757008f3dcf570c59c
child 733865 1c6e51da893ebd84dce1895f045573a945e8c9c6
push id82315
push usergijskruitbosch@gmail.com
push dateFri, 29 Sep 2017 14:45:47 +0000
reviewersmikedeboer
bugs1396423
milestone58.0a1
Bug 1396423 - make it possible to drop items in what visually looks like the palette but isn't, r?mikedeboer MozReview-Commit-ID: AqBwn9ovTjT
browser/components/customizableui/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_drag_outside_palette.js
--- a/browser/components/customizableui/CustomizeMode.jsm
+++ b/browser/components/customizableui/CustomizeMode.jsm
@@ -26,16 +26,18 @@ const kDownloadAutohidePanelId = "downlo
 const kDownloadAutoHidePref = "browser.download.autohideButton";
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource:///modules/CustomizableUI.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/AddonManager.jsm");
 Cu.import("resource://gre/modules/AppConstants.jsm");
 
+Cu.importGlobalProperties(["CSS"]);
+
 XPCOMUtils.defineLazyModuleGetter(this, "DragPositionManager",
                                   "resource:///modules/DragPositionManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserUITelemetry",
                                   "resource:///modules/BrowserUITelemetry.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BrowserUtils",
                                   "resource://gre/modules/BrowserUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager",
                                   "resource://gre/modules/LightweightThemeManager.jsm");
@@ -323,17 +325,17 @@ CustomizeMode.prototype = {
       Services.obs.addObserver(this, "lightweight-theme-window-updated");
 
       // Let everybody in this window know that we're about to customize.
       CustomizableUI.dispatchToolboxEvent("customizationstarting", {}, window);
 
       await this._wrapToolbarItems();
       this.populatePalette();
 
-      this._addDragHandlers(this.visiblePalette);
+      this._setupPaletteDragging();
 
       window.gNavToolbox.addEventListener("toolbarvisibilitychange", this);
 
       this._updateResetButton();
       this._updateUndoResetButton();
 
       this._skipSourceNodeCheck = Services.prefs.getPrefType(kSkipSourceNodePref) == Ci.nsIPrefBranch.PREF_BOOL &&
                                   Services.prefs.getBoolPref(kSkipSourceNodePref);
@@ -433,18 +435,17 @@ CustomizeMode.prototype = {
       }
       let browser = document.getElementById("browser");
       browser.parentNode.selectedPanel = browser;
       let customizer = document.getElementById("customization-container");
       customizer.hidden = true;
 
       window.gNavToolbox.removeEventListener("toolbarvisibilitychange", this);
 
-      DragPositionManager.stop();
-      this._removeDragHandlers(this.visiblePalette);
+      this._teardownPaletteDragging();
 
       await this._unwrapToolbarItems();
 
       if (this._changed) {
         // XXXmconley: At first, it seems strange to also persist the old way with
         //             currentset - but this might actually be useful for switching
         //             to old builds. We might want to keep this around for a little
         //             bit.
@@ -1488,16 +1489,52 @@ CustomizeMode.prototype = {
         }
         break;
       case "unload":
         this.uninit();
         break;
     }
   },
 
+  /**
+   * We handle dragover/drop on the outer palette separately
+   * to avoid overlap with other drag/drop handlers.
+   */
+  _setupPaletteDragging() {
+    this._addDragHandlers(this.visiblePalette);
+
+    this.paletteDragHandler = (aEvent) => {
+      let originalTarget = aEvent.originalTarget;
+      if (this._isUnwantedDragDrop(aEvent) ||
+          this.visiblePalette.contains(originalTarget) ||
+          this.document.getElementById("customization-panelHolder").contains(originalTarget)) {
+        return;
+      }
+      // We have a dragover/drop on the palette.
+      if (aEvent.type == "dragover") {
+        this._onDragOver(aEvent, this.visiblePalette);
+      } else {
+        this._onDragDrop(aEvent, this.visiblePalette);
+      }
+    };
+    let contentContainer = this.document.getElementById("customization-content-container");
+    contentContainer.addEventListener("dragover", this.paletteDragHandler, true);
+    contentContainer.addEventListener("drop", this.paletteDragHandler, true);
+  },
+
+  _teardownPaletteDragging() {
+    DragPositionManager.stop();
+    this._removeDragHandlers(this.visiblePalette);
+
+    let contentContainer = this.document.getElementById("customization-content-container");
+    contentContainer.removeEventListener("dragover", this.paletteDragHandler, true);
+    contentContainer.removeEventListener("drop", this.paletteDragHandler, true);
+    delete this.paletteDragHandler;
+  },
+
   observe(aSubject, aTopic, aData) {
     switch (aTopic) {
       case "nsPref:changed":
         this._updateResetButton();
         this._updateUndoResetButton();
         if (AppConstants.CAN_DRAW_IN_TITLEBAR) {
           this._updateTitlebarCheckbox();
           this._updateDragSpaceCheckbox();
@@ -1631,17 +1668,17 @@ CustomizeMode.prototype = {
         }
       }
       this._initializeDragAfterMove = null;
       this.window.clearTimeout(this._dragInitializeTimeout);
     };
     this._dragInitializeTimeout = this.window.setTimeout(this._initializeDragAfterMove, 0);
   },
 
-  _onDragOver(aEvent) {
+  _onDragOver(aEvent, aOverrideTarget) {
     if (this._isUnwantedDragDrop(aEvent)) {
       return;
     }
     if (this._initializeDragAfterMove) {
       this._initializeDragAfterMove();
     }
 
     __dumpDragData(aEvent);
@@ -1650,17 +1687,17 @@ CustomizeMode.prototype = {
     let documentId = document.documentElement.id;
     if (!aEvent.dataTransfer.mozTypesAt(0)) {
       return;
     }
 
     let draggedItemId =
       aEvent.dataTransfer.mozGetDataAt(kDragDataTypePrefix + documentId, 0);
     let draggedWrapper = document.getElementById("wrapper-" + draggedItemId);
-    let targetArea = this._getCustomizableParent(aEvent.currentTarget);
+    let targetArea = this._getCustomizableParent(aOverrideTarget || aEvent.currentTarget);
     let originArea = this._getCustomizableParent(draggedWrapper);
 
     // Do nothing if the target or origin are not customizable.
     if (!targetArea || !originArea) {
       return;
     }
 
     // Do nothing if the widget is not allowed to be removed.
@@ -1738,26 +1775,26 @@ CustomizeMode.prototype = {
       }
       this._dragOverItem = dragOverItem;
     }
 
     aEvent.preventDefault();
     aEvent.stopPropagation();
   },
 
-  _onDragDrop(aEvent) {
+  _onDragDrop(aEvent, aOverrideTarget) {
     if (this._isUnwantedDragDrop(aEvent)) {
       return;
     }
 
     __dumpDragData(aEvent);
     this._initializeDragAfterMove = null;
     this.window.clearTimeout(this._dragInitializeTimeout);
 
-    let targetArea = this._getCustomizableParent(aEvent.currentTarget);
+    let targetArea = this._getCustomizableParent(aOverrideTarget || aEvent.currentTarget);
     let document = aEvent.target.ownerDocument;
     let documentId = document.documentElement.id;
     let draggedItemId =
       aEvent.dataTransfer.mozGetDataAt(kDragDataTypePrefix + documentId, 0);
     let draggedWrapper = document.getElementById("wrapper-" + draggedItemId);
     let originArea = this._getCustomizableParent(draggedWrapper);
     if (this._dragSizeMap) {
       this._dragSizeMap = new WeakMap();
@@ -2160,77 +2197,59 @@ CustomizeMode.prototype = {
     } else {
       aDraggedItem.parentNode.hidden = true;
     }
     return size;
   },
 
   _getCustomizableParent(aElement) {
     if (aElement) {
-      // Deal with drag/drop on the padding of the panel in photon.
+      // Deal with drag/drop on the padding of the panel.
       let containingPanelHolder = aElement.closest("#customization-panelHolder");
       if (containingPanelHolder) {
         return containingPanelHolder.firstChild;
       }
     }
 
     let areas = CustomizableUI.areas;
     areas.push(kPaletteId);
-    while (aElement) {
-      if (areas.indexOf(aElement.id) != -1) {
-        return aElement;
-      }
-      aElement = aElement.parentNode;
-    }
-
-    return null;
+    return aElement.closest(areas.map(a => "#" + CSS.escape(a)).join(","));
   },
 
   _getDragOverNode(aEvent, aAreaElement, aAreaType, aDraggedItemId) {
     let expectedParent = aAreaElement.customizationTarget || aAreaElement;
+    if (!expectedParent.contains(aEvent.target)) {
+      return expectedParent;
+    }
     // Our tests are stupid. Cope:
     if (!aEvent.clientX && !aEvent.clientY) {
       return aEvent.target;
     }
     // Offset the drag event's position with the offset to the center of
     // the thing we're dragging
     let dragX = aEvent.clientX - this._dragOffset.x;
     let dragY = aEvent.clientY - this._dragOffset.y;
 
     // Ensure this is within the container
     let boundsContainer = expectedParent;
-    // NB: because the panel UI itself is inside a scrolling container, we need
-    // to use the parent bounds (otherwise, if the panel UI is scrolled down,
-    // the numbers we get are in window coordinates which leads to various kinds
-    // of weirdness)
-    if (boundsContainer == this.panelUIContents) {
-      boundsContainer = boundsContainer.parentNode;
-    }
-    let bounds = boundsContainer.getBoundingClientRect();
+    let bounds = this._dwu.getBoundsWithoutFlushing(boundsContainer);
     dragX = Math.min(bounds.right, Math.max(dragX, bounds.left));
     dragY = Math.min(bounds.bottom, Math.max(dragY, bounds.top));
 
     let targetNode;
     if (aAreaType == "toolbar" || aAreaType == "menu-panel") {
       targetNode = aAreaElement.ownerDocument.elementFromPoint(dragX, dragY);
       while (targetNode && targetNode.parentNode != expectedParent) {
         targetNode = targetNode.parentNode;
       }
     } else {
       let positionManager = DragPositionManager.getManagerForArea(aAreaElement);
       // Make it relative to the container:
       dragX -= bounds.left;
-      // NB: but if we're in the panel UI, we need to use the actual panel
-      // contents instead of the scrolling container to determine our origin
-      // offset against:
-      if (expectedParent == this.panelUIContents) {
-        dragY -= this.panelUIContents.getBoundingClientRect().top;
-      } else {
-        dragY -= bounds.top;
-      }
+      dragY -= bounds.top;
       // Find the closest node:
       targetNode = positionManager.find(aAreaElement, dragX, dragY, aDraggedItemId);
     }
     return targetNode || aEvent.target;
   },
 
   _onMouseDown(aEvent) {
     log.debug("_onMouseDown");
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -137,16 +137,17 @@ skip-if = os == "mac"
 [browser_1161838_inserted_new_default_buttons.js]
 [browser_allow_dragging_removable_false.js]
 [browser_bootstrapped_custom_toolbar.js]
 [browser_currentset_post_reset.js]
 [browser_customizemode_contextmenu_menubuttonstate.js]
 [browser_customizemode_dragspace.js]
 skip-if = os == "linux" # linux doesn't get drag space (no tabsintitlebar)
 [browser_customizemode_uidensity.js]
+[browser_drag_outside_palette.js]
 [browser_exit_background_customize_mode.js]
 [browser_insert_before_moved_node.js]
 [browser_overflow_use_subviews.js]
 [browser_panel_keyboard_navigation.js]
 [browser_panel_toggle.js]
 [browser_panelUINotifications.js]
 [browser_panelUINotifications_fullscreen.js]
 tags = fullscreen
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_drag_outside_palette.js
@@ -0,0 +1,42 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Check that moving items from the toolbar or panel to the palette by
+ * dropping on the panel container (not inside the visible panel) works.
+ */
+add_task(async function() {
+  await startCustomizing();
+  let panelContainer = document.getElementById("customization-panel-container");
+  // Try dragging an item from the navbar:
+  let homeButton = document.getElementById("home-button");
+  let oldNavbarPlacements = CustomizableUI.getWidgetIdsInArea("nav-bar");
+  simulateItemDrag(homeButton, panelContainer);
+  assertAreaPlacements(CustomizableUI.AREA_NAVBAR,
+    oldNavbarPlacements.filter(w => w != "home-button"));
+  ok(homeButton.closest("#customization-palette"), "Button should be in the palette");
+
+  // Put it in the panel and try again from there:
+  let panelHolder = document.getElementById("customization-panelHolder");
+  simulateItemDrag(homeButton, panelHolder);
+  assertAreaPlacements(CustomizableUI.AREA_FIXED_OVERFLOW_PANEL,
+    ["home-button"]);
+
+  simulateItemDrag(homeButton, panelContainer);
+  assertAreaPlacements(CustomizableUI.AREA_FIXED_OVERFLOW_PANEL, []);
+
+  ok(homeButton.closest("#customization-palette"), "Button should be in the palette");
+
+  // Check we can't move non-removable items like this:
+  let urlbar = document.getElementById("urlbar-container");
+  simulateItemDrag(urlbar, panelContainer);
+  assertAreaPlacements(CustomizableUI.AREA_NAVBAR,
+    oldNavbarPlacements.filter(w => w != "home-button"));
+});
+
+registerCleanupFunction(async function() {
+  await gCustomizeMode.reset();
+  await endCustomizing();
+});