From dde680ceea2312bacf2526d4fccf6f96147c1de9 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Wed, 11 Dec 2013 21:59:04 +0100 Subject: [PATCH] Bug 947987 - Adjust how Australis' CustomizableUI deals with removable widgets, r=jaws --HG-- extra : rebase_source : c25f4af10e02071b939eaff2ae79f68cb8ca96e8 --- .../customizableui/src/CustomizableUI.jsm | 24 ++++-- .../src/CustomizableWidgets.jsm | 14 ---- .../customizableui/test/browser.ini | 1 + ...wser_938995_indefaultstate_nonremovable.js | 11 +-- .../test/browser_947987_removable_default.js | 79 +++++++++++++++++++ 5 files changed, 105 insertions(+), 24 deletions(-) create mode 100644 browser/components/customizableui/test/browser_947987_removable_default.js diff --git a/browser/components/customizableui/src/CustomizableUI.jsm b/browser/components/customizableui/src/CustomizableUI.jsm index de01bb7eb0e..87c705a0ac1 100644 --- a/browser/components/customizableui/src/CustomizableUI.jsm +++ b/browser/components/customizableui/src/CustomizableUI.jsm @@ -457,10 +457,17 @@ let CustomizableUIInternal = { // If the placements have items in them which are (now) no longer removable, // we shouldn't be moving them: - if (node.parentNode != container && !this.isWidgetRemovable(node)) { + if (provider == CustomizableUI.PROVIDER_API) { + let widgetInfo = gPalette.get(id); + if (!widgetInfo.removable && aArea != widgetInfo.defaultArea) { + placementsToRemove.add(id); + continue; + } + } else if (provider == CustomizableUI.PROVIDER_XUL && + node.parentNode != container && !this.isWidgetRemovable(node)) { placementsToRemove.add(id); continue; - } + } // Special widgets are always removable, so no need to check them if (inPrivateWindow && provider == CustomizableUI.PROVIDER_API) { let widget = gPalette.get(id); @@ -1736,7 +1743,7 @@ let CustomizableUIInternal = { source: aSource || "addon", instances: new Map(), currentArea: null, - removable: false, + removable: true, overflows: true, defaultArea: null, shortcutId: null, @@ -1778,6 +1785,11 @@ let CustomizableUIInternal = { if (aData.defaultArea && gAreas.has(aData.defaultArea)) { widget.defaultArea = aData.defaultArea; + } else if (!widget.removable) { + ERROR("Widget '" + widget.id + "' is not removable but does not specify " + + "a valid defaultArea. That's not possible; it must specify a " + + "valid defaultArea as well."); + return null; } if ("type" in aData && gSupportedWidgetTypes.has(aData.type)) { @@ -2383,11 +2395,13 @@ this.CustomizableUI = { * invoked when a user hides your view. * - tooltiptext: string to use for the tooltip of the widget * - label: string to use for the label of the widget - * - removable: whether the widget is removable (optional, default: false) + * - removable: whether the widget is removable (optional, default: true) + * NB: if you specify false here, you must provide a + * defaultArea, too. * - overflows: whether widget can overflow when in an overflowable * toolbar (optional, default: true) * - defaultArea: default area to add the widget to - * (optional, default: none) + * (optional, default: none; required if non-removable) * - shortcutId: id of an element that has a shortcut for this widget * (optional, default: null). This is only used to display * the shortcut as part of the tooltip for builtin widgets diff --git a/browser/components/customizableui/src/CustomizableWidgets.jsm b/browser/components/customizableui/src/CustomizableWidgets.jsm index 025d4dbfa4c..5ba81b77b5c 100644 --- a/browser/components/customizableui/src/CustomizableWidgets.jsm +++ b/browser/components/customizableui/src/CustomizableWidgets.jsm @@ -61,7 +61,6 @@ const CustomizableWidgets = [{ type: "view", viewId: "PanelUI-history", shortcutId: "key_gotoHistory", - removable: true, defaultArea: CustomizableUI.AREA_PANEL, onViewShowing: function(aEvent) { // Populate our list of history @@ -148,7 +147,6 @@ const CustomizableWidgets = [{ } }, { id: "privatebrowsing-button", - removable: true, shortcutId: "key_privatebrowsing", defaultArea: CustomizableUI.AREA_PANEL, onCommand: function(e) { @@ -161,7 +159,6 @@ const CustomizableWidgets = [{ } }, { id: "save-page-button", - removable: true, shortcutId: "key_savePage", defaultArea: CustomizableUI.AREA_PANEL, onCommand: function(aEvent) { @@ -174,7 +171,6 @@ const CustomizableWidgets = [{ } }, { id: "find-button", - removable: true, shortcutId: "key_find", defaultArea: CustomizableUI.AREA_PANEL, onCommand: function(aEvent) { @@ -187,7 +183,6 @@ const CustomizableWidgets = [{ } }, { id: "open-file-button", - removable: true, shortcutId: "openFileKb", defaultArea: CustomizableUI.AREA_PANEL, onCommand: function(aEvent) { @@ -202,7 +197,6 @@ const CustomizableWidgets = [{ id: "developer-button", type: "view", viewId: "PanelUI-developer", - removable: true, shortcutId: "key_devToolboxMenuItem", defaultArea: CustomizableUI.AREA_PANEL, onViewShowing: function(aEvent) { @@ -265,7 +259,6 @@ const CustomizableWidgets = [{ } }, { id: "add-ons-button", - removable: true, shortcutId: "key_openAddons", defaultArea: CustomizableUI.AREA_PANEL, onCommand: function(aEvent) { @@ -278,7 +271,6 @@ const CustomizableWidgets = [{ } }, { id: "preferences-button", - removable: true, defaultArea: CustomizableUI.AREA_PANEL, #ifdef XP_WIN label: "preferences-button.labelWin", @@ -295,7 +287,6 @@ const CustomizableWidgets = [{ }, { id: "zoom-controls", type: "custom", - removable: true, defaultArea: CustomizableUI.AREA_PANEL, onBuild: function(aDocument) { const kPanelId = "PanelUI-popup"; @@ -441,7 +432,6 @@ const CustomizableWidgets = [{ }, { id: "edit-controls", type: "custom", - removable: true, defaultArea: CustomizableUI.AREA_PANEL, onBuild: function(aDocument) { let inPanel = (this.currentArea == CustomizableUI.AREA_PANEL); @@ -536,7 +526,6 @@ const CustomizableWidgets = [{ id: "feed-button", type: "view", viewId: "PanelUI-feeds", - removable: true, defaultArea: CustomizableUI.AREA_PANEL, onClick: function(aEvent) { let win = aEvent.target.ownerDocument.defaultView; @@ -576,7 +565,6 @@ const CustomizableWidgets = [{ id: "characterencoding-button", type: "view", viewId: "PanelUI-characterEncodingView", - removable: true, defaultArea: CustomizableUI.AREA_PANEL, maybeDisableMenu: function(aDocument) { let window = aDocument.defaultView; @@ -783,7 +771,6 @@ const CustomizableWidgets = [{ } }, { id: "email-link-button", - removable: true, onCommand: function(aEvent) { let win = aEvent.view; win.MailIntegration.sendLinkForWindow(win.content); @@ -801,7 +788,6 @@ if (Services.sysinfo.getProperty("hasWindowsTouchInterface")) { id: "switch-to-metro-button", label: "switch-to-metro-button2.label", tooltiptext: metroTooltip, - removable: true, defaultArea: CustomizableUI.AREA_PANEL, showInPrivateBrowsing: false, /* See bug 928068 */ onCommand: function(aEvent) { diff --git a/browser/components/customizableui/test/browser.ini b/browser/components/customizableui/test/browser.ini index 15bb529953f..38ccb7a93a1 100644 --- a/browser/components/customizableui/test/browser.ini +++ b/browser/components/customizableui/test/browser.ini @@ -45,4 +45,5 @@ skip-if = os == "mac" [browser_942581_unregisterArea_keeps_placements.js] [browser_943683_migration_test.js] [browser_944887_destroyWidget_should_destroy_in_palette.js] +[browser_947987_removable_default.js] [browser_panel_toggle.js] diff --git a/browser/components/customizableui/test/browser_938995_indefaultstate_nonremovable.js b/browser/components/customizableui/test/browser_938995_indefaultstate_nonremovable.js index a8d760c91b4..c2f4dd3829d 100644 --- a/browser/components/customizableui/test/browser_938995_indefaultstate_nonremovable.js +++ b/browser/components/customizableui/test/browser_938995_indefaultstate_nonremovable.js @@ -10,16 +10,17 @@ let gTests = [ let navbar = document.getElementById("nav-bar"); ok(CustomizableUI.inDefaultState, "Should start in default state"); - CustomizableUI.createWidget({id: kWidgetId, removable: false, label: "Test"}); + let button = createDummyXULButton(kWidgetId, "Test non-removable inDefaultState handling"); CustomizableUI.addWidgetToArea(kWidgetId, CustomizableUI.AREA_NAVBAR); + button.setAttribute("removable", "false"); ok(CustomizableUI.inDefaultState, "Should still be in default state after navbar addition"); - CustomizableUI.destroyWidget(kWidgetId); + button.remove(); - CustomizableUI.createWidget({id: kWidgetId, removable: false, label: "Test"}); + button = createDummyXULButton(kWidgetId, "Test non-removable inDefaultState handling"); CustomizableUI.addWidgetToArea(kWidgetId, CustomizableUI.AREA_PANEL); + button.setAttribute("removable", "false"); ok(CustomizableUI.inDefaultState, "Should still be in default state after panel addition"); - CustomizableUI.destroyWidget(kWidgetId); - + button.remove(); ok(CustomizableUI.inDefaultState, "Should be in default state after destroying both widgets"); }, teardown: null diff --git a/browser/components/customizableui/test/browser_947987_removable_default.js b/browser/components/customizableui/test/browser_947987_removable_default.js new file mode 100644 index 00000000000..54b072904af --- /dev/null +++ b/browser/components/customizableui/test/browser_947987_removable_default.js @@ -0,0 +1,79 @@ +/* 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/. */ + +let kWidgetId = "test-removable-widget-default"; +const kNavBar = CustomizableUI.AREA_NAVBAR; +let widgetCounter = 0; +let gTests = [ + { + desc: "Sanity checks", + run: function() { + let brokenSpec = {id: kWidgetId + (widgetCounter++), removable: false}; + SimpleTest.doesThrow(function() CustomizableUI.createWidget(brokenSpec), + "Creating non-removable widget without defaultArea should throw."); + + // Widget without removable set should be removable: + let wrapper = CustomizableUI.createWidget({id: kWidgetId + (widgetCounter++)}); + ok(CustomizableUI.isWidgetRemovable(wrapper.id), "Should be removable by default."); + CustomizableUI.destroyWidget(wrapper.id); + } + }, + { + desc: "Test non-removable widget with defaultArea", + run: function() { + // Non-removable widget with defaultArea should work: + let spec = {id: kWidgetId + (widgetCounter++), removable: false, + defaultArea: kNavBar}; + let widgetWrapper; + try { + widgetWrapper = CustomizableUI.createWidget(spec); + } catch (ex) { + ok(false, "Creating a non-removable widget with a default area should not throw."); + return; + } + + let placement = CustomizableUI.getPlacementOfWidget(spec.id); + ok(placement, "Widget should be placed."); + is(placement.area, kNavBar, "Widget should be in navbar"); + let singleWrapper = widgetWrapper.forWindow(window); + ok(singleWrapper, "Widget should exist in window."); + ok(singleWrapper.node, "Widget node should exist in window."); + let expectedParent = CustomizableUI.getCustomizeTargetForArea(kNavBar, window); + is(singleWrapper.node.parentNode, expectedParent, "Widget should be in navbar."); + + let otherWin = yield openAndLoadWindow(true); + placement = CustomizableUI.getPlacementOfWidget(spec.id); + ok(placement, "Widget should be placed."); + is(placement && placement.area, kNavBar, "Widget should be in navbar"); + + singleWrapper = widgetWrapper.forWindow(otherWin); + ok(singleWrapper, "Widget should exist in other window."); + if (singleWrapper) { + ok(singleWrapper.node, "Widget node should exist in other window."); + if (singleWrapper.node) { + let expectedParent = CustomizableUI.getCustomizeTargetForArea(kNavBar, otherWin); + is(singleWrapper.node.parentNode, expectedParent, + "Widget should be in navbar in other window."); + } + } + otherWin.close(); + } + }, +]; + +function asyncCleanup() { + yield resetCustomization(); +} + +function cleanup() { + removeCustomToolbars(); +} + +function test() { + waitForExplicitFinish(); + registerCleanupFunction(cleanup); + runTests(gTests, asyncCleanup); +} + +