Bug 1387313 - Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. r?gijs draft
authorJared Wein <jwein@mozilla.com>
Mon, 21 Aug 2017 14:20:40 -0400
changeset 651305 df0499bfda198623e88c52cabc7807cb1c3a5854
parent 650685 ab30809b4c8e53b8723c062340960680ca419c7f
child 727660 9f004b62ec975362275ced3fa9239e1d069573ae
push id75670
push userbmo:jaws@mozilla.com
push dateWed, 23 Aug 2017 13:41:55 +0000
reviewersgijs
bugs1387313
milestone57.0a1
Bug 1387313 - Allow dragging non-removable items (url bar, back/forward buttons) within their toolbar. r?gijs MozReview-Commit-ID: 1EQxMcLJn9J
browser/base/content/browser.css
browser/components/customizableui/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_allow_dragging_removable_false.js
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -406,17 +406,16 @@ toolbarpaletteitem > toolbaritem[sdkstyl
 
 toolbarbutton.webextension-menuitem > .toolbarbutton-icon {
   width: 16px;
   height: 16px;
 }
 
 toolbarpaletteitem[removable="false"] {
   opacity: 0.5;
-  cursor: default;
 }
 
 %ifndef XP_MACOSX
 toolbarpaletteitem[place="palette"],
 toolbarpaletteitem[place="panel"],
 toolbarpaletteitem[place="toolbar"] {
   -moz-user-focus: normal;
 }
--- a/browser/components/customizableui/CustomizeMode.jsm
+++ b/browser/components/customizableui/CustomizeMode.jsm
@@ -7,17 +7,16 @@
 this.EXPORTED_SYMBOLS = ["CustomizeMode"];
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 const kPrefCustomizationDebug = "browser.uiCustomization.debug";
 const kPrefCustomizationAnimation = "browser.uiCustomization.disableAnimation";
 const kPaletteId = "customization-palette";
 const kDragDataTypePrefix = "text/toolbarwrapper-id/";
-const kPlaceholderClass = "panel-customization-placeholder";
 const kSkipSourceNodePref = "browser.uiCustomization.skipSourceNodeCheck";
 const kToolbarVisibilityBtn = "customization-toolbar-visibility-button";
 const kDrawInTitlebarPref = "browser.tabs.drawInTitlebar";
 const kMaxTransitionDurationMs = 2000;
 const kKeepBroadcastAttributes = "keepbroadcastattributeswhencustomizing";
 
 const kPanelItemContextMenu = "customizationPanelItemContextMenu";
 const kPaletteItemContextMenu = "customizationPaletteItemContextMenu";
@@ -1563,21 +1562,16 @@ CustomizeMode.prototype = {
       if (item.localName == "toolbar") {
         return;
       }
       item = item.parentNode;
     }
 
     let draggedItem = item.firstChild;
     let placeForItem = CustomizableUI.getPlaceForItem(item);
-    let isRemovable = placeForItem == "palette" ||
-                      CustomizableUI.isWidgetRemovable(draggedItem);
-    if (item.classList.contains(kPlaceholderClass) || !isRemovable) {
-      return;
-    }
 
     let dt = aEvent.dataTransfer;
     let documentId = aEvent.target.ownerDocument.documentElement.id;
 
     dt.mozSetDataAt(kDragDataTypePrefix + documentId, draggedItem.id, 0);
     dt.effectAllowed = "move";
 
     let itemRect = this._dwu.getBoundsWithoutFlushing(draggedItem);
@@ -1747,20 +1741,16 @@ CustomizeMode.prototype = {
     // Need to insert *after* this node if we promised the user that:
     if (targetNode != targetArea && dropDir == "after") {
       if (targetNode.nextSibling) {
         targetNode = targetNode.nextSibling;
       } else {
         targetNode = targetArea;
       }
     }
-    // If the target node is a placeholder, get its sibling as the real target.
-    while (targetNode.classList.contains(kPlaceholderClass) && targetNode.nextSibling) {
-      targetNode = targetNode.nextSibling;
-    }
     if (targetNode.tagName == "toolbarpaletteitem") {
       targetNode = targetNode.firstChild;
     }
 
     this._cancelDragActive(this._dragOverItem, null, true);
 
     try {
       this._applyDrop(aEvent, targetArea, originArea, draggedItemId, targetNode);
@@ -1858,17 +1848,17 @@ CustomizeMode.prototype = {
     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)) {
+    if (itemForPlacement) {
       let targetNodeId = (itemForPlacement.nodeName == "toolbarpaletteitem") ?
                             itemForPlacement.firstChild && itemForPlacement.firstChild.id :
                             itemForPlacement.id;
       placement = CustomizableUI.getPlacementOfWidget(targetNodeId);
     }
     if (!placement) {
       log.debug("Could not get a position for " + aTargetNode.nodeName + "#" + aTargetNode.id + "." + aTargetNode.className);
     }
@@ -2209,18 +2199,17 @@ CustomizeMode.prototype = {
   _onMouseDown(aEvent) {
     log.debug("_onMouseDown");
     if (aEvent.button != 0) {
       return;
     }
     let doc = aEvent.target.ownerDocument;
     doc.documentElement.setAttribute("customizing-movingItem", true);
     let item = this._getWrapper(aEvent.target);
-    if (item && !item.classList.contains(kPlaceholderClass) &&
-        item.getAttribute("removable") == "true") {
+    if (item) {
       item.setAttribute("mousedown", "true");
     }
   },
 
   _onMouseUp(aEvent) {
     log.debug("_onMouseUp");
     if (aEvent.button != 0) {
       return;
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -130,16 +130,17 @@ skip-if = os == "linux" # crashing on Li
 [browser_1058573_showToolbarsDropdown.js]
 [browser_1087303_button_fullscreen.js]
 tags = fullscreen
 skip-if = os == "mac"
 [browser_1087303_button_preferences.js]
 [browser_1089591_still_customizable_after_reset.js]
 [browser_1096763_seen_widgets_post_reset.js]
 [browser_1161838_inserted_new_default_buttons.js]
+[browser_allow_dragging_removable_false.js]
 [browser_bootstrapped_custom_toolbar.js]
 [browser_customizemode_contextmenu_menubuttonstate.js]
 [browser_customizemode_uidensity.js]
 [browser_exit_background_customize_mode.js]
 [browser_overflow_use_subviews.js]
 [browser_panel_keyboard_navigation.js]
 [browser_panel_toggle.js]
 [browser_panelUINotifications.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_allow_dragging_removable_false.js
@@ -0,0 +1,26 @@
+"use strict";
+
+/**
+ * Test dragging a removable=false widget within its own area as well as to the palette.
+ */
+add_task(async function() {
+  await startCustomizing();
+  let forwardButton = document.getElementById("forward-button");
+  is(forwardButton.getAttribute("removable"), "false", "forward-button should not be removable");
+  ok(CustomizableUI.inDefaultState, "Should start in default state.");
+
+  let urlbarContainer = document.getElementById("urlbar-container");
+  let placementsAfterDrag = getAreaWidgetIds(CustomizableUI.AREA_NAVBAR);
+  placementsAfterDrag.splice(placementsAfterDrag.indexOf("forward-button"), 1);
+  placementsAfterDrag.splice(placementsAfterDrag.indexOf("urlbar-container"), 0, "forward-button");
+  simulateItemDrag(forwardButton, urlbarContainer);
+  assertAreaPlacements(CustomizableUI.AREA_NAVBAR, placementsAfterDrag);
+  ok(!CustomizableUI.inDefaultState, "Should no longer be in default state.");
+  let palette = document.getElementById("customization-palette");
+  simulateItemDrag(forwardButton, palette);
+  is(CustomizableUI.getPlacementOfWidget("forward-button").area, CustomizableUI.AREA_NAVBAR, "forward-button was not able to move to palette");
+
+  await endCustomizing();
+  await resetCustomization();
+  ok(CustomizableUI.inDefaultState, "Should be in default state again.");
+});