Backed out changeset 793f8bede918 (bug 1947818) for causing mochitest…

…s failures in browser_sidebar_move.js. CLOSED TREE
mozilla · Feb 27, 2025 · 49eabad · 49eabad
1 parent 7c76740
commit 49eabad
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 55 deletions.
diff --git a/browser/components/sidebar/browser-sidebar.js b/browser/components/sidebar/browser-sidebar.js
@@ -710,57 +710,35 @@ var SidebarController = {
   setPosition() {
     // First reset all ordinals to match DOM ordering.
     let contentArea = document.getElementById("tabbrowser-tabbox");
-    if (this.sidebarRevampVisibility !== "expand-on-hover") {
-      let flattenedSidebarEls = [...this.sidebarWrapper.children, contentArea];
-      flattenedSidebarEls.forEach((node, i) => {
-        if (node.id !== "sidebar-wrapper") {
-          node.style.order = i + 1;
-        }
-      });
-    } else {
-      let browser = document.getElementById("browser");
-      [...browser.children].forEach((node, i) => {
-        node.style.order = i + 1;
-      });
-      [...this.sidebarWrapper.children].forEach((node, i) => {
-        node.style.order = i + 1;
-      });
-    }
+    let browser = document.getElementById("browser");
+    [...browser.children].forEach((node, i) => {
+      node.style.order = i + 1;
+    });
+    [...this.sidebarWrapper.children].forEach((node, i) => {
+      node.style.order = i + 1;
+    });
     let sidebarContainer = document.getElementById("sidebar-main");
     let sidebarMain = document.querySelector("sidebar-main");
     if (!this._positionStart) {
-      // DOM ordering is:     sidebar-main | launcher-splitter | sidebar-box | splitter | after-splitter | tabbrowser-tabbox
-      // Want to display as:  tabbrowser-tabbox | after-splitter |  splitter |  sidebar-box  | launcher-splitter | sidebar-main
-      if (this.sidebarRevampVisibility === "expand-on-hover") {
-        // First switch order of sidebar-wrapper and tabbrowser-tabbox
-        let wrapperOrdinal = this.sidebarWrapper.style.order;
-        this.sidebarWrapper.style.order = contentArea.style.order;
-        contentArea.style.order = wrapperOrdinal;
-
-        // Next swap sidebar-main and after-splitter
-        let afterSplitter = document.getElementById("after-splitter");
-        let mainOrdinal = parseInt(this.sidebarContainer.style.order);
-        this.sidebarContainer.style.order = parseInt(afterSplitter.style.order);
-        afterSplitter.style.order = mainOrdinal;
-
-        // Then swap launcher-splitter and splitter
-        const launcherSplitterOrdinal = parseInt(
-          this._launcherSplitter.style.order
-        );
-        this._launcherSplitter.style.order = parseInt(
-          this._splitter.style.order
-        );
-        this._splitter.style.order = launcherSplitterOrdinal;
-      } else {
-        // First switch order of sidebar-main and tabbrowser-tabbox
-        let mainOrdinal = this.sidebarContainer.style.order;
-        this.sidebarContainer.style.order = contentArea.style.order;
-        contentArea.style.order = mainOrdinal;
-        // Then swap launcher-splitter and splitter
-        let splitterOrdinal = this._splitter.style.order;
-        this._splitter.style.order = this._launcherSplitter.style.order;
-        this._launcherSplitter.style.order = splitterOrdinal;
-      }
+      // First switch order of sidebar-wrapper and tabbrowser-tabbox
+      let wrapperOrdinal = this.sidebarWrapper.style.order;
+      this.sidebarWrapper.style.order = contentArea.style.order;
+      contentArea.style.order = wrapperOrdinal;
+
+      // DOM ordering is:     sidebar-main | launcher-splitter | sidebar-box | splitter | after-splitter |
+      // Want to display as:  after-splitter | splitter |  sidebar-box  | launcher-splitter | sidebar-main
+      // First swap sidebar-box and after-splitter
+      let afterSplitter = document.getElementById("after-splitter");
+      let boxOrdinal = this._box.style.order;
+      this._box.style.order = afterSplitter.style.order;
+      afterSplitter.style.order = boxOrdinal;
+
+      // Then move the launcher-splitter to the right of sidebar-box
+      const launcherSplitterOrdinal = parseInt(this._box.style.order) + 1;
+      this._launcherSplitter.style.order = launcherSplitterOrdinal;
+
+      // Finally move the launcher to the right of the launcher-splitter
+      sidebarContainer.style.order = parseInt(launcherSplitterOrdinal) + 1;
 
       // Indicate we've switched ordering to the box
       this._box.toggleAttribute("positionend", true);
@@ -1949,9 +1927,9 @@ var SidebarController = {
   },
 
   async toggleExpandOnHover(isEnabled) {
-    this.setPosition();
     if (isEnabled) {
       this.sidebarWrapper.classList.add("expandOnHover");
+
       if (!this._state) {
         this._state = new this.SidebarState(this);
       }

diff --git a/browser/components/sidebar/tests/browser/browser_sidebar_position.js b/browser/components/sidebar/tests/browser/browser_sidebar_position.js
@@ -31,8 +31,8 @@ add_task(async function test_sidebar_position_end() {
   ok(sidebar, "Sidebar is shown.");
   await sidebar.updateComplete;
 
-  is(sidebar.style.order, "6", "Sidebar is shown at the end");
-  is(sidebarBox.style.order, "3", "Sidebar is shown at the end");
+  is(sidebar.style.order, "7", "Sidebar is shown at the end");
+  is(sidebarBox.style.order, "5", "Sidebar is shown at the end");
 
   await BrowserTestUtils.closeWindow(win);
 });
@@ -45,8 +45,8 @@ add_task(async function test_sidebar_position_end_new_window() {
   ok(sidebar, "Sidebar is shown.");
   await sidebar.updateComplete;
 
-  is(sidebar.style.order, "6", "Sidebar is shown at the end");
-  is(sidebarBox.style.order, "3", "Sidebar is shown at the end");
+  is(sidebar.style.order, "7", "Sidebar is shown at the end");
+  is(sidebarBox.style.order, "5", "Sidebar is shown at the end");
 
   await BrowserTestUtils.closeWindow(win);
 });
diff --git a/browser/themes/shared/sidebar.css b/browser/themes/shared/sidebar.css
@@ -235,11 +235,10 @@ sidebar-main,
 /* Expand on hover */
 
 #sidebar-wrapper {
-  display: contents;
+  position: relative;
 
   &.expandOnHover {
     position: absolute;
-    display: flex;
     inset: 0;
     inset-inline-end: unset;
     z-index: var(--browser-area-z-index-sidebar-wrapper);