Bug 1428839 - Part 8 - Fix the sliding transition when views are reordered. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Wed, 28 Feb 2018 13:43:51 +0000
changeset 761009 c647f89d710631c7a3f6252b68967d606652158f
parent 761008 28706bcd77ce1002ef633614002a99a791faa2eb
push id100802
push userpaolo.mozmail@amadzone.org
push dateWed, 28 Feb 2018 13:44:56 +0000
reviewersGijs
bugs1428839
milestone60.0a1
Bug 1428839 - Part 8 - Fix the sliding transition when views are reordered. r=Gijs Views moved to a different panel and then moved back could be placed after the subviews they give access to, if the other subviews were not moved. The transition would be incorrect when these subviews are opened later. MozReview-Commit-ID: 6JJa0p0McxL
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -863,50 +863,49 @@ var PanelMultiView = class extends this.
     let viewRect;
     if (reverse) {
       // Use the cached size when going back to a previous view, but not when
       // reopening a subview, because its contents may have changed.
       viewRect = { width: nextPanelView.knownWidth,
                    height: nextPanelView.knownHeight };
       nextPanelView.visible = true;
     } else if (viewNode.customRectGetter) {
-      // Can't use Object.assign directly with a DOM Rect object because its properties
-      // aren't enumerable.
+      // We use a customRectGetter for WebExtensions panels, because they need
+      // to query the size from an embedded browser. The presence of this
+      // getter also provides an indication that the view node shouldn't be
+      // moved around, otherwise the state of the browser would get disrupted.
       let width = prevPanelView.knownWidth;
       let height = prevPanelView.knownHeight;
       viewRect = Object.assign({height, width}, viewNode.customRectGetter());
       let header = viewNode.firstChild;
       if (header && header.classList.contains("panel-header")) {
         viewRect.height += this._dwu.getBoundsWithoutFlushing(header).height;
       }
       nextPanelView.visible = true;
       nextPanelView.descriptionHeightWorkaround();
     } else {
-      let oldSibling = viewNode.nextSibling || null;
       this._offscreenViewStack.style.minHeight = olderView.knownHeight + "px";
       this._offscreenViewStack.appendChild(viewNode);
       nextPanelView.visible = true;
 
       // Now that the subview is visible, we can check the height of the
       // description elements it contains.
       nextPanelView.descriptionHeightWorkaround();
 
       viewRect = await window.promiseDocumentFlushed(() => {
         return this._dwu.getBoundsWithoutFlushing(viewNode);
       });
       // Bail out if the panel was closed in the meantime.
       if (!nextPanelView.isOpenIn(this)) {
         return;
       }
 
-      try {
-        this._viewStack.insertBefore(viewNode, oldSibling);
-      } catch (ex) {
-        this._viewStack.appendChild(viewNode);
-      }
+      // Place back the view after all the other views that are already open in
+      // order for the transition to work as expected.
+      this._viewStack.appendChild(viewNode);
 
       this._offscreenViewStack.style.removeProperty("min-height");
     }
 
     this._transitioning = true;
     details.phase = TRANSITION_PHASES.PREPARE;
 
     // The 'magic' part: build up the amount of pixels to move right or left.