Bug 1428839 - Part 5 - Open the anchor when subview navigation starts and close it asynchronously afterwards. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 27 Feb 2018 15:52:07 +0000
changeset 761006 071398e4381f890fdb097e09f90dde6c5cd910df
parent 761005 fbf5424642a6eec4ff24a53d8f11d1bc7292182e
child 761007 2a0bc71f245bc93095497d5f75ae6fc5f3ae505a
push id100802
push userpaolo.mozmail@amadzone.org
push dateWed, 28 Feb 2018 13:44:56 +0000
reviewersGijs
bugs1428839
milestone60.0a1
Bug 1428839 - Part 5 - Open the anchor when subview navigation starts and close it asynchronously afterwards. r=Gijs The anchor state does not need to be cleaned up synchronously, so we can handle it seperately from the transition state. MozReview-Commit-ID: 1CBP9OS5WmM
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -626,47 +626,59 @@ var PanelMultiView = class extends this.
     if (!prevPanelView.active) {
       return;
     }
     // Marking the view that is about to scrolled out of the visible area as
     // inactive will prevent re-entrancy and also disable keyboard navigation.
     // From this point onwards, "await" statements can be used safely.
     prevPanelView.active = false;
 
-    // If the ViewShowing event cancels the operation we have to re-enable
-    // keyboard navigation, but this must be avoided if the panel was closed.
-    if (!(await this._openView(nextPanelView))) {
-      if (prevPanelView.isOpenIn(this)) {
-        // We don't raise a ViewShown event because nothing actually changed.
-        // Technically we should use a different state flag just because there
-        // is code that could check the "active" property to determine whether
-        // to wait for a ViewShown event later, but this only happens in
-        // regression tests and is less likely to be a technique used in
-        // production code, where use of ViewShown is less common.
-        prevPanelView.active = true;
+    // Provide visual feedback while navigation is in progress, starting before
+    // the transition starts and ending when the previous view is invisible.
+    if (anchor) {
+      anchor.setAttribute("open", "true");
+    }
+    try {
+      // If the ViewShowing event cancels the operation we have to re-enable
+      // keyboard navigation, but this must be avoided if the panel was closed.
+      if (!(await this._openView(nextPanelView))) {
+        if (prevPanelView.isOpenIn(this)) {
+          // We don't raise a ViewShown event because nothing actually changed.
+          // Technically we should use a different state flag just because there
+          // is code that could check the "active" property to determine whether
+          // to wait for a ViewShown event later, but this only happens in
+          // regression tests and is less likely to be a technique used in
+          // production code, where use of ViewShown is less common.
+          prevPanelView.active = true;
+        }
+        return;
       }
-      return;
+
+      prevPanelView.captureKnownSize();
+
+      // The main view of a panel can be a subview in another one. Make sure to
+      // reset all the properties that may be set on a subview.
+      nextPanelView.mainview = false;
+      // The header may change based on how the subview was opened.
+      nextPanelView.headerText = viewNode.getAttribute("title") ||
+                                 (anchor && anchor.getAttribute("label"));
+      // The constrained width of subviews may also vary between panels.
+      nextPanelView.minMaxWidth = prevPanelView.knownWidth;
+
+      if (anchor) {
+        viewNode.classList.add("PanelUI-subView");
+      }
+
+      await this._transitionViews(prevPanelView.node, viewNode, false, anchor);
+    } finally {
+      if (anchor) {
+        anchor.removeAttribute("open");
+      }
     }
 
-    prevPanelView.captureKnownSize();
-
-    // The main view of a panel can be a subview in another one. Make sure to
-    // reset all the properties that may be set on a subview.
-    nextPanelView.mainview = false;
-    // The header may change based on how the subview was opened.
-    nextPanelView.headerText = viewNode.getAttribute("title") ||
-                               (anchor && anchor.getAttribute("label"));
-    // The constrained width of subviews may also vary between panels.
-    nextPanelView.minMaxWidth = prevPanelView.knownWidth;
-
-    if (anchor) {
-      viewNode.classList.add("PanelUI-subView");
-    }
-
-    await this._transitionViews(prevPanelView.node, viewNode, false, anchor);
     this._activateView(nextPanelView);
   }
 
   /**
    * Navigates backwards by sliding out the most recent subview.
    */
   goBack() {
     this._goBack().catch(Cu.reportError);
@@ -823,39 +835,33 @@ var PanelMultiView = class extends this.
    *
    * @param {panelview} previousViewNode Node that is currently displayed, but
    *                                     is about to be transitioned away. This
    *                                     must be already inactive at this point.
    * @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 {Element}   anchor           the anchor for which we're opening
-   *                                     a new panelview, if any
    */
-  async _transitionViews(previousViewNode, viewNode, reverse, anchor) {
+  async _transitionViews(previousViewNode, viewNode, reverse) {
     // Clean up any previous transition that may be active at this point.
     this._cleanupTransitionPhase();
 
     const { window } = 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,
-      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;
     this._viewContainer.style.minHeight = olderView.knownHeight + "px";
     this._viewContainer.style.height = prevPanelView.knownHeight + "px";
     this._viewContainer.style.width = prevPanelView.knownWidth + "px";
     // Lock the dimensions of the window that hosts the popup panel.
     let rect = this._panel.popupBoxObject.getOuterScreenRect();
@@ -989,25 +995,20 @@ var PanelMultiView = class extends this.
    * @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, resolve, listener, cancelListener, anchor} = details;
+    let {phase, resolve, listener, cancelListener} = 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");
-
     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(() => {