Bug 1401383 - remove anchor state after transition even if the transition is canceled, and always set main view as current, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 19 Sep 2017 23:53:59 +0100
changeset 667334 5dbab193f4a04b7369da5b4693b28fe9d580ca98
parent 666884 e4261f5b96ebfd63e7cb8af3035ff9fea90c74a5
child 732344 f6c08238904d3d1659dfe8021fbccf18d4655ccd
push id80668
push userbmo:gijskruitbosch+bugs@gmail.com
push dateTue, 19 Sep 2017 23:04:14 +0000
reviewersmikedeboer
bugs1401383
milestone57.0a1
Bug 1401383 - remove anchor state after transition even if the transition is canceled, and always set main view as current, r?mikedeboer Prior to this change, showMainView set the 'current' attribute on a main view, but then _cleanupTransitionPhase() would remove it again. This patch fixes that by calling showMainView *after* _cleanupTransitionPhase. Separately, we weren't removing the 'open' attribute from the anchor if the transition didn't complete. This patch fixes this by moving the addition of 'open' into _transitionViews, and its removal into _cleanupTransitionPhase. MozReview-Commit-ID: 24FYaxDVlga
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -529,23 +529,17 @@ this.PanelMultiView = class {
         if (!playTransition) {
           viewNode.setAttribute("current", true);
           this.descriptionHeightWorkaround(viewNode);
         }
       }
 
       // Now we have to transition the panel.
       if (this.panelViews && playTransition) {
-        if (aAnchor)
-          aAnchor.setAttribute("open", true);
-
-        await this._transitionViews(previousViewNode, viewNode, reverse, previousRect);
-
-        if (aAnchor)
-          aAnchor.removeAttribute("open");
+        await this._transitionViews(previousViewNode, viewNode, reverse, previousRect, aAnchor);
 
         this._dispatchViewEvent(viewNode, "ViewShown");
         this._updateKeyboardFocus(viewNode);
       } else if (!this.panelViews) {
         this._transitionHeight(() => {
           viewNode.setAttribute("current", true);
           if (viewNode.id == this._mainViewId) {
             this.node.setAttribute("viewtype", "main");
@@ -572,37 +566,39 @@ this.PanelMultiView = class {
    * @param {panelview} previousViewNode Node that is currently shown as active,
    *                                     but is about to be transitioned away.
    * @param {panelview} viewNode         Node that will becode the active view,
    *                                     after the transition has finished.
    * @param {Boolean}   reverse          Whether we're navigation back to a
    *                                     previous view or forward to a next view.
    * @param {Object}    previousRect     Rect object, with the same structure as
    *                                     a DOMRect, of the `previousViewNode`.
-   * @param {Function}  callback         Function that will be invoked when the
-   *                                     transition is finished or when the
-   *                                     operation was canceled (early return).
+   * @param {Element}   anchor           the anchor for which we're opening
+   *                                     a new panelview, if any
    */
-  async _transitionViews(previousViewNode, viewNode, reverse, previousRect) {
+  async _transitionViews(previousViewNode, viewNode, reverse, previousRect, anchor) {
     // There's absolutely no need to show off our epic animation skillz when
     // the panel's not even open.
     if (this._panel.state != "open") {
       return;
     }
 
     const {window, document} = this;
 
     if (this._autoResizeWorkaroundTimer)
       window.clearTimeout(this._autoResizeWorkaroundTimer);
 
     this._transitionDetails = {
       phase: TRANSITION_PHASES.START,
-      previousViewNode, viewNode, reverse
+      previousViewNode, viewNode, reverse, anchor
     };
 
+    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);
@@ -699,27 +695,30 @@ this.PanelMultiView = class {
    * Attempt to clean up the attributes and properties set by `_transitionViews`
    * above. Which attributes and properties depends on the phase the transition
    * was left from - normally that'd be `TRANSITION_PHASES.END`.
    */
   async _cleanupTransitionPhase() {
     if (!this._transitionDetails)
       return;
 
-    let {phase, previousViewNode, viewNode, reverse, resolve, listener} = this._transitionDetails;
+    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");
     if (reverse)
       this._resetKeyNavigation(previousViewNode);
     this.descriptionHeightWorkaround(viewNode);
 
+    if (anchor)
+      anchor.removeAttribute("open");
+
     if (phase >= TRANSITION_PHASES.START) {
       this._panel.removeAttribute("width");
       this._panel.removeAttribute("height");
       // 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 = this.window.setTimeout(() => {
@@ -990,17 +989,16 @@ this.PanelMultiView = class {
         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;
         this.node.removeAttribute("panelopen");
-        this.showMainView();
         if (this.panelViews) {
           this._cleanupTransitionPhase();
           for (let panelView of this._viewStack.children) {
             if (panelView.nodeName != "children") {
               panelView.__lastKnownBoundingRect = null;
               panelView.style.removeProperty("min-width");
               panelView.style.removeProperty("max-width");
             }
@@ -1013,16 +1011,17 @@ this.PanelMultiView = class {
           // 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._viewStack.style.removeProperty("max-height");
           this._viewContainer.style.removeProperty("min-width");
           this._viewContainer.style.removeProperty("max-width");
         }
+        this.showMainView();
 
         // 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");
         }
         this._dispatchViewEvent(this.node, "PanelMultiViewHidden");