Bug 1428839 - Part 3 - Clean up view properties when opening them. r=Gijs
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 27 Feb 2018 15:35:42 +0000
changeset 761259 288b230b356c58e0a4980e1f4cd3642d3fa2d872
parent 761258 0605579057e6b57aac46deecae3ead4084575427
child 761260 96dd6c0d53b9be1532119e23876d21e9d51b9078
push id100926
push userrwood@mozilla.com
push dateWed, 28 Feb 2018 21:51:29 +0000
reviewersGijs
bugs1428839
milestone60.0a1
Bug 1428839 - Part 3 - Clean up view properties when opening them. r=Gijs This allows the state to be handled correctly when views are moved to a different panel. The "margin-inline-start" style property on the view stack is also reset unconditionally, allowing less state to be stored in the transition details object. MozReview-Commit-ID: IpgnYsVvx0w
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -696,16 +696,19 @@ var PanelMultiView = class extends this.
 
     return true;
   }
 
   /**
    * Opens the specified PanelView and dispatches the ViewShowing event, which
    * can be used to populate the subview or cancel the operation.
    *
+   * This also clears all the attributes and styles that may be left by a
+   * transition that was interrupted.
+   *
    * @resolves With true if the view was opened, false otherwise.
    */
   async _openView(panelView) {
     if (panelView.node.parentNode != this._viewStack) {
       this._viewStack.appendChild(panelView.node);
     }
 
     panelView.node.panelMultiView = this.node;
@@ -724,16 +727,23 @@ var PanelMultiView = class extends this.
     if (canceled) {
       // Handlers for ViewShowing can't know if a different handler requested
       // cancellation, so this will dispatch a ViewHiding event to give a chance
       // to clean up.
       this._closeLatestView();
       return false;
     }
 
+    // Clean up all the attributes and styles related to transitions. We do this
+    // here rather than when the view is closed because we are likely to make
+    // other DOM modifications soon, which isn't the case when closing.
+    let { style } = panelView.node;
+    style.removeProperty("outline");
+    style.removeProperty("width");
+
     return true;
   }
 
   /**
    * Activates the specified view and raises the ViewShown event, unless the
    * view was closed in the meantime.
    */
   _activateView(panelView) {
@@ -750,17 +760,18 @@ var PanelMultiView = class extends this.
    *       to ViewHidden or ViewClosed instead, see bug 1438507.
    */
   _closeLatestView() {
     let panelView = this.openViews.pop();
     panelView.clearNavigation();
     panelView.dispatchCustomEvent("ViewHiding");
     panelView.node.panelMultiView = null;
     // Views become invisible synchronously when they are closed, and they won't
-    // become visible again until they are opened.
+    // become visible again until they are opened. When this is called at the
+    // end of backwards navigation, the view is already invisible.
     panelView.visible = false;
   }
 
   /**
    * Closes all the views that are currently open.
    */
   closeAllViews() {
     // Raise ViewHiding events for open views in reverse order.
@@ -800,17 +811,17 @@ var PanelMultiView = class extends this.
     let nextPanelView = PanelView.forNode(viewNode);
     let prevPanelView = PanelView.forNode(previousViewNode);
 
     if (this._autoResizeWorkaroundTimer)
       window.clearTimeout(this._autoResizeWorkaroundTimer);
 
     let details = this._transitionDetails = {
       phase: TRANSITION_PHASES.START,
-      previousViewNode, viewNode, reverse, anchor
+      anchor
     };
 
     if (anchor)
       anchor.setAttribute("open", "true");
 
     // Set the viewContainer dimensions to make sure only the current view is
     // visible.
     let olderView = reverse ? nextPanelView : prevPanelView;
@@ -930,44 +941,44 @@ var PanelMultiView = class extends this.
         if (ev.target != this._viewStack)
           return;
         this._viewContainer.removeEventListener("transitioncancel", details.cancelListener);
         delete details.cancelListener;
         resolve();
       });
     });
 
-    // Apply the final visibility, unless the view was closed in the meantime.
-    if (nextPanelView.isOpenIn(this)) {
-      prevPanelView.visible = false;
+    // Bail out if the panel was closed during the transition.
+    if (!nextPanelView.isOpenIn(this)) {
+      return;
     }
+    prevPanelView.visible = false;
 
     // This will complete the operation by removing any transition properties.
+    nextPanelView.node.style.removeProperty("width");
+    deepestNode.style.removeProperty("outline");
     this._cleanupTransitionPhase(details);
 
-    // Focus the correct element, unless the view was closed in the meantime.
-    if (nextPanelView.isOpenIn(this)) {
-      nextPanelView.focusSelectedElement();
-    }
+    nextPanelView.focusSelectedElement();
   }
 
   /**
    * 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.
    *
    * @param {Object} details Dictionary object containing details of the transition
    *                         that should be cleaned up after. Defaults to the most
    *                         recent details.
    */
   _cleanupTransitionPhase(details = this._transitionDetails) {
     if (!details || !this.node)
       return;
 
-    let {phase, previousViewNode, viewNode, reverse, resolve, listener, cancelListener, anchor} = details;
+    let {phase, resolve, listener, cancelListener, anchor} = details;
     if (details == this._transitionDetails)
       this._transitionDetails = null;
 
     // Do the things we _always_ need to do whenever the transition ends or is
     // interrupted.
     if (anchor)
       anchor.removeAttribute("open");
 
@@ -982,25 +993,21 @@ var PanelMultiView = class extends this.
         if (!this._viewContainer)
           return;
         this._viewContainer.style.removeProperty("height");
         this._viewContainer.style.removeProperty("width");
       }, 500);
     }
     if (phase >= TRANSITION_PHASES.PREPARE) {
       this._transitioning = false;
-      if (reverse)
-        this._viewStack.style.removeProperty("margin-inline-start");
-      let deepestNode = reverse ? previousViewNode : viewNode;
-      deepestNode.style.removeProperty("outline");
+      this._viewStack.style.removeProperty("margin-inline-start");
       this._viewStack.style.removeProperty("transition");
     }
     if (phase >= TRANSITION_PHASES.TRANSITION) {
       this._viewStack.style.removeProperty("transform");
-      viewNode.style.removeProperty("width");
       if (listener)
         this._viewContainer.removeEventListener("transitionend", listener);
       if (cancelListener)
         this._viewContainer.removeEventListener("transitioncancel", cancelListener);
       if (resolve)
         resolve();
     }
   }