Bug 976792 - [australis] CustomizableUI's insertNodeInWindow should go through placements and try followup next nodes. r=Gijs r=jaws

--HG--
extra : rebase_source : 47cb1183f584fe453be6b3c461c0eff7cdcbf203

browser/components/customizableui/src/CustomizableUI.jsm

@@ -884,15 +884,14 @@ let CustomizableUIInternal = {
       return;
     }
 
-    let nextNodeId = placements[aPosition + 1];
     // Go through each of the nodes associated with this area and move the
     // widget to the requested location.
     for (let areaNode of areaNodes) {
-      this.insertNodeInWindow(aWidgetId, areaNode, nextNodeId, isNew);
+      this.insertNodeInWindow(aWidgetId, areaNode, isNew);
     }
   },
 
-  insertNodeInWindow: function(aWidgetId, aAreaNode, aNextNodeId, isNew) {
+  insertNodeInWindow: function(aWidgetId, aAreaNode, isNew) {
     let window = aAreaNode.ownerDocument.defaultView;
     let showInPrivateBrowsing = gPalette.has(aWidgetId)
                               ? gPalette.get(aWidgetId).showInPrivateBrowsing
@@ -916,8 +915,7 @@ let CustomizableUIInternal = {
       }
     }
 
-    let container = aAreaNode.customizationTarget;
-    let [insertionContainer, nextNode] = this.findInsertionPoints(widgetNode, aNextNodeId, aAreaNode);
+    let [insertionContainer, nextNode] = this.findInsertionPoints(widgetNode, aAreaNode);
     this.insertWidgetBefore(widgetNode, nextNode, insertionContainer, areaId);
 
     if (gAreas.get(areaId).get("type") == CustomizableUI.TYPE_TOOLBAR) {
@@ -925,17 +923,29 @@ let CustomizableUIInternal = {
     }
   },
 
-  findInsertionPoints: function(aNode, aNextNodeId, aAreaNode) {
-    let props = gAreas.get(aAreaNode.id);
-    if (props.get("type") == CustomizableUI.TYPE_TOOLBAR && props.get("overflowable") &&
-        aAreaNode.getAttribute("overflowing") == "true") {
-      return aAreaNode.overflowable.getOverflowedInsertionPoints(aNode, aNextNodeId);
+  findInsertionPoints: function(aNode, aAreaNode) {
+    let areaId = aAreaNode.id;
+    let props = gAreas.get(areaId);
+
+    // For overflowable toolbars, rely on them (because the work is more complicated):
+    if (props.get("type") == CustomizableUI.TYPE_TOOLBAR && props.get("overflowable")) {
+      return aAreaNode.overflowable.findOverflowedInsertionPoints(aNode);
     }
-    let nextNode = null;
-    if (aNextNodeId) {
-      nextNode = aAreaNode.customizationTarget.getElementsByAttribute("id", aNextNodeId)[0];
+
+    let container = aAreaNode.customizationTarget;
+    let placements = gPlacements.get(areaId);
+    let nodeIndex = placements.indexOf(aNode.id);
+
+    while (++nodeIndex < placements.length) {
+      let nextNodeId = placements[nodeIndex];
+      let nextNode = container.getElementsByAttribute("id", nextNodeId).item(0);
+
+      if (nextNode) {
+        return [container, nextNode];
+      }
     }
-    return [aAreaNode.customizationTarget, nextNode];
+
+    return [container, null];
   },
 
   insertWidgetBefore: function(aNode, aNextNode, aContainer, aArea) {
@@ -2284,9 +2294,7 @@ let CustomizableUIInternal = {
       return true;
     }
 
-    let placementAry = gPlacements.get(placement.area);
-    let nextNodeId = placementAry[placement.position + 1];
-    this.insertNodeInWindow(aWidgetId, container[0], nextNodeId, true);
+    this.insertNodeInWindow(aWidgetId, container[0], true);
     return true;
   },
 
@@ -3772,23 +3780,37 @@ OverflowableToolbar.prototype = {
     }
   },
 
-  getOverflowedInsertionPoints: function(aNode, aNextNodeId) {
-    if (aNode.getAttribute("overflows") == "false") {
-      return [this._target, null];
-    }
-    // Inserting at the end means we're in the overflow list by definition:
-    if (!aNextNodeId) {
-      return [this._list, null];
+  findOverflowedInsertionPoints: function(aNode) {
+    let newNodeCanOverflow = aNode.getAttribute("overflows") != "false";
+    let areaId = this._toolbar.id;
+    let placements = gPlacements.get(areaId);
+    let nodeIndex = placements.indexOf(aNode.id);
+    let nodeBeforeNewNodeIsOverflown = false;
+
+    let loopIndex = -1;
+    while (++loopIndex < placements.length) {
+      let nextNodeId = placements[loopIndex];
+      if (loopIndex > nodeIndex) {
+        if (newNodeCanOverflow && this._collapsed.has(nextNodeId)) {
+          let nextNode = this._list.getElementsByAttribute("id", nextNodeId).item(0);
+          if (nextNode) {
+            return [this._list, nextNode];
+          }
+        }
+        if (!nodeBeforeNewNodeIsOverflown || !newNodeCanOverflow) {
+          let nextNode = this._target.getElementsByAttribute("id", nextNodeId).item(0);
+          if (nextNode) {
+            return [this._target, nextNode];
+          }
+        }
+      } else if (loopIndex < nodeIndex && this._collapsed.has(nextNodeId)) {
+        nodeBeforeNewNodeIsOverflown = true;
+      }
     }
 
-    let nextNode = this._list.getElementsByAttribute("id", aNextNodeId)[0];
-    // If this is the first item, we can actually just append the node
-    // to the end of the toolbar.  If it results in an overflow event, we'll move
-    // the new node to the overflow target.
-    if (!nextNode.previousSibling) {
-      return [this._target, null];
-    }
-    return [this._list, nextNode];
+    let containerForAppending = (this._collapsed.size && newNodeCanOverflow) ?
+                                this._list : this._target;
+    return [containerForAppending, null];
   },
 
   getContainerFor: function(aNode) {

browser/components/customizableui/test/browser.ini

@@ -73,6 +73,10 @@ skip-if = os == "linux"
 [browser_972267_customizationchange_events.js]
 [browser_973932_addonbar_currentset.js]
 [browser_975719_customtoolbars_behaviour.js]
+
+[browser_976792_insertNodeInWindow.js]
+skip-if = os == "linux"
+
 [browser_978084_dragEnd_after_move.js]
 [browser_980155_add_overflow_toolbar.js]
 [browser_981418-widget-onbeforecreated-handler.js]

browser/components/customizableui/test/browser_976792_insertNodeInWindow.js

@@ -0,0 +1,414 @@
+/* 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 kToolbarName = "test-insertNodeInWindow-placements-toolbar";
+const kTestWidgetPrefix = "test-widget-for-insertNodeInWindow-placements-";
+
+
+/*
+Tries to replicate the situation of having a placement list like this:
+
+exists-1,trying-to-insert-this,doesn't-exist,exists-2
+*/
+add_task(function() {
+  let testWidgetExists = [true, false, false, true];
+  let widgetIds = [];
+  for (let i = 0; i < testWidgetExists.length; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    if (testWidgetExists[i]) {
+      let spec = {id: id, type: "button", removable: true, label: "test", tooltiptext: "" + i};
+      CustomizableUI.createWidget(spec);
+    }
+  }
+
+  let toolbarNode = createToolbarWithPlacements(kToolbarName, widgetIds);
+  assertAreaPlacements(kToolbarName, widgetIds);
+
+  let btnId = kTestWidgetPrefix + 1;
+  let btn = createDummyXULButton(btnId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(btnId, window);
+
+  is(btn.parentNode.id, kToolbarName, "New XUL widget should be placed inside new toolbar");
+
+  is(btn.previousSibling.id, toolbarNode.firstChild.id,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  btn.remove();
+  removeCustomToolbars();
+  yield resetCustomization();
+});
+
+
+/*
+Tests nodes get placed inside the toolbar's overflow as expected. Replicates a
+situation similar to:
+
+exists-1,exists-2,overflow-1,trying-to-insert-this,overflow-2
+*/
+add_task(function() {
+  let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
+
+  let widgetIds = [];
+  for (let i = 0; i < 5; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    let spec = {id: id, type: "button", removable: true, label: "insertNodeInWindow test", tooltiptext: "" + i};
+    CustomizableUI.createWidget(spec);
+    CustomizableUI.addWidgetToArea(id, "nav-bar");
+  }
+
+  for (let id of widgetIds) {
+    document.getElementById(id).style.minWidth = "200px";
+  }
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => navbar.hasAttribute("overflowing"));
+
+  let testWidgetId = kTestWidgetPrefix + 3;
+
+  CustomizableUI.destroyWidget(testWidgetId);
+
+  let btn = createDummyXULButton(testWidgetId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(testWidgetId, window);
+
+  is(btn.parentNode.id, navbar.overflowable._list.id, "New XUL widget should be placed inside overflow of toolbar");
+  is(btn.previousSibling.id, kTestWidgetPrefix + 2,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+  is(btn.nextSibling.id, kTestWidgetPrefix + 4,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  CustomizableUI.removeWidgetFromArea(btn.id, kToolbarName);
+  btn.remove();
+  yield resetCustomization();
+  yield waitForCondition(() => !navbar.hasAttribute("overflowing"));
+});
+
+
+/*
+Tests nodes get placed inside the toolbar's overflow as expected. Replicates a
+placements situation similar to:
+
+exists-1,exists-2,overflow-1,doesn't-exist,trying-to-insert-this,overflow-2
+*/
+add_task(function() {
+  let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
+
+  let widgetIds = [];
+  for (let i = 0; i < 5; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    let spec = {id: id, type: "button", removable: true, label: "insertNodeInWindow test", tooltiptext: "" + i};
+    CustomizableUI.createWidget(spec);
+    CustomizableUI.addWidgetToArea(id, "nav-bar");
+  }
+
+  for (let id of widgetIds) {
+    document.getElementById(id).style.minWidth = "200px";
+  }
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => navbar.hasAttribute("overflowing"));
+
+  let testWidgetId = kTestWidgetPrefix + 3;
+
+  CustomizableUI.destroyWidget(kTestWidgetPrefix + 2);
+  CustomizableUI.destroyWidget(testWidgetId);
+
+  let btn = createDummyXULButton(testWidgetId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(testWidgetId, window);
+
+  is(btn.parentNode.id, navbar.overflowable._list.id, "New XUL widget should be placed inside overflow of toolbar");
+  is(btn.previousSibling.id, kTestWidgetPrefix + 1,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+  is(btn.nextSibling.id, kTestWidgetPrefix + 4,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  CustomizableUI.removeWidgetFromArea(btn.id, kToolbarName);
+  btn.remove();
+  yield resetCustomization();
+  yield waitForCondition(() => !navbar.hasAttribute("overflowing"));
+});
+
+
+/*
+Tests nodes get placed inside the toolbar's overflow as expected. Replicates a
+placements situation similar to:
+
+exists-1,exists-2,overflow-1,doesn't-exist,trying-to-insert-this,doesn't-exist
+*/
+add_task(function() {
+  let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
+
+  let widgetIds = [];
+  for (let i = 0; i < 5; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    let spec = {id: id, type: "button", removable: true, label: "insertNodeInWindow test", tooltiptext: "" + i};
+    CustomizableUI.createWidget(spec);
+    CustomizableUI.addWidgetToArea(id, "nav-bar");
+  }
+
+  for (let id of widgetIds) {
+    document.getElementById(id).style.minWidth = "200px";
+  }
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => navbar.hasAttribute("overflowing"));
+
+  let testWidgetId = kTestWidgetPrefix + 3;
+
+  CustomizableUI.destroyWidget(kTestWidgetPrefix + 2);
+  CustomizableUI.destroyWidget(testWidgetId);
+  CustomizableUI.destroyWidget(kTestWidgetPrefix + 4);
+
+  let btn = createDummyXULButton(testWidgetId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(testWidgetId, window);
+
+  is(btn.parentNode.id, navbar.overflowable._list.id, "New XUL widget should be placed inside overflow of toolbar");
+  is(btn.previousSibling.id, kTestWidgetPrefix + 1,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+  is(btn.nextSibling, null,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  CustomizableUI.removeWidgetFromArea(btn.id, kToolbarName);
+  btn.remove();
+  yield resetCustomization();
+  yield waitForCondition(() => !navbar.hasAttribute("overflowing"));
+});
+
+
+/*
+Tests nodes get placed inside the toolbar's overflow as expected. Replicates a
+placements situation similar to:
+
+exists-1,exists-2,overflow-1,can't-overflow,trying-to-insert-this,overflow-2
+*/
+add_task(function() {
+  let navbar = document.getElementById(CustomizableUI.AREA_NAVBAR);
+
+  let widgetIds = [];
+  for (let i = 5; i >= 0; i--) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    let spec = {id: id, type: "button", removable: true, label: "insertNodeInWindow test", tooltiptext: "" + i};
+    CustomizableUI.createWidget(spec);
+    CustomizableUI.addWidgetToArea(id, "nav-bar", 0);
+  }
+
+  for (let i = 10; i < 15; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    let spec = {id: id, type: "button", removable: true, label: "insertNodeInWindow test", tooltiptext: "" + i};
+    CustomizableUI.createWidget(spec);
+    CustomizableUI.addWidgetToArea(id, "nav-bar");
+  }
+
+  for (let id of widgetIds) {
+    document.getElementById(id).style.minWidth = "200px";
+  }
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => navbar.hasAttribute("overflowing"));
+
+  // Find last widget that doesn't allow overflowing
+  let nonOverflowing = navbar.customizationTarget.lastChild;
+  is(nonOverflowing.getAttribute("overflows"), "false", "Last child is expected to not allow overflowing");
+  isnot(nonOverflowing.getAttribute("skipintoolbarset"), "true", "Last child is expected to not be skipintoolbarset");
+
+  let testWidgetId = kTestWidgetPrefix + 10;
+  CustomizableUI.destroyWidget(testWidgetId);
+
+  let btn = createDummyXULButton(testWidgetId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(testWidgetId, window);
+
+  is(btn.parentNode.id, navbar.overflowable._list.id, "New XUL widget should be placed inside overflow of toolbar");
+  is(btn.nextSibling.id, kTestWidgetPrefix + 11,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  CustomizableUI.removeWidgetFromArea(btn.id, kToolbarName);
+  btn.remove();
+  yield resetCustomization();
+  yield waitForCondition(() => !navbar.hasAttribute("overflowing"));
+});
+
+
+/*
+Tests nodes get placed inside the toolbar's overflow as expected. Replicates a
+placements situation similar to:
+
+exists-1,exists-2,overflow-1,trying-to-insert-this,can't-overflow,overflow-2
+*/
+add_task(function() {
+  let widgetIds = [];
+  let missingId = 2;
+  let nonOverflowableId = 3;
+  for (let i = 0; i < 5; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    if (i != missingId) {
+      // Setting min-width to make the overflow state not depend on styling of the button and/or
+      // screen width
+      let spec = {id: id, type: "button", removable: true, label: "test", tooltiptext: "" + i,
+                  onCreated: function(node) {
+                    node.style.minWidth = "200px";
+                    if (id == (kTestWidgetPrefix + nonOverflowableId)) {
+                      node.setAttribute("overflows", false);
+                    }
+                 }};
+      info("Creating: " + id);
+      CustomizableUI.createWidget(spec);
+    }
+  }
+
+  let toolbarNode = createOverflowableToolbarWithPlacements(kToolbarName, widgetIds);
+  assertAreaPlacements(kToolbarName, widgetIds);
+  ok(!toolbarNode.hasAttribute("overflowing"), "Toolbar shouldn't overflow to start with.");
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => toolbarNode.hasAttribute("overflowing"));
+  ok(toolbarNode.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
+
+  let btnId = kTestWidgetPrefix + missingId;
+  let btn = createDummyXULButton(btnId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(btnId, window);
+
+  is(btn.parentNode.id, kToolbarName + "-overflow-list", "New XUL widget should be placed inside new toolbar's overflow");
+  is(btn.previousSibling.id, kTestWidgetPrefix + 1,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+  is(btn.nextSibling.id, kTestWidgetPrefix + 4,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+  yield waitForCondition(() => !toolbarNode.hasAttribute("overflowing"));
+
+  btn.remove();
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  removeCustomToolbars();
+  yield resetCustomization();
+});
+
+
+/*
+Tests nodes do *not* get placed in the toolbar's overflow. Replicates a
+plcements situation similar to:
+
+exists-1,trying-to-insert-this,exists-2,overflowed-1
+*/
+add_task(function() {
+  let widgetIds = [];
+  let missingId = 1;
+  for (let i = 0; i < 5; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    if (i != missingId) {
+      // Setting min-width to make the overflow state not depend on styling of the button and/or
+      // screen width
+      let spec = {id: id, type: "button", removable: true, label: "test", tooltiptext: "" + i,
+                  onCreated: function(node) { node.style.minWidth = "100px"; }};
+      info("Creating: " + id);
+      CustomizableUI.createWidget(spec);
+    }
+  }
+
+  let toolbarNode = createOverflowableToolbarWithPlacements(kToolbarName, widgetIds);
+  assertAreaPlacements(kToolbarName, widgetIds);
+  ok(!toolbarNode.hasAttribute("overflowing"), "Toolbar shouldn't overflow to start with.");
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => toolbarNode.hasAttribute("overflowing"));
+  ok(toolbarNode.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
+
+  let btnId = kTestWidgetPrefix + missingId;
+  let btn = createDummyXULButton(btnId, "test");
+  CustomizableUI.ensureWidgetPlacedInWindow(btnId, window);
+
+  is(btn.parentNode.id, kToolbarName + "-target", "New XUL widget should be placed inside new toolbar");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+  yield waitForCondition(() => !toolbarNode.hasAttribute("overflowing"));
+
+  btn.remove();
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  removeCustomToolbars();
+  yield resetCustomization();
+});
+
+
+/*
+Tests inserting a node onto the end of an overflowing toolbar *doesn't* put it in
+the overflow list when the widget disallows overflowing. ie:
+
+exists-1,exists-2,overflows-1,trying-to-insert-this
+
+Where trying-to-insert-this has overflows=false
+*/
+add_task(function() {
+  let widgetIds = [];
+  let missingId = 3;
+  for (let i = 0; i < 5; i++) {
+    let id = kTestWidgetPrefix + i;
+    widgetIds.push(id);
+    if (i != missingId) {
+      // Setting min-width to make the overflow state not depend on styling of the button and/or
+      // screen width
+      let spec = {id: id, type: "button", removable: true, label: "test", tooltiptext: "" + i,
+                  onCreated: function(node) { node.style.minWidth = "200px"; }};
+      info("Creating: " + id);
+      CustomizableUI.createWidget(spec);
+    }
+  }
+
+  let toolbarNode = createOverflowableToolbarWithPlacements(kToolbarName, widgetIds);
+  assertAreaPlacements(kToolbarName, widgetIds);
+  ok(!toolbarNode.hasAttribute("overflowing"), "Toolbar shouldn't overflow to start with.");
+
+  let originalWindowWidth = window.outerWidth;
+  window.resizeTo(400, window.outerHeight);
+  yield waitForCondition(() => toolbarNode.hasAttribute("overflowing"));
+  ok(toolbarNode.hasAttribute("overflowing"), "Should have an overflowing toolbar.");
+
+  let btnId = kTestWidgetPrefix + missingId;
+  let btn = createDummyXULButton(btnId, "test");
+  btn.setAttribute("overflows", false);
+  CustomizableUI.ensureWidgetPlacedInWindow(btnId, window);
+
+  is(btn.parentNode.id, kToolbarName + "-target", "New XUL widget should be placed inside new toolbar");
+  is(btn.nextSibling, null,
+     "insertNodeInWindow should have placed new XUL widget in correct place in DOM according to placements");
+
+  window.resizeTo(originalWindowWidth, window.outerHeight);
+  yield waitForCondition(() => !toolbarNode.hasAttribute("overflowing"));
+
+  btn.remove();
+  widgetIds.forEach(id => CustomizableUI.destroyWidget(id));
+  removeCustomToolbars();
+  yield resetCustomization();
+});
+
+
+add_task(function asyncCleanUp() {
+  yield resetCustomization();
+});