Bug 968565 - [Australis] When inserting a customizable item before another item or at the end of an area, skip over hidden items. r=Gijs, a=sledru.
authorJared Wein <jwein@mozilla.com>
Wed, 05 Mar 2014 15:03:00 -0500
changeset 183271 262af03ee2c6017c914a77b9983fbaf049f80a54
parent 183270 fd7cb7a420cdfbdb43873b75bb07b4077915f432
child 183272 45cbc102a4145ea2c52b685938d42d35ff2049aa
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, sledru
bugs968565
milestone29.0a2
Bug 968565 - [Australis] When inserting a customizable item before another item or at the end of an area, skip over hidden items. r=Gijs, a=sledru.
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js
browser/components/customizableui/test/browser_968565_insert_before_hidden_items.js
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -1278,30 +1278,32 @@ CustomizeMode.prototype = {
     let targetNode = this._getDragOverNode(aEvent, targetArea, targetIsToolbar, draggedItemId);
 
     // We need to determine the place that the widget is being dropped in
     // the target.
     let dragOverItem, dragValue;
     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;
           let itemRect = dragOverItem.getBoundingClientRect();
           let dropTargetCenter = itemRect.left + (itemRect.width / 2);
           let existingDir = dragOverItem.getAttribute("dragover");
           if ((existingDir == "before") == (direction == "ltr")) {
             dropTargetCenter += (parseInt(dragOverItem.style.borderLeftWidth) || 0) / 2;
@@ -1857,17 +1859,26 @@ CustomizeMode.prototype = {
   },
 
   _removePanelCustomizationPlaceholders: function() {
     let contents = this.panelUIContents;
     let oldPlaceholders = contents.getElementsByClassName(kPlaceholderClass);
     while (oldPlaceholders.length) {
       contents.removeChild(oldPlaceholders[0]);
     }
-  }
+  },
+
+  _findVisiblePreviousSiblingNode: function(aReferenceNode) {
+    while (aReferenceNode &&
+           aReferenceNode.localName == "toolbarpaletteitem" &&
+           aReferenceNode.firstChild.hidden) {
+      aReferenceNode = aReferenceNode.previousSibling;
+    }
+    return aReferenceNode;
+  },
 };
 
 function __dumpDragData(aEvent, caller) {
   if (!gDebug) {
     return;
   }
   let str = "Dumping drag data (CustomizeMode.jsm) {\n";
   str += "  type: " + aEvent["type"] + "\n";
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -61,15 +61,16 @@ skip-if = os == "linux"
 [browser_943683_migration_test.js]
 [browser_944887_destroyWidget_should_destroy_in_palette.js]
 [browser_945739_showInPrivateBrowsing_customize_mode.js]
 [browser_947987_removable_default.js]
 [browser_948985_non_removable_defaultArea.js]
 [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_972267_customizationchange_events.js]
 [browser_973932_addonbar_currentset.js]
 [browser_975719_customtoolbars_behaviour.js]
 [browser_panel_toggle.js]
--- a/browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js
+++ b/browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js
@@ -47,30 +47,62 @@ let move = {
 function isLast(containerId, defaultPlacements, id) {
   assertAreaPlacements(containerId, defaultPlacements.concat([id]));
   is(document.getElementById(containerId).customizationTarget.lastChild.firstChild.id, id,
      "Widget " + id + " should be in " + containerId + " in customizing window.");
   is(otherWin.document.getElementById(containerId).customizationTarget.lastChild.id, 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,
      "Widget " + id + " should be in " + containerId + " in customizing window.");
   is(otherWin.document.getElementById(containerId).customizationTarget.firstChild.id, id,
      "Widget " + id + " should be in " + containerId + " in other window.");
 }
 
 function checkToolbar(id, method) {
   // Place at start of the toolbar:
   let toolbarPlacements = getAreaWidgetIds(kToolbar);
   move[method](id, kToolbar);
   if (method == "dragToItem") {
     isFirst(kToolbar, toolbarPlacements, id);
+  } else if (method == "drag") {
+    isLastVisibleInToolbar(kToolbar, toolbarPlacements, id);
   } else {
     isLast(kToolbar, toolbarPlacements, id);
   }
   checkWrapper(id);
 }
 
 function checkPanel(id, method) {
   let panelPlacements = getAreaWidgetIds(kPanel);
new file mode 100644
--- /dev/null
+++ b/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();
+});