Bug 2052330 - Don't let the launcher overflow observer hide tool buttons in horizontal tabs r=sclements

Differential Revision: https://phabricator.services.mozilla.com/D311147

Author: kcochrane-mozilla

browser/components/sidebar/sidebar-main.mjs

@@ -170,6 +170,13 @@ export default class SidebarMain extends MozLitElement {
   createToolsObservers() {
     this._toolsIntersectionObserver = new IntersectionObserver(
       entries => {
+        // In horizontal tabs mode while the launcher is collapsed or hidden, every
+        // button is considered non-intersecting by the intersection observer which
+        // can cause visibility issues once the sidebar is shown again. We should
+        // return early here if horizontal tabs are enabled to prevent this.
+        if (!window.SidebarController.sidebarVerticalTabsEnabled) {
+          return;
+        }
         this.shouldShowOverflowButton = entries.some(
           entry =>
             !entry.isIntersecting &&
@@ -649,26 +656,40 @@ export default class SidebarMain extends MozLitElement {
   }
 
   updated() {
+    const isExpandOnHover =
+      window.SidebarController.sidebarRevampVisibility === "expand-on-hover";
+
     if (
-      window.SidebarController.sidebarRevampVisibility !== "expand-on-hover"
+      !isExpandOnHover &&
+      window.SidebarController.sidebarVerticalTabsEnabled
     ) {
       for (const buttonEl of this.allButtons) {
         if (buttonEl.hasAttribute("view")) {
           this._toolsIntersectionObserver.observe(buttonEl);
         }
       }
-
       this._toolsResizeObserver.observe(this.buttonGroup);
-    } else {
-      this.shouldShowOverflowButton = !this.expanded;
-      for (const buttonEl of this.allButtons) {
-        if (buttonEl.style.visibility === "hidden") {
-          buttonEl.style.visibility = "visible";
-        }
+      return;
+    }
+
+    // In expand-on-hover or horizontal tabs mode we don't track tool overflow,
+    // so restore any previously overflown/hidden tool buttons and stop
+    // observing. In horizontal tabs mode we also clear the overflow panel
+    // copies that were populated while in vertical tabs.
+    this.shouldShowOverflowButton = isExpandOnHover ? !this.expanded : false;
+    const overflowList = isExpandOnHover
+      ? null
+      : document.getElementById("tools-overflow-list");
+    for (const buttonEl of this.allButtons) {
+      if (buttonEl.style.visibility === "hidden") {
+        buttonEl.style.visibility = "visible";
       }
-      this._toolsIntersectionObserver.disconnect();
-      this._toolsResizeObserver.disconnect();
+      overflowList
+        ?.querySelector(`[view='${buttonEl.getAttribute("view")}']`)
+        ?.remove();
     }
+    this._toolsIntersectionObserver.disconnect();
+    this._toolsResizeObserver.disconnect();
   }
 
   getEntrypointValues(action) {

browser/components/sidebar/tests/browser/browser_customize_sidebar.js

@@ -319,16 +319,41 @@ add_task(async function test_open_tools_from_sidebar_horizontal() {
     "Unchecking turns on the panel switcher dropdown (hide-launcher)."
   );
 
-  info("Re-check to place tools back in the launcher.");
+  // Let the launcher's overflow IntersectionObserver run while the
+  // launcher is hidden and ensure we aren't left with a sidebar
+  // with unexpectedly hidden buttons
+  const { sidebarMain } = window.SidebarController;
+  await sidebarMain.updateComplete;
+  await waitForRepaint();
+  for (const button of sidebarMain.toolButtons) {
+    isnot(
+      button.style.visibility,
+      "hidden",
+      `Tool button ${button.getAttribute("view")} isn't hidden while the ` +
+        `launcher is hidden.`
+    );
+  }
+
   input.click();
   await panel.updateComplete;
   ok(input.checked, "Open tools from sidebar is checked again.");
   is(
     Services.prefs.getStringPref(SIDEBAR_VISIBILITY_PREF),
     "hide-on-close",
-    "Checking places tools in the launcher (hide-on-close)."
+    "Checking 'Open tools from sidebar' option places tool buttons back in the launcher."
   );
 
+  await sidebarMain.updateComplete;
+  await waitForRepaint();
+  ok(sidebarMain.toolButtons.length, "Launcher still has tool buttons.");
+  for (const button of sidebarMain.toolButtons) {
+    is(
+      window.getComputedStyle(button).visibility,
+      "visible",
+      `Tool button ${button.getAttribute("view")} is visible in the launcher.`
+    );
+  }
+
   Services.prefs.clearUserPref(SIDEBAR_VISIBILITY_PREF);
   await SpecialPowers.popPrefEnv();
 });

browser/components/sidebar/tests/browser/browser_tools_overflow.js

@@ -294,6 +294,83 @@ add_task(async function test_overflow_menu_with_keyboard() {
   }
 });
 
+add_task(
+  async function test_overflowing_buttons_restored_switching_to_horizontal() {
+    await SidebarController.updateUIState({
+      launcherExpanded: false,
+    });
+
+    await SpecialPowers.pushPrefEnv({
+      set: [
+        ["sidebar.main.tools", "aichat,passwords,syncedtabs,history,bookmarks"],
+      ],
+    });
+
+    const sidebar = SidebarController.sidebarMain;
+    await resetToolsHeight();
+
+    info("Resize the tools container to force overflow.");
+    await resizeTools(600);
+    await sidebar.updateComplete;
+    await SidebarController.waitUntilStable();
+    await BrowserTestUtils.waitForMutationCondition(
+      sidebar.buttonsWrapper,
+      { attributes: true, attributeFilter: ["overflowing"] },
+      () => sidebar.shouldShowOverflowButton
+    );
+
+    await BrowserTestUtils.waitForMutationCondition(
+      sidebar.buttonsWrapper,
+      { attributes: true, attributeFilter: ["style"], subtree: true },
+      () =>
+        Array.from(sidebar.toolButtons).some(
+          button => button.style.visibility === "hidden"
+        ),
+      "At least one tool button is hidden while overflowing in vertical tabs."
+    );
+
+    info("Switch to horizontal tabs.");
+    await SpecialPowers.pushPrefEnv({ set: [[VERTICAL_TABS_PREF, false]] });
+    await sidebar.updateComplete;
+    await SidebarController.waitUntilStable();
+
+    await BrowserTestUtils.waitForMutationCondition(
+      sidebar.buttonsWrapper,
+      { attributes: true, attributeFilter: ["style"], subtree: true },
+      () =>
+        Array.from(sidebar.toolButtons).every(
+          button => button.style.visibility !== "hidden"
+        ),
+      "No tool buttons remain hidden after switching to horizontal tabs."
+    );
+    for (const button of sidebar.toolButtons) {
+      is(
+        window.getComputedStyle(button).visibility,
+        "visible",
+        `Tool button ${button.getAttribute("view")} is visible in horizontal tabs.`
+      );
+    }
+    ok(
+      !sidebar.shouldShowOverflowButton,
+      "Overflow button is not shown in horizontal tabs."
+    );
+    is(
+      document.getElementById("tools-overflow-list").childElementCount,
+      0,
+      "Overflow panel copies were cleared."
+    );
+
+    info("Switch back to vertical tabs.");
+    await SpecialPowers.popPrefEnv();
+    await sidebar.updateComplete;
+    await resetToolsHeight();
+
+    while (gBrowser.tabs.length > 1) {
+      BrowserTestUtils.removeTab(gBrowser.tabs.at(-1));
+    }
+  }
+);
+
 add_task(async function test_tools_overflow() {
   const sidebar = document.querySelector("sidebar-main");
   ok(sidebar, "Sidebar is shown.");