Bug 982656 - 'Restore Defaults' should place all 3rd party widgets in the palette. r=Gijs
authorJared Wein <jwein@mozilla.com>
Tue, 10 Jun 2014 10:52:18 -0400
changeset 187887 d93b579522fde4abac45301cdb0d02c20314e3b5
parent 187886 db50d648115a249e98b1cee85899dea16c771d51
child 187888 93c0524d1a411c044d68d02755f4e32c5886396c
push id7215
push userjwein@mozilla.com
push dateTue, 10 Jun 2014 18:24:14 +0000
treeherderfx-team@d93b579522fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs982656
milestone33.0a1
Bug 982656 - 'Restore Defaults' should place all 3rd party widgets in the palette. r=Gijs
browser/components/customizableui/src/CustomizableUI.jsm
browser/components/customizableui/src/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_982656_restore_defaults_builtin_widgets.js
--- a/browser/components/customizableui/src/CustomizableUI.jsm
+++ b/browser/components/customizableui/src/CustomizableUI.jsm
@@ -1924,23 +1924,31 @@ let CustomizableUIInternal = {
       if (cache) {
         cache.delete(widget.id);
       }
     }
 
     this.notifyListeners("onWidgetCreated", widget.id);
 
     if (widget.defaultArea) {
+      let addToDefaultPlacements = false;
       let area = gAreas.get(widget.defaultArea);
-      //XXXgijs this won't have any effect for legacy items. Sort of OK because
-      // consumers can modify currentset? Maybe?
-      if (area.has("defaultPlacements")) {
-        area.get("defaultPlacements").push(widget.id);
-      } else {
-        area.set("defaultPlacements", [widget.id]);
+      if (widget.source == CustomizableUI.SOURCE_BUILTIN) {
+        addToDefaultPlacements = true;
+      } else if (!CustomizableUI.isBuiltinToolbar(widget.defaultArea) &&
+                 widget.defaultArea != CustomizableUI.AREA_PANEL) {
+        addToDefaultPlacements = true;
+      }
+
+      if (addToDefaultPlacements) {
+        if (area.has("defaultPlacements")) {
+          area.get("defaultPlacements").push(widget.id);
+        } else {
+          area.set("defaultPlacements", [widget.id]);
+        }
       }
     }
 
     // Look through previously saved state to see if we're restoring a widget.
     let seenAreas = new Set();
     let widgetMightNeedAutoAdding = true;
     for (let [area, placements] of gPlacements) {
       seenAreas.add(area);
--- a/browser/components/customizableui/src/CustomizeMode.jsm
+++ b/browser/components/customizableui/src/CustomizeMode.jsm
@@ -660,17 +660,17 @@ CustomizeMode.prototype = {
     let numberOfAreas = areas.length;
     for (let i = 0; i < numberOfAreas; i++) {
       let area = areas[i];
       let areaNode = aNode.ownerDocument.getElementById(area);
       let customizationTarget = areaNode && areaNode.customizationTarget;
       if (customizationTarget && customizationTarget != areaNode) {
         areas.push(customizationTarget.id);
       }
-      let overflowTarget = areaNode.getAttribute("overflowtarget");
+      let overflowTarget = areaNode && areaNode.getAttribute("overflowtarget");
       if (overflowTarget) {
         areas.push(overflowTarget);
       }
     }
     areas.push(kPaletteId);
 
     while (aNode && aNode.parentNode) {
       let parent = aNode.parentNode;
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -89,16 +89,17 @@ skip-if = e10s # Bug ?????? - test uses 
 [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_982656_restore_defaults_builtin_widgets.js]
 
 [browser_984455_bookmarks_items_reparenting.js]
 skip-if = os == "linux"
 
 [browser_985815_propagate_setToolbarVisibility.js]
 [browser_981305_separator_insertion.js]
 [browser_988072_sidebar_events.js]
 [browser_989751_subviewbutton_class.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_982656_restore_defaults_builtin_widgets.js
@@ -0,0 +1,57 @@
+/* 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";
+
+// Restoring default should not place addon widgets back in the toolbar
+add_task(function() {
+  ok(CustomizableUI.inDefaultState, "Default state to begin");
+
+  const kWidgetId = "bug982656-add-on-widget-should-not-restore-to-default-area";
+  let widgetSpec = {
+    id: kWidgetId,
+    defaultArea: CustomizableUI.AREA_NAVBAR
+  };
+  CustomizableUI.createWidget(widgetSpec);
+
+  ok(!CustomizableUI.inDefaultState, "Not in default state after widget added");
+  is(CustomizableUI.getPlacementOfWidget(kWidgetId).area, CustomizableUI.AREA_NAVBAR, "Widget should be in navbar");
+
+  yield resetCustomization();
+
+  ok(CustomizableUI.inDefaultState, "Back in default state after reset");
+  is(CustomizableUI.getPlacementOfWidget(kWidgetId), null, "Widget now in palette");
+  CustomizableUI.destroyWidget(kWidgetId);
+});
+
+
+// resetCustomization shouldn't move 3rd party widgets out of custom toolbars
+add_task(function() {
+  const kToolbarId = "bug982656-toolbar-with-defaultset";
+  const kWidgetId = "bug982656-add-on-widget-should-restore-to-default-area-when-area-is-not-builtin";
+  ok(CustomizableUI.inDefaultState, "Everything should be in its default state.");
+  let toolbar = createToolbarWithPlacements(kToolbarId);
+  ok(CustomizableUI.areas.indexOf(kToolbarId) != -1,
+     "Toolbar has been registered.");
+  is(CustomizableUI.getAreaType(kToolbarId), CustomizableUI.TYPE_TOOLBAR,
+     "Area should be registered as toolbar");
+
+  let widgetSpec = {
+    id: kWidgetId,
+    defaultArea: kToolbarId
+  };
+  CustomizableUI.createWidget(widgetSpec);
+
+  ok(!CustomizableUI.inDefaultState, "No longer in default state after toolbar is registered and visible.");
+  is(CustomizableUI.getPlacementOfWidget(kWidgetId).area, kToolbarId, "Widget should be in custom toolbar");
+
+  yield resetCustomization();
+  ok(CustomizableUI.inDefaultState, "Back in default state after reset");
+  is(CustomizableUI.getPlacementOfWidget(kWidgetId).area, kToolbarId, "Widget still in custom toolbar");
+  ok(toolbar.collapsed, "Custom toolbar should be collapsed after reset");
+
+  toolbar.remove();
+  CustomizableUI.destroyWidget(kWidgetId);
+  CustomizableUI.unregisterArea(kToolbarId);
+});