Bug 1382243 - Refactor panelmultiview code to reduce complexity and length of functions, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 04 Aug 2017 11:33:55 +0200
changeset 642612 944fba660dbf8ddf5183db27e944fbce4d3ba1ee
parent 642518 a921bfb8a2cf3db4d9edebe9b35799a3f9d035da
child 725059 72b64d17c61e7af46cf78a9e7e52c32cd61eadb6
push id72824
push usergijskruitbosch@gmail.com
push dateTue, 08 Aug 2017 15:49:41 +0000
reviewersmikedeboer
bugs1382243
milestone57.0a1
Bug 1382243 - Refactor panelmultiview code to reduce complexity and length of functions, r?mikedeboer While this creates several methods with just 1 callsite, given the previous length of the showSubView method (230 lines), it seems wise to split this up and to try to reduce the nesting, as well as making it easier to reason about what happens if we hide the panel mid-transition. MozReview-Commit-ID: 9Vf4p4fVBSs
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -425,236 +425,260 @@ this.PanelMultiView = class {
       }
     }
 
     if (!this.panelViews) {
       this._shiftMainView();
     }
   }
 
+  /**
+   * Get a node object given either that node object or a view id.
+   * Ensure the resulting panelview is present in the panelmultiview.
+   */
+  _getAndPlaceViewNode(viewId) {
+    // Support passing in the node directly.
+    let viewNode = typeof viewId == "string" ? this.node.querySelector("#" + viewId) : viewId;
+    if (!viewNode) {
+      viewNode = this.document.getElementById(viewId);
+      if (viewNode) {
+        this._placeSubView(viewNode);
+      } else {
+        throw new Error(`Subview ${viewId} doesn't exist!`);
+      }
+    } else if (viewNode.parentNode == this._panelViewCache) {
+      this._placeSubView(viewNode);
+    }
+    return viewNode;
+  }
+
+  _getPreviousRectAndPanelBounds(viewNode, previousViewNode) {
+    let dwu = this._dwu;
+    let previousRect = previousViewNode.__lastKnownBoundingRect =
+      dwu.getBoundsWithoutFlushing(previousViewNode);
+    if (this.panelViews) {
+      // Here go the measures that have the same caching lifetime as the width
+      // of the main view, i.e. 'forever', during the instance lifetime.
+      if (!this._mainViewWidth) {
+        this._mainViewWidth = previousRect.width;
+        let top = dwu.getBoundsWithoutFlushing(previousViewNode.firstChild || previousViewNode).top;
+        let bottom = dwu.getBoundsWithoutFlushing(previousViewNode.lastChild || previousViewNode).bottom;
+        this._viewVerticalPadding = previousRect.height - (bottom - top);
+      }
+      // Here go the measures that have the same caching lifetime as the height
+      // of the main view, i.e. whilst the panel is shown and/ or visible.
+      if (!this._mainViewHeight) {
+        this._mainViewHeight = previousRect.height;
+        this._viewContainer.style.minHeight = this._mainViewHeight + "px";
+      }
+      if (this._mainViewWidth)
+        viewNode.style.maxWidth = viewNode.style.minWidth = this._mainViewWidth + "px";
+    }
+    return previousRect;
+  }
+
+  async _dispatchViewShowing(viewNode, anchor) {
+    this._viewShowing = viewNode;
+
+    // Make sure that new panels always have a title set.
+    if (this.panelViews && anchor) {
+      if (!viewNode.hasAttribute("title"))
+        viewNode.setAttribute("title", anchor.getAttribute("label"));
+      viewNode.classList.add("PanelUI-subView");
+    }
+    // Emit the ViewShowing event so that the widget definition has a chance
+    // to lazily populate the subview with things.
+    let detail = {
+      blockers: new Set(),
+      addBlocker(promise) {
+        this.blockers.add(promise);
+      }
+    };
+    let cancel = this._dispatchViewEvent(viewNode, "ViewShowing", anchor, detail);
+    if (detail.blockers.size) {
+      try {
+        let results = await Promise.all(detail.blockers);
+        cancel = cancel || results.some(val => val === false);
+      } catch (e) {
+        Cu.reportError(e);
+        cancel = true;
+      }
+    }
+
+    this._viewShowing = null;
+    return !cancel;
+  }
+
+  _clearTransitionedStyles(view) {
+    view.style.removeProperty("border-inline-start");
+    view.style.removeProperty("transition");
+    view.style.removeProperty("transform");
+    view.style.removeProperty("width");
+    view.style.removeProperty("margin-inline-start");
+  }
+
+  // There are a few parts to transitioning the panel to a different subview:
+  //
+  // 1) The main view content gets shifted so that the center of the anchor
+  //    node is at the left-most edge of the panel.
+  // 2) The subview deck slides in so that it takes up almost all of the
+  //    panel.
+  // 3) If the subview is taller then the main panel contents, then the panel
+  //    must grow to meet that new height. Otherwise, it must shrink.
+  //
+  // All three of these actions make use of CSS transformations, so they
+  // should all occur simultaneously.
+  async _transitionToSubView(viewNode, previousViewNode, anchor, previousRect, reverse) {
+    const {window} = this;
+    // 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.
+    let onTransitionEnd = () => {
+      this._dispatchViewEvent(previousViewNode, "ViewHiding");
+      previousViewNode.removeAttribute("current");
+      this.descriptionHeightWorkaround(viewNode);
+    };
+
+    // There's absolutely no need to show off our epic animation skillz when
+    // the panel's not even open.
+    if (this._panel.state != "open") {
+      onTransitionEnd();
+      return;
+    }
+
+    if (anchor)
+      anchor.setAttribute("open", true);
+
+    // Set the viewContainer dimensions to make sure only the current view
+    // is visible.
+    this._viewContainer.style.height = Math.max(previousRect.height, this._mainViewHeight) + "px";
+    this._viewContainer.style.width = previousRect.width + "px";
+    // Lock the dimensions of the window that hosts the popup panel.
+    let rect = this._panel.popupBoxObject.getOuterScreenRect();
+    this._panel.setAttribute("width", rect.width);
+    this._panel.setAttribute("height", rect.height);
+
+    const viewRect = await this._viewBoundsOffscreen(viewNode, previousRect);
+
+    this._transitioning = true;
+    if (this._autoResizeWorkaroundTimer)
+      window.clearTimeout(this._autoResizeWorkaroundTimer);
+    this._viewContainer.setAttribute("transition-reverse", reverse);
+    let nodeToAnimate = reverse ? previousViewNode : viewNode;
+
+    if (!reverse) {
+      // We set the margin here to make sure the view is positioned next
+      // to the view that is currently visible. The animation is taken
+      // care of by transitioning the `transform: translateX()` property
+      // instead.
+      // Once the transition finished, we clean both properties up.
+      nodeToAnimate.style.marginInlineStart = previousRect.width + "px";
+    }
+
+    // Set the transition style and listen for its end to clean up and
+    // make sure the box sizing becomes dynamic again.
+    // Somehow, putting these properties in PanelUI.css doesn't work for
+    // newly shown nodes in a XUL parent node.
+    nodeToAnimate.style.transition = "transform ease-" + (reverse ? "in" : "out") +
+      " var(--panelui-subview-transition-duration)";
+    nodeToAnimate.style.willChange = "transform";
+    nodeToAnimate.style.borderInlineStart = "1px solid var(--panel-separator-color)";
+
+    // Wait until after the first paint to ensure setting 'current=true'
+    // has taken full effect; once both views are visible, we want to
+    // correctly measure rects using `dwu.getBoundsWithoutFlushing`.
+    if (!(await this._promiseEventOnceOrHidden(window, "MozAfterPaint")) ||
+        this._panel.state != "open") {
+      // Popup got hidden, bail.
+      onTransitionEnd();
+      return;
+    }
+    // Now set the viewContainer dimensions to that of the new view, which
+    // kicks of the height animation.
+    this._viewContainer.style.height = Math.max(viewRect.height, this._mainViewHeight) + "px";
+    this._viewContainer.style.width = viewRect.width + "px";
+    this._panel.removeAttribute("width");
+    this._panel.removeAttribute("height");
+
+    // The 'magic' part: build up the amount of pixels to move right or left.
+    let moveToLeft = (this._dir == "rtl" && !reverse) || (this._dir == "ltr" && reverse);
+    let movementX = reverse ? viewRect.width : previousRect.width;
+    let moveX = (moveToLeft ? "" : "-") + movementX;
+    nodeToAnimate.style.transform = "translateX(" + moveX + "px)";
+    // We're setting the width property to prevent flickering during the
+    // sliding animation with smaller views.
+    nodeToAnimate.style.width = viewRect.width + "px";
+
+    let transitionChecker = ev => {
+      // It's quite common that `height` on the view container doesn't need
+      // to transition, so we make sure to do all the work on the transform
+      // transition-end, because that is guaranteed to happen.
+      return ev.target != this._viewStack || ev.propertyName != "transform";
+    }
+    let didTransition = await this._promiseEventOnceOrHidden(this._viewContainer, "transitionend",
+                                                             transitionChecker);
+    onTransitionEnd();
+    this._transitioning = false;
+    this._resetKeyNavigation(previousViewNode);
+    if (!didTransition) {
+      return;
+    }
+
+    // Myeah, panel layout auto-resizing is a funky thing. We'll wait
+    // another few milliseconds to remove the width and height 'fixtures',
+    // to be sure we don't flicker annoyingly.
+    // NB: HACK! Bug 1363756 is there to fix this.
+    this._autoResizeWorkaroundTimer = window.setTimeout(() => {
+      this._viewContainer.style.removeProperty("height");
+      this._viewContainer.style.removeProperty("width");
+    }, 500);
+
+    // Take another breather, just like before, to wait for the 'current'
+    // attribute removal to take effect. This prevents a flicker.
+    // The cleanup we do doesn't affect the display anymore, so we're not
+    // too fussed about the timing here.
+    let popupHidden = !(await this._promiseEventOnceOrHidden(window, "MozAfterPaint"));
+    this._clearTransitionedStyles(nodeToAnimate);
+    if (anchor)
+      anchor.removeAttribute("open");
+
+    this._viewContainer.removeAttribute("transition-reverse");
+    if (!popupHidden) {
+      this._dispatchViewEvent(viewNode, "ViewShown");
+    }
+  }
+
   showSubView(aViewId, aAnchor, aPreviousView) {
-    const {document, window} = this;
     return (async () => {
-      // Support passing in the node directly.
-      let viewNode = typeof aViewId == "string" ? this.node.querySelector("#" + aViewId) : aViewId;
-      if (!viewNode) {
-        viewNode = document.getElementById(aViewId);
-        if (viewNode) {
-          this._placeSubView(viewNode);
-        } else {
-          throw new Error(`Subview ${aViewId} doesn't exist!`);
-        }
-      } else if (viewNode.parentNode == this._panelViewCache) {
-        this._placeSubView(viewNode);
-      }
+      let viewNode = this._getAndPlaceViewNode(aViewId);
 
-      let reverse = !!aPreviousView;
       let previousViewNode = aPreviousView || this._currentSubView;
       let playTransition = (!!previousViewNode && previousViewNode != viewNode);
 
-      let dwu, previousRect;
+      let previousRect;
       if (playTransition || this.panelViews) {
-        dwu = this._dwu;
-        previousRect = previousViewNode.__lastKnownBoundingRect =
-          dwu.getBoundsWithoutFlushing(previousViewNode);
-        if (this.panelViews) {
-          // Here go the measures that have the same caching lifetime as the width
-          // of the main view, i.e. 'forever', during the instance lifetime.
-          if (!this._mainViewWidth) {
-            this._mainViewWidth = previousRect.width;
-            let top = dwu.getBoundsWithoutFlushing(previousViewNode.firstChild || previousViewNode).top;
-            let bottom = dwu.getBoundsWithoutFlushing(previousViewNode.lastChild || previousViewNode).bottom;
-            this._viewVerticalPadding = previousRect.height - (bottom - top);
-          }
-          // Here go the measures that have the same caching lifetime as the height
-          // of the main view, i.e. whilst the panel is shown and/ or visible.
-          if (!this._mainViewHeight) {
-            this._mainViewHeight = previousRect.height;
-            this._viewContainer.style.minHeight = this._mainViewHeight + "px";
-          }
-        }
+        previousRect = this._getPreviousRectAndPanelBounds(viewNode, previousViewNode);
       }
 
-      this._viewShowing = viewNode;
-
-      // Make sure that new panels always have a title set.
-      if (this.panelViews && aAnchor) {
-        if (!viewNode.hasAttribute("title"))
-          viewNode.setAttribute("title", aAnchor.getAttribute("label"));
-        viewNode.classList.add("PanelUI-subView");
-      }
-      if (this.panelViews && this._mainViewWidth)
-        viewNode.style.maxWidth = viewNode.style.minWidth = this._mainViewWidth + "px";
-
-      // Emit the ViewShowing event so that the widget definition has a chance
-      // to lazily populate the subview with things.
-      let detail = {
-        blockers: new Set(),
-        addBlocker(promise) {
-          this.blockers.add(promise);
-        }
-      };
-      let cancel = this._dispatchViewEvent(viewNode, "ViewShowing", aAnchor, detail);
-      if (detail.blockers.size) {
-        try {
-          let results = await Promise.all(detail.blockers);
-          cancel = cancel || results.some(val => val === false);
-        } catch (e) {
-          Cu.reportError(e);
-          cancel = true;
-        }
-      }
-
-      this._viewShowing = null;
-      if (cancel) {
+      if (!await this._dispatchViewShowing(viewNode, aAnchor)) {
         return;
       }
 
       this._currentSubView = viewNode;
       viewNode.setAttribute("current", true);
+
       if (this.panelViews) {
         this.node.setAttribute("viewtype", "subview");
-        if (!playTransition)
+        if (!playTransition) {
           this.descriptionHeightWorkaround(viewNode);
-      }
-
-      // Now we have to transition the panel. There are a few parts to this:
-      //
-      // 1) The main view content gets shifted so that the center of the anchor
-      //    node is at the left-most edge of the panel.
-      // 2) The subview deck slides in so that it takes up almost all of the
-      //    panel.
-      // 3) If the subview is taller then the main panel contents, then the panel
-      //    must grow to meet that new height. Otherwise, it must shrink.
-      //
-      // All three of these actions make use of CSS transformations, so they
-      // should all occur simultaneously.
-      if (this.panelViews && playTransition) {
-        // 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.
-        let onTransitionEnd = () => {
-          this._dispatchViewEvent(previousViewNode, "ViewHiding");
-          previousViewNode.removeAttribute("current");
-          this.descriptionHeightWorkaround(viewNode);
-        };
-
-        // There's absolutely no need to show off our epic animation skillz when
-        // the panel's not even open.
-        if (this._panel.state != "open") {
-          onTransitionEnd();
-          return;
+        } else {
+          let reverse = !!aPreviousView;
+          this._transitionToSubView(viewNode, previousViewNode, aAnchor, previousRect, reverse);
         }
-
-        if (aAnchor)
-          aAnchor.setAttribute("open", true);
-
-        // Set the viewContainer dimensions to make sure only the current view
-        // is visible.
-        this._viewContainer.style.height = Math.max(previousRect.height, this._mainViewHeight) + "px";
-        this._viewContainer.style.width = previousRect.width + "px";
-        // Lock the dimensions of the window that hosts the popup panel.
-        let rect = this._panel.popupBoxObject.getOuterScreenRect();
-        this._panel.setAttribute("width", rect.width);
-        this._panel.setAttribute("height", rect.height);
-
-        this._viewBoundsOffscreen(viewNode, previousRect, viewRect => {
-          this._transitioning = true;
-          if (this._autoResizeWorkaroundTimer)
-            window.clearTimeout(this._autoResizeWorkaroundTimer);
-          this._viewContainer.setAttribute("transition-reverse", reverse);
-          let nodeToAnimate = reverse ? previousViewNode : viewNode;
-
-          if (!reverse) {
-            // We set the margin here to make sure the view is positioned next
-            // to the view that is currently visible. The animation is taken
-            // care of by transitioning the `transform: translateX()` property
-            // instead.
-            // Once the transition finished, we clean both properties up.
-            nodeToAnimate.style.marginInlineStart = previousRect.width + "px";
-          }
-
-          // Set the transition style and listen for its end to clean up and
-          // make sure the box sizing becomes dynamic again.
-          // Somehow, putting these properties in PanelUI.css doesn't work for
-          // newly shown nodes in a XUL parent node.
-          nodeToAnimate.style.transition = "transform ease-" + (reverse ? "in" : "out") +
-            " var(--panelui-subview-transition-duration)";
-          nodeToAnimate.style.willChange = "transform";
-          nodeToAnimate.style.borderInlineStart = "1px solid var(--panel-separator-color)";
-
-          // Wait until after the first paint to ensure setting 'current=true'
-          // has taken full effect; once both views are visible, we want to
-          // correctly measure rects using `dwu.getBoundsWithoutFlushing`.
-          window.addEventListener("MozAfterPaint", () => {
-            if (this._panel.state != "open") {
-              onTransitionEnd();
-              return;
-            }
-            // Now set the viewContainer dimensions to that of the new view, which
-            // kicks of the height animation.
-            this._viewContainer.style.height = Math.max(viewRect.height, this._mainViewHeight) + "px";
-            this._viewContainer.style.width = viewRect.width + "px";
-            this._panel.removeAttribute("width");
-            this._panel.removeAttribute("height");
-
-            // The 'magic' part: build up the amount of pixels to move right or left.
-            let moveToLeft = (this._dir == "rtl" && !reverse) || (this._dir == "ltr" && reverse);
-            let movementX = reverse ? viewRect.width : previousRect.width;
-            let moveX = (moveToLeft ? "" : "-") + movementX;
-            nodeToAnimate.style.transform = "translateX(" + moveX + "px)";
-            // We're setting the width property to prevent flickering during the
-            // sliding animation with smaller views.
-            nodeToAnimate.style.width = viewRect.width + "px";
-
-            this._viewContainer.addEventListener("transitionend", this._transitionEndListener = ev => {
-              // It's quite common that `height` on the view container doesn't need
-              // to transition, so we make sure to do all the work on the transform
-              // transition-end, because that is guaranteed to happen.
-              if (ev.target != nodeToAnimate || ev.propertyName != "transform")
-                return;
-
-              this._viewContainer.removeEventListener("transitionend", this._transitionEndListener);
-              this._transitionEndListener = null;
-              onTransitionEnd();
-              this._transitioning = false;
-              this._resetKeyNavigation(previousViewNode);
-
-              // Myeah, panel layout auto-resizing is a funky thing. We'll wait
-              // another few milliseconds to remove the width and height 'fixtures',
-              // to be sure we don't flicker annoyingly.
-              // NB: HACK! Bug 1363756 is there to fix this.
-              this._autoResizeWorkaroundTimer = window.setTimeout(() => {
-                this._viewContainer.style.removeProperty("height");
-                this._viewContainer.style.removeProperty("width");
-              }, 500);
-
-              // Take another breather, just like before, to wait for the 'current'
-              // attribute removal to take effect. This prevents a flicker.
-              // The cleanup we do doesn't affect the display anymore, so we're not
-              // too fussed about the timing here.
-              window.addEventListener("MozAfterPaint", () => {
-                nodeToAnimate.style.removeProperty("border-inline-start");
-                nodeToAnimate.style.removeProperty("transition");
-                nodeToAnimate.style.removeProperty("transform");
-                nodeToAnimate.style.removeProperty("width");
-
-                if (!reverse)
-                  viewNode.style.removeProperty("margin-inline-start");
-                if (aAnchor)
-                  aAnchor.removeAttribute("open");
-
-                this._viewContainer.removeAttribute("transition-reverse");
-
-                this._dispatchViewEvent(viewNode, "ViewShown");
-              }, { once: true });
-            });
-          }, { once: true });
-        });
-      } else if (!this.panelViews) {
+      } else {
         this._transitionHeight(() => {
           viewNode.setAttribute("current", true);
           this.node.setAttribute("viewtype", "subview");
           // Now that the subview is visible, we can check the height of the
           // description elements it contains.
           this.descriptionHeightWorkaround(viewNode);
           this._dispatchViewEvent(viewNode, "ViewShown");
         });
@@ -687,59 +711,75 @@ this.PanelMultiView = class {
       cancelable: eventName == "ViewShowing"
     });
     viewNode.dispatchEvent(evt);
     if (!cancel)
       cancel = evt.defaultPrevented;
     return cancel;
   }
 
+  // Returns a promise that resolves with `false` if the popup got hidden,
+  // and `true` if the event did occur.
+  _promiseEventOnceOrHidden(target, eventType, checkFn) {
+    return new Promise(resolve => {
+      // We also invoke this when the popup hides, in which case `ev` will be null.
+      this._pendingTransitionResolution = ev => {
+        if (ev && checkFn && !checkFn(ev)) {
+          return;
+        }
+        target.removeEventListener(ev, this._pendingTransitionResolution);
+        resolve(!!ev);
+      };
+      target.addEventListener(eventType, this._pendingTransitionResolution);
+    });
+  }
+
   /**
    * Calculate the correct bounds of a panelview node offscreen to minimize the
    * amount of paint flashing and keep the stack vs panel layouts from interfering.
    *
    * @param {panelview} viewNode Node to measure the bounds of.
    * @param {Rect}      previousRect Rect representing the previous view
    *                                 (used to fill in any blanks).
    * @param {Function}  callback Called when we got the measurements in and pass
    *                             them on as its first argument.
    */
-  _viewBoundsOffscreen(viewNode, previousRect, callback) {
+  async _viewBoundsOffscreen(viewNode, previousRect, callback) {
     if (viewNode.__lastKnownBoundingRect) {
-      callback(viewNode.__lastKnownBoundingRect);
-      return;
+      return viewNode.__lastKnownBoundingRect;
     }
 
     if (viewNode.customRectGetter) {
       // Can't use Object.assign directly with a DOM Rect object because its properties
       // aren't enumerable.
       let {height, width} = previousRect;
       let rect = Object.assign({height, width}, viewNode.customRectGetter());
       let {header} = viewNode;
       if (header) {
         rect.height += this._dwu.getBoundsWithoutFlushing(header).height;
       }
-      callback(rect);
-      return;
+      return rect;
     }
 
     let oldSibling = viewNode.nextSibling || null;
     this._offscreenViewStack.appendChild(viewNode);
 
-    this.window.addEventListener("MozAfterPaint", () => {
-      let viewRect = this._dwu.getBoundsWithoutFlushing(viewNode);
+    return new Promise(resolve => {
+      this.window.addEventListener("MozAfterPaint", () => {
+        let viewRect = this._dwu.getBoundsWithoutFlushing(viewNode);
 
-      try {
-        this._viewStack.insertBefore(viewNode, oldSibling);
-      } catch (ex) {
-        this._viewStack.appendChild(viewNode);
-      }
+        try {
+          this._viewStack.insertBefore(viewNode, oldSibling);
+        } catch (ex) {
+          this._viewStack.appendChild(viewNode);
+        }
 
-      callback(viewRect);
-    }, { once: true });
+        resolve(viewRect);
+      }, { once: true });
+    });
   }
 
   /**
    * Applies the height transition for which <panelmultiview> is designed.
    *
    * The height transition involves two elements, the viewContainer and its only
    * immediate child the viewStack. In order for this to work correctly, the
    * viewContainer must have "overflow: hidden;" and the two elements must have
@@ -943,42 +983,43 @@ this.PanelMultiView = class {
         // description elements it contains.
         this.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;
+        // If we hide mid-transition, resolve that:
+        if (this._pendingTransitionResolution) {
+          this._pendingTransitionResolution();
+        }
         this.node.removeAttribute("panelopen");
         this.showMainView();
         if (this.panelViews) {
-          if (this._transitionEndListener) {
-            this._viewContainer.removeEventListener("transitionend", this._transitionEndListener);
-            this._transitionEndListener = null;
-          }
           for (let panelView of this._viewStack.children) {
             if (panelView.nodeName != "children") {
               panelView.__lastKnownBoundingRect = null;
               panelView.style.removeProperty("min-width");
               panelView.style.removeProperty("max-width");
+              this._clearTransitionedStyles(panelView);
             }
           }
           this.window.removeEventListener("keydown", this);
           this._panel.removeEventListener("mousemove", this);
           this._resetKeyNavigation();
 
           // Clear the main view size caches. The dimensions could be different
           // when the popup is opened again, e.g. through touch mode sizing.
           this._mainViewHeight = 0;
           this._mainViewWidth = 0;
           this._viewContainer.style.removeProperty("min-height");
+          this._viewContainer.style.removeProperty("height");
+          this._viewContainer.style.removeProperty("width");
           this._viewStack.style.removeProperty("max-height");
-          this._viewContainer.style.removeProperty("min-width");
-          this._viewContainer.style.removeProperty("max-width");
         }
 
         // Always try to layout the panel normally when reopening it. This is
         // also the layout that will be used in customize mode.
         if (this._mainView.hasAttribute("blockinboxworkaround")) {
           this._mainView.style.removeProperty("height");
           this._mainView.removeAttribute("exceeding");
         }