Bug 1518500 - ensure we don't end up without a flexible space if movi…

…ng items directly to toolbars, r=dao

Before this patch, we change aDraggedItemId somewhat late in the _applyDrop method -
significantly, we do this after the aTargetNode == areaCustomizationTarget check. So
we end up bailing out before adjusting aDraggedItemId, and we add the specific dummy
item from the palette into the toolbar, rather than adding a new spring to the
toolbar, and leaving the existing palette item alone.

Simply moving this adjustment to aDraggedItemId earlier into the method is sufficient
to fix the issue at hand.

Differential Revision: https://phabricator.services.mozilla.com/D62948

browser/components/customizableui/CustomizeMode.jsm

@@ -1882,7 +1882,7 @@ CustomizeMode.prototype = {
 
     let document = aEvent.target.ownerDocument;
     let documentId = document.documentElement.id;
-    if (!aEvent.dataTransfer.mozTypesAt(0)) {
+    if (!aEvent.dataTransfer.mozTypesAt(0).length) {
       return;
     }
 
@@ -2143,6 +2143,16 @@ CustomizeMode.prototype = {
       return;
     }
 
+    // Force creating a new spacer/spring/separator if dragging from the palette
+    if (
+      CustomizableUI.isSpecialWidget(aDraggedItemId) &&
+      aOriginArea.id == kPaletteId
+    ) {
+      aDraggedItemId = aDraggedItemId.match(
+        /^customizableui-special-(spring|spacer|separator)/
+      )[1];
+    }
+
     // Is the target the customization area itself? If so, we just add the
     // widget to the end of the area.
     if (aTargetNode == areaCustomizationTarget) {
@@ -2190,16 +2200,6 @@ CustomizeMode.prototype = {
     }
     let position = placement ? placement.position : null;
 
-    // Force creating a new spacer/spring/separator if dragging from the palette
-    if (
-      CustomizableUI.isSpecialWidget(aDraggedItemId) &&
-      aOriginArea.id == kPaletteId
-    ) {
-      aDraggedItemId = aDraggedItemId.match(
-        /^customizableui-special-(spring|spacer|separator)/
-      )[1];
-    }
-
     // 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.

browser/components/customizableui/test/browser.ini

@@ -145,6 +145,7 @@ 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_flexible_space_area.js]
 [browser_insert_before_moved_node.js]
 [browser_library_after_appMenu.js]
 [browser_overflow_use_subviews.js]

browser/components/customizableui/test/browser_flexible_space_area.js

@@ -0,0 +1,48 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+function getSpringCount(area) {
+  return CustomizableUI.getWidgetIdsInArea(area).filter(id =>
+    id.includes("spring")
+  ).length;
+}
+
+/**
+ * Check that no matter where we add a flexible space, we
+ * never end up without a flexible space in the palette.
+ */
+add_task(async function test_flexible_space_addition() {
+  await startCustomizing();
+  let palette = document.getElementById("customization-palette");
+  // Make the bookmarks toolbar visible:
+  CustomizableUI.setToolbarVisibility(CustomizableUI.AREA_BOOKMARKS, true);
+  let areas = [CustomizableUI.AREA_NAVBAR, CustomizableUI.AREA_BOOKMARKS];
+  if (AppConstants.platform != "macosx") {
+    areas.push(CustomizableUI.AREA_MENUBAR);
+  }
+
+  for (let area of areas) {
+    let spacer = palette.querySelector("toolbarspring");
+    let toolbar = document.getElementById(area);
+    toolbar = CustomizableUI.getCustomizationTarget(toolbar);
+
+    let springCount = getSpringCount(area);
+    simulateItemDrag(spacer, toolbar);
+    // Check we added the spring:
+    is(
+      springCount + 1,
+      getSpringCount(area),
+      "Should now have an extra spring"
+    );
+
+    // Check there's still one in the palette:
+    let newSpacer = palette.querySelector("toolbarspring");
+    ok(newSpacer, "Should have created a new spring");
+  }
+});
+registerCleanupFunction(async function asyncCleanup() {
+  await endCustomizing();
+  await resetCustomization();
+});