Bug 1439358 - Part 3 - Always raise ViewShowing events and don't update the "current" property. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 19 Feb 2018 11:17:52 +0000
changeset 758479 81f2c22ef023eb2bcb1332ca554a54c3e66f2c80
parent 758478 7a34b33ba4ffbf7c56fe2dbaf5fc35c70c3b0cc7
child 758480 c481e7957c2372f8613b3cac9ecc61563d60c4e6
push id100065
push userpaolo.mozmail@amadzone.org
push dateThu, 22 Feb 2018 14:26:14 +0000
reviewersGijs
bugs1439358
milestone60.0a1
Bug 1439358 - Part 3 - Always raise ViewShowing events and don't update the "current" property. r=Gijs The ViewShowing event is now called earlier and unconditionally, since we don't set the "current" attribute and call showMainView while the panel is closing anymore. It is already the case that the ViewShowing event handlers don't depend on the "current" property, so we don't need to keep track of it before ViewShown events are dispatched. MozReview-Commit-ID: Ii4SN03KjwW
browser/components/customizableui/PanelMultiView.jsm
browser/components/places/content/browserPlacesViews.js
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -282,17 +282,17 @@ this.PanelMultiView = class extends this
                                     .getService(Ci.nsIScreenManager);
   }
   /**
    * @return {panelview} the currently visible subview OR the subview that is
    *                     about to be shown whilst a 'ViewShowing' event is being
    *                     dispatched.
    */
   get current() {
-    return this.node && (this._viewShowing || this._currentSubView);
+    return this.node && 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;
   }
 
@@ -592,18 +592,16 @@ this.PanelMultiView = class extends this
   hideAllViewsExcept(nextPanelView = null) {
     for (let panelView of this.knownViews) {
       // When the panelview was already reparented, don't interfere any more.
       if (panelView == nextPanelView || !this.node || panelView.node.panelMultiView != this.node)
         continue;
       panelView.current = false;
     }
 
-    this._viewShowing = null;
-
     if (!this.node || !nextPanelView)
       return;
 
     if (!this.openViews.includes(nextPanelView))
       this.openViews.push(nextPanelView);
 
     nextPanelView.current = true;
     this.showingSubView = nextPanelView.node.id != this._mainViewId;
@@ -617,25 +615,30 @@ this.PanelMultiView = class extends this
       viewNode.panelMultiView = this.node;
 
       let previousViewNode = previousView || this._currentSubView;
 
       if (viewNode.parentNode != this._viewStack) {
         this._viewStack.appendChild(viewNode);
       }
 
+      // Emit the ViewShowing event so that the widget definition has a chance
+      // to lazily populate the subview with things or perhaps even cancel this
+      // whole operation.
+      if (await nextPanelView.dispatchAsyncEvent("ViewShowing")) {
+        return false;
+      }
+
       // If the panelview to show is the same as the previous one, the 'ViewShowing'
       // event has already been dispatched. Don't do it twice.
       let showingSameView = viewNode == previousViewNode;
 
       let prevPanelView = PanelView.forNode(previousViewNode);
       prevPanelView.captureKnownSize();
 
-      this._viewShowing = viewNode;
-
       let reverse = !!previousView;
       if (!reverse) {
         // We are opening a new view, either because we are navigating forward
         // or because we are showing the main view. Some properties of the view
         // may vary between panels, so we make sure to update them every time.
         // Firstly, make sure that the header matches how the view was opened.
         nextPanelView.headerText = viewNode.getAttribute("title") ||
                                    (anchor && anchor.getAttribute("label"));
@@ -645,26 +648,16 @@ this.PanelMultiView = class extends this
         // The constrained width of subviews may also vary between panels.
         nextPanelView.minMaxWidth = isMainView ? 0 : prevPanelView.knownWidth;
       }
 
       if (anchor) {
         viewNode.classList.add("PanelUI-subView");
       }
 
-      if (!showingSameView || !viewNode.hasAttribute("current")) {
-        // Emit the ViewShowing event so that the widget definition has a chance
-        // to lazily populate the subview with things or perhaps even cancel this
-        // whole operation.
-        if (await nextPanelView.dispatchAsyncEvent("ViewShowing")) {
-          this._viewShowing = null;
-          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, anchor);
         nextPanelView.focusSelectedElement();
       } else {
         this.hideAllViewsExcept(nextPanelView);
@@ -992,17 +985,16 @@ 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:
-        this._viewShowing = null;
         this._transitioning = false;
         this.node.removeAttribute("panelopen");
         // Raise the ViewHiding event for the current view.
         this._cleanupTransitionPhase();
         this.hideAllViewsExcept(null);
         this.window.removeEventListener("keydown", this);
         this._panel.removeEventListener("mousemove", this);
         this.openViews.forEach(panelView => panelView.clearNavigation());
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -2250,20 +2250,16 @@ this.PlacesPanelview = class extends Pla
     } else {
       panelview.removeAttribute("emptyplacesresult");
       try {
         panelview.removeChild(panelview._emptyMenuitem);
       } catch (ex) {}
     }
   }
 
-  _isPopupOpen() {
-    return this.panel.state == "open" && this.panelMultiView.current == this._viewElt;
-  }
-
   _onPopupHidden(event) {
     let panelview = event.originalTarget;
     let placesNode = panelview._placesNode;
     // Avoid handling ViewHiding of inner views
     if (placesNode && PlacesUIUtils.getViewForNode(panelview) == this) {
       // UI performance: folder queries are cheap, keep the resultnode open
       // so we don't rebuild its contents whenever the popup is reopened.
       // Though, we want to always close feed containers so their expiration