Bug 1399490 - fix unhiding the downloads button in customize mode if it was moved to the palette, r?nhnt11 draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 13 Sep 2017 16:24:13 +0100
changeset 664858 bad6145b4b01e500f82161ddbad4f02be2d1ccb0
parent 664736 dd6b788f149763c4014c27f2fe1a1d13228bda82
child 731563 6674c72b08b9b55611603f220ed119e24e8b798d
push id79825
push usergijskruitbosch@gmail.com
push dateThu, 14 Sep 2017 13:41:34 +0000
reviewersnhnt11
bugs1399490
milestone57.0a1
Bug 1399490 - fix unhiding the downloads button in customize mode if it was moved to the palette, r?nhnt11 MozReview-Commit-ID: 7Q8yPzBVxim
browser/components/downloads/content/indicator.js
browser/components/downloads/test/browser/browser_downloads_autohide.js
--- a/browser/components/downloads/content/indicator.js
+++ b/browser/components/downloads/content/indicator.js
@@ -125,18 +125,33 @@ const DownloadsButton = {
   /**
    * Allows the temporary anchor to be hidden.
    */
   releaseAnchor() {
     this._anchorRequested = false;
     this._getAnchorInternal();
   },
 
-  unhide() {
+  /**
+   * Unhide the button. Generally, this only needs to use the placeholder.
+   * However, when starting customize mode, if the button is in the palette,
+   * we need to unhide it before customize mode is entered, otherwise it
+   * gets ignored by customize mode. To do this, we pass true for
+   * `includePalette`. We don't always look in the palette because it's
+   * inefficient (compared to getElementById), shouldn't be necessary, and
+   * if _placeholder returned the node even if in the palette, other checks
+   * would break.
+   *
+   * @param includePalette  whether to search the palette, too. Defaults to false.
+   */
+  unhide(includePalette = false) {
     let button = this._placeholder;
+    if (!button && includePalette) {
+      button = gNavToolbox.palette.querySelector("#downloads-button");
+    }
     if (button && button.hasAttribute("hidden")) {
       button.removeAttribute("hidden");
       if (this._navBar.contains(button)) {
         this._navBar.setAttribute("downloadsbuttonshown", "true");
       }
     }
   },
 
@@ -185,17 +200,17 @@ const DownloadsButton = {
    * moved freely between the toolbars and the customization palette.
    */
   onCustomizeStart(win) {
     if (win == window) {
       // Prevent the indicator from being displayed as a temporary anchor
       // during customization, even if requested using the getAnchor method.
       this._customizing = true;
       this._anchorRequested = false;
-      this.unhide();
+      this.unhide(true);
     }
   },
 
   onCustomizeEnd(win) {
     if (win == window) {
       this._customizing = false;
       this.checkForAutoHide();
       DownloadsIndicatorView.afterCustomize();
@@ -303,17 +318,19 @@ const DownloadsIndicatorView = {
 
     DownloadsOverlayLoader.ensureOverlayLoaded(
       DownloadsButton.kIndicatorOverlay,
       () => {
         this._operational = true;
 
         // If the view is initialized, we need to update the elements now that
         // they are finally available in the document.
-        if (this._initialized) {
+        // We need to re-check for the placeholder because it might have
+        // disappeared since then.
+        if (this._initialized && DownloadsButton._placeholder) {
           DownloadsCommon.getIndicatorData(window).refreshView(this);
         }
 
         if (aCallback) {
           aCallback();
         }
       });
   },
--- a/browser/components/downloads/test/browser/browser_downloads_autohide.js
+++ b/browser/components/downloads/test/browser/browser_downloads_autohide.js
@@ -246,16 +246,51 @@ add_task(async function checkStateForDow
   downloads = await publicList.getAll();
   for (let download of downloads) {
     publicList.remove(download);
   }
 
   ok(!downloadsButton.hasAttribute("hidden"),
      "Button should still not be hidden in the panel " +
      "when downloads count reaches 0 after being non-0.");
+
+  CustomizableUI.reset();
+});
+
+/**
+ * Check that if the button is moved to the palette, we unhide it
+ * in customize mode even if it was always hidden. We use a new
+ * window to test this.
+ */
+add_task(async function checkStateWhenHiddenInPalette() {
+  ok(Services.prefs.getBoolPref(kDownloadAutoHidePref),
+     "Pref should be causing us to autohide");
+  gCustomizeMode.removeFromArea(document.getElementById("downloads-button"));
+  // In a new window, the button will have been hidden
+  let otherWin = await BrowserTestUtils.openNewBrowserWindow();
+  ok(!otherWin.document.getElementById("downloads-button"),
+     "Button shouldn't be visible in the window");
+
+  let paletteButton = otherWin.gNavToolbox.palette.querySelector("#downloads-button");
+  ok(paletteButton, "Button should exist in the palette");
+  if (paletteButton) {
+    ok(paletteButton.hidden, "Button will still have the hidden attribute");
+    await promiseCustomizeStart(otherWin);
+    ok(!paletteButton.hidden,
+       "Button should no longer be hidden in customize mode");
+    ok(otherWin.document.getElementById("downloads-button"),
+       "Button should be in the document now.");
+    await promiseCustomizeEnd(otherWin);
+    // We purposefully don't assert anything about what happens next.
+    // It doesn't really matter if the button remains unhidden in
+    // the palette, and if we move it we'll unhide it then (the other
+    // tests check this).
+  }
+  await BrowserTestUtils.closeWindow(otherWin);
+  CustomizableUI.reset();
 });
 
 function promiseCustomizeStart(aWindow = window) {
   return new Promise(resolve => {
     aWindow.gNavToolbox.addEventListener("customizationready", resolve,
                                          {once: true});
     aWindow.gCustomizeMode.enter();
   });