Bug 939091 - fix forward-moving detection to check if we actually can, r=mikedeboer
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 15 Nov 2013 15:25:41 +0100
changeset 155721 7bd5ee7b71e0
parent 155720 0c4c1c29fc19
child 155722 8b514254b168
push id630
push usergijskruitbosch@gmail.com
push date2013-11-15 17:50 +0000
treeherderux@7bd5ee7b71e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs939091
milestone28.0a1
Bug 939091 - fix forward-moving detection to check if we actually can, r=mikedeboer
browser/components/customizableui/src/PanelWideWidgetTracker.jsm
browser/components/customizableui/test/browser_880382_drag_wide_widgets_in_panel.js
--- a/browser/components/customizableui/src/PanelWideWidgetTracker.jsm
+++ b/browser/components/customizableui/src/PanelWideWidgetTracker.jsm
@@ -69,17 +69,35 @@ let PanelWideWidgetTracker = {
   },
   // When widgets get destroyed, we remove them from our sets of stuff we care about:
   onWidgetDestroyed: function(aWidgetId) {
     gSeenWidgets.delete(aWidgetId);
     gWideWidgets.delete(aWidgetId);
   },
   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) {
       return;
     }
     this.adjusting = true;
     let widgetsAffected = [w for (w of gPanelPlacements) if (gWideWidgets.has(w))];
     // If we're moving the wide widgets forwards (down/to the right in the panel)
@@ -102,36 +120,22 @@ let PanelWideWidgetTracker = {
     let placementIndex = gPanelPlacements.indexOf(aWidgetId);
     let prevSiblingCount = 0;
     let fixedPos = null;
     while (placementIndex--) {
       let thisWidgetId = gPanelPlacements[placementIndex];
       if (gWideWidgets.has(thisWidgetId)) {
         continue;
       }
-      let widgetWrapper = CustomizableUI.getWidget(gPanelPlacements[placementIndex]);
-      // This widget might not actually exist:
-      if (!widgetWrapper) {
-        continue;
-      }
-      // This widget might still not actually exist:
-      if (widgetWrapper.provider == CustomizableUI.PROVIDER_XUL &&
-          widgetWrapper.instances.length == 0) {
+      let widgetStatus = this.checkWidgetStatus(thisWidgetId);
+      if (!widgetStatus) {
         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++;
       }
     }
 
     if (fixedPos !== null || prevSiblingCount % kColumnsInMenuPanel) {
       let desiredPos = (fixedPos !== null) ? fixedPos : gPanelPlacements.indexOf(aWidgetId);
@@ -139,14 +143,41 @@ let PanelWideWidgetTracker = {
       if (aMoveForwards && fixedPos == null) {
         // +1 because otherwise we'd count ourselves:
         desiredChange = kColumnsInMenuPanel + desiredChange + 1;
       }
       desiredPos += desiredChange;
       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);
     CustomizableUI.addListener(this);
   },
 };
--- 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
@@ -402,16 +402,50 @@ let gTests = [
       is(developerButton.parentNode.tagName, "toolbarpaletteitem",
          "developer-button should be wrapped by a toolbarpaletteitem");
       let editControls = document.getElementById("edit-controls");
       is(editControls.parentNode.tagName, "toolbarpaletteitem",
          "edit-controls should be wrapped by a toolbarpaletteitem");
       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() {
   yield endCustomizing();
   Services.prefs.clearUserPref("browser.uiCustomization.skipSourceNodeCheck");
   yield resetCustomization();
 }