Bug 1258344 - fix customizemode re-entering issues when switching tabs, r=jaws
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 22 Mar 2016 14:32:32 +0000
changeset 291461 7d3eb3e1e13536cf2bc0814727348097f2352b39
parent 291460 80bcaeaf909743f1500db6423d3de82f1e10f42b
child 291462 43f2fc45eaf759727dfed6893a8c2efc8c35ddf9
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1258344
milestone48.0a1
Bug 1258344 - fix customizemode re-entering issues when switching tabs, r=jaws MozReview-Commit-ID: 3KONzSxf8g9
browser/components/customizableui/CustomizeMode.jsm
browser/components/customizableui/test/browser.ini
browser/components/customizableui/test/browser_switch_to_customize_mode.js
browser/components/customizableui/test/head.js
--- a/browser/components/customizableui/CustomizeMode.jsm
+++ b/browser/components/customizableui/CustomizeMode.jsm
@@ -60,16 +60,17 @@ var gDraggingInToolbars;
 var gTab;
 
 function closeGlobalTab() {
   let win = gTab.ownerGlobal;
   if (win.gBrowser.browsers.length == 1) {
     win.BrowserOpenTab();
   }
   win.gBrowser.removeTab(gTab);
+  gTab = null;
 }
 
 function unregisterGlobalTab() {
   gTab.removeEventListener("TabClose", unregisterGlobalTab);
   gTab.ownerGlobal.removeEventListener("unload", unregisterGlobalTab);
   gTab.removeAttribute("customizemode");
   gTab = null;
 }
@@ -205,17 +206,20 @@ CustomizeMode.prototype = {
     if (!gTab) {
       this.setTab(this.browser.loadOneTab("about:blank",
                                           { inBackground: false,
                                             forceNotRemote: true,
                                             skipAnimation: true }));
       return;
     }
     if (!gTab.selected) {
+      // This will force another .enter() to be called via the
+      // onlocationchange handler of the tabbrowser, so we return early.
       gTab.ownerGlobal.gBrowser.selectedTab = gTab;
+      return;
     }
     gTab.ownerGlobal.focus();
     if (gTab.ownerDocument != this.document) {
       return;
     }
 
     let window = this.window;
     let document = this.document;
--- a/browser/components/customizableui/test/browser.ini
+++ b/browser/components/customizableui/test/browser.ini
@@ -142,8 +142,9 @@ tags = fullscreen
 skip-if = os == "mac"
 [browser_1087303_button_preferences.js]
 [browser_1089591_still_customizable_after_reset.js]
 [browser_1096763_seen_widgets_post_reset.js]
 [browser_1161838_inserted_new_default_buttons.js]
 [browser_bootstrapped_custom_toolbar.js]
 [browser_customizemode_contextmenu_menubuttonstate.js]
 [browser_panel_toggle.js]
+[browser_switch_to_customize_mode.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/customizableui/test/browser_switch_to_customize_mode.js
@@ -0,0 +1,34 @@
+"use strict";
+
+add_task(function*() {
+  yield startCustomizing();
+  is(gBrowser.tabs.length, 2, "Should have 2 tabs");
+
+  let paletteKidCount = document.getElementById("customization-palette").childElementCount;
+  let nonCustomizingTab = gBrowser.tabContainer.querySelector("tab:not([customizemode=true])");
+  let finishedCustomizing = BrowserTestUtils.waitForEvent(gNavToolbox, "aftercustomization");
+  yield BrowserTestUtils.switchTab(gBrowser, nonCustomizingTab);
+  yield finishedCustomizing;
+
+  let startedCount = 0;
+  let handler = e => startedCount++;
+  gNavToolbox.addEventListener("customizationstarting", handler);
+  yield startCustomizing();
+  CustomizableUI.removeWidgetFromArea("home-button");
+  yield gCustomizeMode.reset().catch(e => {
+    ok(false, "Threw an exception trying to reset after making modifications in customize mode: " + e);
+  });
+
+  let newKidCount = document.getElementById("customization-palette").childElementCount;
+  is(newKidCount, paletteKidCount, "Should have just as many items in the palette as before.");
+  yield endCustomizing();
+  is(startedCount, 1, "Should have only started once");
+  gNavToolbox.removeEventListener("customizationstarting", handler);
+  let customizableToolbars = document.querySelectorAll("toolbar[customizable=true]:not([autohide=true])");
+  for (let toolbar of customizableToolbars) {
+    ok(!toolbar.hasAttribute("customizing"), "Toolbar " + toolbar.id + " is no longer customizing");
+  }
+  let menuitem = document.getElementById("PanelUI-customize");
+  isnot(menuitem.getAttribute("label"), menuitem.getAttribute("exitLabel"), "Should have exited successfully");
+});
+
--- a/browser/components/customizableui/test/head.js
+++ b/browser/components/customizableui/test/head.js
@@ -186,37 +186,17 @@ function endCustomizing(aWindow=window) 
   function onCustomizationEnds() {
     Services.prefs.setBoolPref("browser.uiCustomization.disableAnimation", false);
     aWindow.gNavToolbox.removeEventListener("aftercustomization", onCustomizationEnds);
     deferredEndCustomizing.resolve();
   }
   aWindow.gNavToolbox.addEventListener("aftercustomization", onCustomizationEnds);
   aWindow.gCustomizeMode.exit();
 
-  return deferredEndCustomizing.promise.then(function() {
-    let deferredLoadNewTab = Promise.defer();
-
-    //XXXgijs so some tests depend on this tab being about:blank. Make it so.
-    let newTabBrowser = aWindow.gBrowser.selectedBrowser;
-    newTabBrowser.stop();
-
-    // If we stop early enough, this might actually be about:blank.
-    if (newTabBrowser.currentURI.spec == "about:blank") {
-      return null;
-    }
-
-    // Otherwise, make it be about:blank, and wait for that to be done.
-    function onNewTabLoaded(e) {
-      newTabBrowser.removeEventListener("load", onNewTabLoaded, true);
-      deferredLoadNewTab.resolve();
-    }
-    newTabBrowser.addEventListener("load", onNewTabLoaded, true);
-    newTabBrowser.loadURI("about:blank");
-    return deferredLoadNewTab.promise;
-  });
+  return deferredEndCustomizing.promise;
 }
 
 function startCustomizing(aWindow=window) {
   if (aWindow.document.documentElement.getAttribute("customizing") == "true") {
     return null;
   }
   Services.prefs.setBoolPref("browser.uiCustomization.disableAnimation", true);
   let deferred = Promise.defer();