Bug 1401991 - Ensure that we don't hide panelviews that are already reparented to another multi-view and ensure to hide other panels consistently. r?Gijs draft
authorMike de Boer <mdeboer@mozilla.com>
Mon, 25 Sep 2017 21:35:17 +0200
changeset 670084 8d36df3089db1300aeaa722fddd004544582dd32
parent 669742 5f3f19824efa14cc6db546baf59c54a0fc15ddc9
child 733121 ba62af06205c6ff4196ca840a1f3693f0231b4f2
push id81505
push usermdeboer@mozilla.com
push dateMon, 25 Sep 2017 19:40:59 +0000
reviewersGijs
bugs1401991
milestone58.0a1
Bug 1401991 - Ensure that we don't hide panelviews that are already reparented to another multi-view and ensure to hide other panels consistently. r?Gijs MozReview-Commit-ID: 5HpJKs1Ny5j
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -413,46 +413,46 @@ this.PanelMultiView = class {
         this._viewStack.insertBefore(aNewMainView, this._viewStack.firstChild);
       }
     } else {
       this._mainViewContainer.appendChild(aNewMainView);
     }
   }
 
   showMainView() {
-    if (this.showingSubView) {
-      let viewNode = this._currentSubView;
-      this._dispatchViewEvent(viewNode, "ViewHiding");
-      if (this.panelViews) {
-        viewNode.removeAttribute("current");
-        this.showSubView(this._mainViewId);
-      } else {
+    if (this.panelViews) {
+      this.showSubView(this._mainView);
+    } else {
+      if (this.showingSubView) {
+        let viewNode = this._currentSubView;
+        this._dispatchViewEvent(viewNode, "ViewHiding");
         this._transitionHeight(() => {
           viewNode.removeAttribute("current");
           this._currentSubView = null;
           this.node.setAttribute("viewtype", "main");
         });
       }
-    } else if (this.panelViews) {
-      // Make sure to hide all subviews, except for the mainView.
-      let mainView = this._mainView;
-      for (let panelview of this._panelViews) {
-        if (panelview == mainView)
-          panelview.setAttribute("current", true);
-        else
-          panelview.removeAttribute("current");
-      }
-      this.node.setAttribute("viewtype", "main");
-    }
 
-    if (!this.panelViews) {
       this._shiftMainView();
     }
   }
 
+  hideAllViewsButOne(theOne = this._mainView) {
+    for (let panelview of this._panelViews) {
+      // When the panelview was already reparented, don't interfere any more.
+      if (panelview.panelMultiView != this.node)
+        continue;
+      if (panelview == theOne)
+        panelview.setAttribute("current", true);
+      else
+        panelview.removeAttribute("current");
+    }
+    this.node.setAttribute("viewtype", (theOne.id == this._mainViewId) ? "main" : "subview");
+  }
+
   showSubView(aViewId, aAnchor, aPreviousView) {
     return (async () => {
       // 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._placeSubView(viewNode);
@@ -523,37 +523,30 @@ this.PanelMultiView = class {
       }
 
       this._viewShowing = null;
       if (cancel) {
         return;
       }
 
       this._currentSubView = viewNode;
+
+      // Now we have to transition the panel.
       if (this.panelViews) {
-        if (viewNode.id == this._mainViewId) {
-          this.node.setAttribute("viewtype", "main");
-        } else {
-          this.node.setAttribute("viewtype", "subview");
-        }
         // If we've got an older transition still running, make sure to clean it up.
         await this._cleanupTransitionPhase();
-        if (!playTransition) {
-          viewNode.setAttribute("current", true);
+        if (playTransition) {
+          await this._transitionViews(previousViewNode, viewNode, reverse, previousRect, aAnchor);
+        } else {
+          this.hideAllViewsButOne(viewNode);
           this.descriptionHeightWorkaround(viewNode);
         }
-      }
-
-      // Now we have to transition the panel.
-      if (this.panelViews && playTransition) {
-        await this._transitionViews(previousViewNode, viewNode, reverse, previousRect, aAnchor);
-
         this._dispatchViewEvent(viewNode, "ViewShown");
         this._updateKeyboardFocus(viewNode);
-      } else if (!this.panelViews) {
+      } else {
         this._transitionHeight(() => {
           viewNode.setAttribute("current", true);
           if (viewNode.id == this._mainViewId) {
             this.node.setAttribute("viewtype", "main");
           } else {
             this.node.setAttribute("viewtype", "subview");
           }
           // Now that the subview is visible, we can check the height of the
@@ -711,17 +704,17 @@ this.PanelMultiView = class {
       return;
 
     let {phase, previousViewNode, viewNode, reverse, resolve, listener, anchor} = this._transitionDetails;
     this._transitionDetails = null;
 
     // Do the things we _always_ need to do whenever the transition ends or is
     // interrupted.
     this._dispatchViewEvent(previousViewNode, "ViewHiding");
-    previousViewNode.removeAttribute("current");
+    this.hideAllViewsButOne(viewNode);
     if (reverse)
       this._resetKeyNavigation(previousViewNode);
     this.descriptionHeightWorkaround(viewNode);
 
     if (anchor)
       anchor.removeAttribute("open");
 
     if (phase >= TRANSITION_PHASES.START) {