From 708b5dcbd78efcc62b040395cbb47cdd443cf1f0 Mon Sep 17 00:00:00 2001
From: Gijs Kruitbosch <gijskruitbosch@gmail.com>
Date: Mon, 12 May 2014 21:47:17 +0100
Subject: [PATCH] Bug 996635 - fix removing widgets from outside their area,
 r=jaws

--HG--
extra : rebase_source : c6e5fc5b2cb5e79f4fb7c1307c383157f8577c61
---
 .../customizableui/src/CustomizableUI.jsm     | 11 ++---
 .../customizableui/test/browser.ini           |  1 +
 .../test/browser_996635_remove_non_widgets.js | 43 +++++++++++++++++++
 3 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 browser/components/customizableui/test/browser_996635_remove_non_widgets.js

diff --git a/browser/components/customizableui/src/CustomizableUI.jsm b/browser/components/customizableui/src/CustomizableUI.jsm
index 2e2beb761621..cd63a90dbdfc 100644
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -772,15 +772,16 @@ let CustomizableUIInternal = {
         continue;
       }
 
+      let container = areaNode.customizationTarget;
       let widgetNode = window.document.getElementById(aWidgetId);
-      if (!widgetNode) {
+      if (widgetNode && isOverflowable) {
+        container = areaNode.overflowable.getContainerFor(widgetNode);
+      }
+
+      if (!widgetNode || !container.contains(widgetNode)) {
         INFO("Widget not found, unable to remove");
         continue;
       }
-      let container = areaNode.customizationTarget;
-      if (isOverflowable) {
-        container = areaNode.overflowable.getContainerFor(widgetNode);
-      }
 
       this.notifyListeners("onWidgetBeforeDOMChange", widgetNode, null, container, true);
 
diff --git a/browser/components/customizableui/test/browser.ini b/browser/components/customizableui/test/browser.ini
index 549593496f15..2f5ec2c4d231 100644
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -108,6 +108,7 @@ skip-if = os == "linux"
 [browser_993322_widget_notoolbar.js]
 [browser_995164_registerArea_during_customize_mode.js]
 [browser_996364_registerArea_different_properties.js]
+[browser_996635_remove_non_widgets.js]
 [browser_1003588_no_specials_in_panel.js]
 [browser_1008559_anchor_undo_restore.js]
 [browser_bootstrapped_custom_toolbar.js]
diff --git a/browser/components/customizableui/test/browser_996635_remove_non_widgets.js b/browser/components/customizableui/test/browser_996635_remove_non_widgets.js
new file mode 100644
index 000000000000..14a446eecad4
--- /dev/null
+++ b/browser/components/customizableui/test/browser_996635_remove_non_widgets.js
@@ -0,0 +1,43 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// NB: This is testing what happens if something that /isn't/ a customizable
+// widget gets used in CustomizableUI APIs. Don't use this as an example of
+// what should happen in a "normal" case or how you should use the API.
+function test() {
+  // First create a button that isn't customizable, and add it in the nav-bar,
+  // but not in the customizable part of it (the customization target) but
+  // next to the main (hamburger) menu button.
+  const buttonID = "Test-non-widget-non-removable-button";
+  let btn = document.createElement("toolbarbutton");
+  btn.id = buttonID;
+  btn.label = "Hi";
+  btn.setAttribute("style", "width: 20px; height: 20px; background-color: red");
+  document.getElementById("nav-bar").appendChild(btn);
+  registerCleanupFunction(function() {
+    btn.remove();
+  });
+
+  // Now try to add this non-customizable button to the tabstrip. This will
+  // update the internal bookkeeping (ie placements) information, but shouldn't
+  // move the node.
+  CustomizableUI.addWidgetToArea(buttonID, CustomizableUI.AREA_TABSTRIP);
+  let placement = CustomizableUI.getPlacementOfWidget(buttonID);
+  // Check our bookkeeping
+  ok(placement, "Button should be placed");
+  is(placement && placement.area, CustomizableUI.AREA_TABSTRIP, "Should be placed on tabstrip.");
+  // Check we didn't move the node.
+  is(btn.parentNode && btn.parentNode.id, "nav-bar", "Actual button should still be on navbar.");
+
+  // Now remove the node again. This should remove the bookkeeping, but again
+  // not affect the actual node.
+  CustomizableUI.removeWidgetFromArea(buttonID);
+  placement = CustomizableUI.getPlacementOfWidget(buttonID);
+  // Check our bookkeeping:
+  ok(!placement, "Button should no longer have a placement.");
+  // Check our node.
+  is(btn.parentNode && btn.parentNode.id, "nav-bar", "Actual button should still be on navbar.");
+}
+