Bug 968565 - [Australis] When inserting a customizable item before another item or at the end of an area, skip over hidden items. r=Gijs

browser/components/customizableui/src/CustomizeMode.jsm

@@ -1302,20 +1302,22 @@ CustomizeMode.prototype = {
     if (targetNode == targetArea.customizationTarget) {
       // We'll assume if the user is dragging directly over the target, that
       // they're attempting to append a child to that target.
-      dragOverItem = targetNode.lastChild || targetNode;
+      dragOverItem = (targetIsToolbar ? this._findVisiblePreviousSiblingNode(targetNode.lastChild) :
+                                        targetNode.lastChild) || targetNode;
       dragValue = "after";
     } else {
       let targetParent = targetNode.parentNode;
       let position = Array.indexOf(targetParent.children, targetNode);
       if (position == -1) {
-        dragOverItem = targetParent.lastChild;
+        dragOverItem = targetIsToolbar ? this._findVisiblePreviousSiblingNode(targetNode.lastChild) :
+                                         targetParent.lastChild;
         dragValue = "after";
       } else {
         dragOverItem = targetParent.children[position];
         if (!targetIsToolbar) {
           dragValue = "before";
-          dragOverItem = position == -1 ? targetParent.firstChild : targetParent.children[position];
         } else {
+          dragOverItem = this._findVisiblePreviousSiblingNode(targetParent.children[position]);
           // Check if the aDraggedItem is hovered past the first half of dragOverItem
           let window = dragOverItem.ownerDocument.defaultView;
           let direction = window.getComputedStyle(dragOverItem, null).direction;
@@ -1924,6 +1926,14 @@ CustomizeMode.prototype = {
     }
   },
 
+  _findVisiblePreviousSiblingNode: function(aReferenceNode) {
+    while (aReferenceNode &&
+           aReferenceNode.localName == "toolbarpaletteitem" &&
+           aReferenceNode.firstChild.hidden) {
+      aReferenceNode = aReferenceNode.previousSibling;
+    }
+    return aReferenceNode;
+  },
 };
 
 function __dumpDragData(aEvent, caller) {

browser/components/customizableui/test/browser.ini

@@ -66,6 +66,7 @@ skip-if = os == "linux"
 [browser_952963_areaType_getter_no_area.js]
 [browser_956602_remove_special_widget.js]
 [browser_968447_bookmarks_toolbar_items_in_panel.js]
+[browser_968565_insert_before_hidden_items.js]
 [browser_969427_recreate_destroyed_widget_after_reset.js]
 [browser_969661_character_encoding_navbar_disabled.js]
 [browser_970511_undo_restore_default.js]

browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js

@@ -52,6 +52,36 @@ function isLast(containerId, defaultPlacements, id) {
      "Widget " + id + " should be in " + containerId + " in other window.");
 }
 
+function getLastVisibleNodeInToolbar(containerId, win=window) {
+  let container = win.document.getElementById(containerId).customizationTarget;
+  let rv = container.lastChild;
+  while (rv && (rv.getAttribute('hidden') == 'true' || (rv.firstChild && rv.firstChild.getAttribute('hidden') == 'true'))) {
+    rv = rv.previousSibling;
+  }
+  return rv;
+}
+
+function isLastVisibleInToolbar(containerId, defaultPlacements, id) {
+  let newPlacements;
+  for (let i = defaultPlacements.length - 1; i >= 0; i--) {
+    let el = document.getElementById(defaultPlacements[i]);
+    if (el && el.getAttribute('hidden') != 'true') {
+      newPlacements = [...defaultPlacements];
+      newPlacements.splice(i + 1, 0, id);
+      break;
+    }
+  }
+  if (!newPlacements) {
+    assertAreaPlacements(containerId, defaultPlacements.concat([id]));
+  } else {
+    assertAreaPlacements(containerId, newPlacements);
+  }
+  is(getLastVisibleNodeInToolbar(containerId).firstChild.id, id,
+     "Widget " + id + " should be in " + containerId + " in customizing window.");
+  is(getLastVisibleNodeInToolbar(containerId, otherWin).id, id,
+     "Widget " + id + " should be in " + containerId + " in other window.");
+}
+
 function isFirst(containerId, defaultPlacements, id) {
   assertAreaPlacements(containerId, [id].concat(defaultPlacements));
   is(document.getElementById(containerId).customizationTarget.firstChild.firstChild.id, id,
@@ -66,6 +96,8 @@ function checkToolbar(id, method) {
   move[method](id, kToolbar);
   if (method == "dragToItem") {
     isFirst(kToolbar, toolbarPlacements, id);
+  } else if (method == "drag") {
+    isLastVisibleInToolbar(kToolbar, toolbarPlacements, id);
   } else {
     isLast(kToolbar, toolbarPlacements, id);
   }

browser/components/customizableui/test/browser_968565_insert_before_hidden_items.js

@@ -0,0 +1,82 @@
+/* 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/. */
+
+"use strict";
+
+const kHidden1Id = "test-hidden-button-1";
+const kHidden2Id = "test-hidden-button-2";
+
+let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
+
+// When we drag an item onto a customizable area, and not over a specific target, we
+// should assume that we're appending them to the area. If doing so, we should scan
+// backwards over any hidden items and insert the item before those hidden items.
+add_task(function() {
+  ok(CustomizableUI.inDefaultState, "Should be in the default state");
+
+  // Iterate backwards over the items in the nav-bar until we find the first
+  // one that is not hidden.
+  let placements = CustomizableUI.getWidgetsInArea(CustomizableUI.AREA_NAVBAR);
+  let lastVisible = null;
+  for (let widgetGroup of placements.reverse()) {
+    let widget = widgetGroup.forWindow(window);
+    if (widget && widget.node && !widget.node.hidden) {
+      lastVisible = widget.node;
+      break;
+    }
+  }
+
+  if (!lastVisible) {
+    ok(false, "Apparently, there are no visible items in the nav-bar.");
+  }
+
+  info("The last visible item in the nav-bar has ID: " + lastVisible.id);
+
+  let hidden1 = createDummyXULButton(kHidden1Id, "You can't see me");
+  let hidden2 = createDummyXULButton(kHidden2Id, "You can't see me either.");
+  hidden1.hidden = hidden2.hidden = true;
+
+  // Make sure we have some hidden items at the end of the nav-bar.
+  navbar.insertItem(hidden1.id);
+  navbar.insertItem(hidden2.id);
+
+  // Drag an item and drop it onto the nav-bar customization target, but
+  // not over a particular item.
+  yield startCustomizing();
+  let downloadsButton = document.getElementById("downloads-button");
+  simulateItemDrag(downloadsButton, navbar.customizationTarget);
+
+  yield endCustomizing();
+
+  is(downloadsButton.previousSibling.id, lastVisible.id,
+     "The downloads button should be placed after the last visible item.");
+
+  yield resetCustomization();
+});
+
+// When we drag an item onto a target that has a hidden element before it, we should
+// instead place the new item before the hidden elements.
+add_task(function() {
+  ok(CustomizableUI.inDefaultState, "Should be in the default state");
+
+  let hidden1 = createDummyXULButton(kHidden1Id, "You can't see me");
+  hidden1.hidden = true;
+
+  let homeButton = document.getElementById("home-button");
+  CustomizableUI.addWidgetToArea(kHidden1Id, CustomizableUI.AREA_NAVBAR,
+                                 CustomizableUI.getPlacementOfWidget(homeButton.id).position);
+
+  hidden1 = document.getElementById(kHidden1Id);
+  is(hidden1.nextSibling.id, homeButton.id, "The hidden item should be before the home button");
+
+  yield startCustomizing();
+  let downloadsButton = document.getElementById("downloads-button");
+  simulateItemDrag(downloadsButton.parentNode, homeButton.parentNode);
+  yield endCustomizing();
+
+  is(hidden1.nextSibling.id, homeButton.id, "The hidden item should still be before the home button");
+  is(downloadsButton.nextSibling.id, hidden1.id, "The downloads button should now be before the hidden button");
+
+  yield resetCustomization();
+});