From 2aa29cf4dad726037f3eae203c3c0a65cce3c2de Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Wed, 5 Mar 2014 15:03:00 -0500 Subject: [PATCH] Bug 968565 - [Australis] When inserting a customizable item before another item or at the end of an area, skip over hidden items. r=Gijs --- .../customizableui/src/CustomizeMode.jsm | 16 +++- .../customizableui/test/browser.ini | 1 + .../browser_876926_customize_mode_wrapping.js | 32 ++++++++ ...owser_968565_insert_before_hidden_items.js | 82 +++++++++++++++++++ 4 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 browser/components/customizableui/test/browser_968565_insert_before_hidden_items.js diff --git a/browser/components/customizableui/src/CustomizeMode.jsm b/browser/components/customizableui/src/CustomizeMode.jsm index 0be627d8c86b..7ee859f5bc71 100644 --- a/browser/components/customizableui/src/CustomizeMode.jsm +++ b/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) { diff --git a/browser/components/customizableui/test/browser.ini b/browser/components/customizableui/test/browser.ini index 4896c9ba514e..8220ee61a5bc 100644 --- a/browser/components/customizableui/test/browser.ini +++ b/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] diff --git a/browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js b/browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js index 7a951fe2b107..8c281b94c2ac 100644 --- a/browser/components/customizableui/test/browser_876926_customize_mode_wrapping.js +++ b/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); } diff --git a/browser/components/customizableui/test/browser_968565_insert_before_hidden_items.js b/browser/components/customizableui/test/browser_968565_insert_before_hidden_items.js new file mode 100644 index 000000000000..6afdbc1f6b78 --- /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(); +});