From 6ba79676278f8ce705898b5ed0ec60bfd005bdf4 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Fri, 15 Nov 2013 15:25:41 +0100 Subject: [PATCH] Bug 939091 - fix forward-moving detection to check if we actually can, r=mikedeboer --- .../src/PanelWideWidgetTracker.jsm | 69 ++++++++++++++----- ...owser_880382_drag_wide_widgets_in_panel.js | 34 +++++++++ 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/browser/components/customizableui/src/PanelWideWidgetTracker.jsm b/browser/components/customizableui/src/PanelWideWidgetTracker.jsm index f3950a459657..8aa28a7ab0f7 100644 --- a/browser/components/customizableui/src/PanelWideWidgetTracker.jsm +++ b/browser/components/customizableui/src/PanelWideWidgetTracker.jsm @@ -74,7 +74,25 @@ let PanelWideWidgetTracker = { }, shouldMoveForward: function(aWidgetId, aPosition) { let currentWidgetAtPosition = gPanelPlacements[aPosition + 1]; - return gWideWidgets.has(currentWidgetAtPosition) && !gWideWidgets.has(aWidgetId); + let rv = gWideWidgets.has(currentWidgetAtPosition) && !gWideWidgets.has(aWidgetId); + // We might now think we can move forward, but for that we need at least 2 more small + // widgets to be present: + if (rv) { + let furtherWidgets = gPanelPlacements.slice(aPosition + 2); + let realWidgets = 0; + if (furtherWidgets.length >= 2) { + while (furtherWidgets.length && realWidgets < 2) { + let w = furtherWidgets.shift(); + if (!gWideWidgets.has(w) && this.checkWidgetStatus(w)) { + realWidgets++; + } + } + } + if (realWidgets < 2) { + rv = false; + } + } + return rv; }, adjustWidgets: function(aWidgetId, aMoveForwards) { if (this.adjusting) { @@ -107,26 +125,12 @@ let PanelWideWidgetTracker = { if (gWideWidgets.has(thisWidgetId)) { continue; } - let widgetWrapper = CustomizableUI.getWidget(gPanelPlacements[placementIndex]); - // This widget might not actually exist: - if (!widgetWrapper) { + let widgetStatus = this.checkWidgetStatus(thisWidgetId); + if (!widgetStatus) { continue; } - // This widget might still not actually exist: - if (widgetWrapper.provider == CustomizableUI.PROVIDER_XUL && - widgetWrapper.instances.length == 0) { - continue; - } - - // Or it might only be there some of the time: - if (widgetWrapper.provider == CustomizableUI.PROVIDER_API && - widgetWrapper.showInPrivateBrowsing === false) { - if (!fixedPos) { - fixedPos = placementIndex; - } else { - fixedPos = Math.min(fixedPos, placementIndex); - } - // We want to position ourselves before this item: + if (widgetStatus == "public-only") { + fixedPos = !fixedPos ? placementIndex : Math.min(fixedPos, placementIndex); prevSiblingCount = 0; } else { prevSiblingCount++; @@ -144,6 +148,33 @@ let PanelWideWidgetTracker = { CustomizableUI.moveWidgetWithinArea(aWidgetId, desiredPos); } }, + + /* + * Check whether a widget id is actually known anywhere. + * @returns false if the widget doesn't exist, + * "public-only" if it's not shown in private windows + * "real" if it does exist and is shown even in private windows + */ + checkWidgetStatus: function(aWidgetId) { + let widgetWrapper = CustomizableUI.getWidget(aWidgetId); + // This widget might not actually exist: + if (!widgetWrapper) { + return false; + } + // This widget might still not actually exist: + if (widgetWrapper.provider == CustomizableUI.PROVIDER_XUL && + widgetWrapper.instances.length == 0) { + return false; + } + + // Or it might only be there some of the time: + if (widgetWrapper.provider == CustomizableUI.PROVIDER_API && + widgetWrapper.showInPrivateBrowsing === false) { + return "public-only"; + } + return "real"; + }, + init: function() { // Initialize our local placements copy and register the listener gPanelPlacements = CustomizableUI.getWidgetIdsInArea(gPanel); diff --git a/browser/components/customizableui/test/browser_880382_drag_wide_widgets_in_panel.js b/browser/components/customizableui/test/browser_880382_drag_wide_widgets_in_panel.js index f672670966f2..9c1af8191522 100644 --- a/browser/components/customizableui/test/browser_880382_drag_wide_widgets_in_panel.js +++ b/browser/components/customizableui/test/browser_880382_drag_wide_widgets_in_panel.js @@ -407,6 +407,40 @@ let gTests = [ ok(CustomizableUI.inDefaultState, "Should still be in default state."); }, }, + { + desc: "Dragging a small button onto the last big button should work.", + setup: startCustomizing, + run: function() { + let editControls = document.getElementById("edit-controls"); + let panel = document.getElementById(CustomizableUI.AREA_PANEL); + let target = panel.getElementsByClassName("panel-customization-placeholder")[0]; + let placementsAfterMove = ["zoom-controls", + "new-window-button", + "privatebrowsing-button", + "save-page-button", + "print-button", + "history-panelmenu", + "fullscreen-button", + "find-button", + "preferences-button", + "add-ons-button", + "edit-controls"]; + simulateItemDrag(editControls, target); + assertAreaPlacements(CustomizableUI.AREA_PANEL, placementsAfterMove); + let itemToDrag = "sync-button"; + let button = document.getElementById(itemToDrag); + placementsAfterMove.push(itemToDrag); + simulateItemDrag(button, editControls); + assertAreaPlacements(CustomizableUI.AREA_PANEL, placementsAfterMove); + + // Put stuff back: + let palette = document.getElementById("customization-palette"); + let zoomControls = document.getElementById("zoom-controls"); + simulateItemDrag(button, palette); + simulateItemDrag(editControls, zoomControls); + ok(CustomizableUI.inDefaultState, "Should be in default state again."); + }, + }, ]; function asyncCleanup() {