Bug 1437811 - Part 2 - Prepare the main view just before opening the panel. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Wed, 14 Feb 2018 15:16:40 +0000
changeset 755557 5ec65cf52fcae903c7faddc01c486abf9315d6d3
parent 755556 85191989c5b6925c4659c5afd8f7647e6950b49f
child 755558 4cf7059b78b6fdd752c6b479671c43898484aa5b
push id99192
push userpaolo.mozmail@amadzone.org
push dateThu, 15 Feb 2018 13:35:45 +0000
reviewersGijs
bugs1437811
milestone60.0a1
Bug 1437811 - Part 2 - Prepare the main view just before opening the panel. r=Gijs This allows the ViewShowing event for the main view to prevent the panel from opening. It also avoids setting up the main view if the panel is not opened. MozReview-Commit-ID: LK8tBcz6lkK
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/content/panelUI.js
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -221,23 +221,16 @@ this.PanelMultiView = class extends this
     return this.node && (this._viewShowing || this._currentSubView);
   }
   get _currentSubView() {
     // Peek the top of the stack, but fall back to the main view if the list of
     // opened views is currently empty.
     let panelView = this.openViews[this.openViews.length - 1];
     return (panelView && panelView.node) || this._mainView;
   }
-  /**
-   * @return {Promise} showSubView() returns a promise, which is kept here for
-   *                   random access.
-   */
-  get currentShowPromise() {
-    return this._currentShowPromise || Promise.resolve();
-  }
 
   constructor(node) {
     super(node);
     this._openPopupPromise = Promise.resolve(false);
     this._openPopupCancelCallback = () => {};
   }
 
   connect() {
@@ -266,27 +259,26 @@ this.PanelMultiView = class extends this
     this._panel.addEventListener("popupshowing", this);
     this._panel.addEventListener("popuppositioned", this);
     this._panel.addEventListener("popuphidden", this);
     this._panel.addEventListener("popupshown", this);
     let cs = window.getComputedStyle(document.documentElement);
     // Set CSS-determined attributes now to prevent a layout flush when we do
     // it when transitioning between panels.
     this._dir = cs.direction;
-    this.showMainView();
 
     // Proxy these public properties and methods, as used elsewhere by various
     // parts of the browser, to this instance.
     ["goBack", "showMainView", "showSubView"].forEach(method => {
       Object.defineProperty(this.node, method, {
         enumerable: true,
         value: (...args) => this[method](...args)
       });
     });
-    ["current", "currentShowPromise", "showingSubView"].forEach(property => {
+    ["current", "showingSubView"].forEach(property => {
       Object.defineProperty(this.node, property, {
         enumerable: true,
         get: () => this[property]
       });
     });
   }
 
   destructor() {
@@ -400,17 +392,20 @@ this.PanelMultiView = class extends this
           // The XBL binding must be connected at this point. If this is not the
           // case, the calling code should be updated to unhide the panel.
           if (!this.connected) {
             throw new Error("The binding for the panelmultiview element isn't" +
                             " connected. The containing panel may still have" +
                             " its display turned off by the hidden attribute.");
           }
         }
-        // (The rest of the asynchronous preparation goes here.)
+        // Allow any of the ViewShowing handlers to prevent showing the main view.
+        if (!(await this.showMainView())) {
+          cancelCallback();
+        }
       } catch (ex) {
         cancelCallback();
         throw ex;
       }
       // If a cancellation request was received there is nothing more to do.
       if (!canCancel || !this.node) {
         return false;
       }
@@ -485,17 +480,17 @@ this.PanelMultiView = class extends this
 
     let previous = this.openViews.pop().node;
     let current = this._currentSubView;
     this.showSubView(current, null, previous);
   }
 
   showMainView() {
     if (!this.node || !this._mainViewId)
-      return Promise.resolve();
+      return Promise.resolve(false);
 
     return this.showSubView(this._mainView);
   }
 
   /**
    * Ensures that all the panelviews, that are currently part of this instance,
    * are hidden, except one specifically.
    *
@@ -517,18 +512,18 @@ this.PanelMultiView = class extends this
 
     if (!this.openViews.includes(nextPanelView))
       this.openViews.push(nextPanelView);
 
     nextPanelView.current = true;
     this.showingSubView = nextPanelView.node.id != this._mainViewId;
   }
 
-  showSubView(aViewId, aAnchor, aPreviousView) {
-    this._currentShowPromise = (async () => {
+  async showSubView(aViewId, aAnchor, aPreviousView) {
+    try {
       // Support passing in the node directly.
       let viewNode = typeof aViewId == "string" ? this.node.querySelector("#" + aViewId) : aViewId;
       if (!viewNode) {
         viewNode = this.document.getElementById(aViewId);
         if (viewNode) {
           this._viewStack.appendChild(viewNode);
         } else {
           throw new Error(`Subview ${aViewId} doesn't exist!`);
@@ -589,31 +584,35 @@ this.PanelMultiView = class extends this
           } catch (e) {
             Cu.reportError(e);
             cancel = true;
           }
         }
 
         if (cancel) {
           this._viewShowing = null;
-          return;
+          return false;
         }
       }
 
       // Now we have to transition the panel. If we've got an older transition
       // still running, make sure to clean it up.
       await this._cleanupTransitionPhase();
       if (!showingSameView && this._panel.state == "open") {
         await this._transitionViews(previousViewNode, viewNode, reverse, aAnchor);
         nextPanelView.focusSelectedElement();
       } else {
         this.hideAllViewsExcept(nextPanelView);
       }
-    })().catch(e => Cu.reportError(e));
-    return this._currentShowPromise;
+
+      return true;
+    } catch (ex) {
+      Cu.reportError(ex);
+      return false;
+    }
   }
 
   /**
    * Apply a transition to 'slide' from the currently active view to the next
    * one.
    * Sliding the next subview in means that the previous panelview stays where it
    * is and the active panelview slides in from the left in LTR mode, right in
    * RTL mode.
@@ -926,20 +925,27 @@ this.PanelMultiView = class extends this
       case "popupshown":
         // Now that the main view is visible, we can check the height of the
         // description elements it contains.
         PanelView.forNode(this._mainView).descriptionHeightWorkaround();
         break;
       case "popuphidden": {
         // WebExtensions consumers can hide the popup from viewshowing, or
         // mid-transition, which disrupts our state:
+        if (this._viewShowing) {
+          PanelView.forNode(this._viewShowing).dispatchCustomEvent("ViewHiding");
+        }
         this._viewShowing = null;
         this._transitioning = false;
         this.node.removeAttribute("panelopen");
-        this.showMainView();
+        // Raise the ViewHiding event for the current view.
+        this.hideAllViewsExcept(null);
+        this._cleanupTransitionPhase();
+        if (this._autoResizeWorkaroundTimer)
+          this.window.clearTimeout(this._autoResizeWorkaroundTimer);
         this.window.removeEventListener("keydown", this);
         this._panel.removeEventListener("mousemove", this);
         this.openViews.forEach(panelView => panelView.clearNavigation());
         this.openViews = [];
 
         // Clear the main view size caches. The dimensions could be different
         // when the popup is opened again, e.g. through touch mode sizing.
         this._viewContainer.style.removeProperty("min-height");
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -417,69 +417,65 @@ const PanelUI = {
 
       // If the panelview is already selected in another PanelMultiView instance
       // as a subview, make sure to properly hide it there.
       let oldMultiView = viewNode.panelMultiView;
       if (oldMultiView && oldMultiView.current == viewNode) {
         await oldMultiView.showMainView();
       }
 
-      let viewShown = false;
-      let listener = () => viewShown = true;
-      viewNode.addEventListener("ViewShown", listener, {once: true});
-
       let multiView = document.createElement("panelmultiview");
       multiView.setAttribute("id", "customizationui-widget-multiview");
       multiView.setAttribute("viewCacheId", "appMenu-viewCache");
       multiView.setAttribute("mainViewId", viewNode.id);
       multiView.setAttribute("ephemeral", true);
       document.getElementById("appMenu-viewCache").appendChild(viewNode);
       tempPanel.appendChild(multiView);
       viewNode.classList.add("cui-widget-panelview");
 
+      let viewShown = false;
       let panelRemover = () => {
         viewNode.classList.remove("cui-widget-panelview");
         if (viewShown) {
           CustomizableUI.removePanelCloseListeners(tempPanel);
           tempPanel.removeEventListener("popuphidden", panelRemover);
         }
         aAnchor.open = false;
 
         // Ensure we run the destructor:
         multiView.instance.destructor();
 
         tempPanel.remove();
       };
 
-      // Wait until all the tasks needed to show a view are done.
-      await multiView.currentShowPromise;
-
-      if (!viewShown) {
-        viewNode.removeEventListener("ViewShown", listener);
-        panelRemover();
-        return;
-      }
-
-      CustomizableUI.addPanelCloseListeners(tempPanel);
-      tempPanel.addEventListener("popuphidden", panelRemover);
-
       if (aAnchor.parentNode.id == "PersonalToolbar") {
         tempPanel.classList.add("bookmarks-toolbar");
       }
 
       let anchor = this._getPanelAnchor(aAnchor);
 
       if (aAnchor != anchor && aAnchor.id) {
         anchor.setAttribute("consumeanchor", aAnchor.id);
       }
 
-      PanelMultiView.openPopup(tempPanel, anchor, {
-        position: "bottomcenter topright",
-        triggerEvent: domEvent,
-      }).catch(Cu.reportError);
+      try {
+        viewShown = await PanelMultiView.openPopup(tempPanel, anchor, {
+          position: "bottomcenter topright",
+          triggerEvent: domEvent,
+        });
+      } catch (ex) {
+        Cu.reportError(ex);
+      }
+
+      if (viewShown) {
+        CustomizableUI.addPanelCloseListeners(tempPanel);
+        tempPanel.addEventListener("popuphidden", panelRemover);
+      } else {
+        panelRemover();
+      }
     }
   },
 
   /**
    * Sets up the event listener for when the Library view is shown.
    *
    * @param {panelview} viewNode The library view.
    */