Bug 1944931 - Ensure sidebar.visibility gets set when vertical tabs are enabled. r=sidebar-reviewers,sclements,nsharpley

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

browser/base/content/test/keyboard/browser_toolbarKeyNav.js

@@ -148,6 +148,11 @@ add_setup(async function () {
   // The page actions button is not normally visible, so we must
   // unhide it.
   BrowserPageActions.mainButtonNode.style.visibility = "visible";
+
+  // Make sure the sidebar launcher is visible (when sidebar.revamp is true);
+  // previous tests might have hidden it.
+  await SidebarController.initializeUIState({ launcherVisible: true });
+
   registerCleanupFunction(() => {
     BrowserPageActions.mainButtonNode.style.removeProperty("visibility");
   });

browser/components/shopping/tests/browser/browser_reviewchecker_sidebar.js

@@ -21,12 +21,14 @@ add_setup(async function setup() {
   registerCleanupFunction(async () => {
     SidebarController.hide();
   });
+  await SidebarController.initializeUIState({ launcherVisible: true });
 });
 
 add_task(async function test_integrated_sidebar() {
   await BrowserTestUtils.withNewTab(CONTENT_PAGE, async function (browser) {
     const { document } = browser.ownerGlobal;
     let sidebar = document.querySelector("sidebar-main");
+    await sidebar.updateComplete;
     let reviewCheckerButton = await TestUtils.waitForCondition(
       () =>
         sidebar.shadowRoot.querySelector(

browser/components/sidebar/SidebarManager.sys.mjs

@@ -20,11 +20,15 @@ XPCOMUtils.defineLazyPreferenceGetter(
   "sidebarBackupState",
   BACKUP_STATE_PREF
 );
+
 XPCOMUtils.defineLazyPreferenceGetter(
   lazy,
   "verticalTabsEnabled",
   "sidebar.verticalTabs",
-  false
+  false,
+  (pref, oldVal, newVal) => {
+    SidebarManager.handleVerticalTabsPrefChange(newVal);
+  }
 );
 
 export const SidebarManager = {
@@ -74,7 +78,16 @@ export const SidebarManager = {
       );
     });
 
-    if (!lazy.verticalTabsEnabled) {
+    this.handleVerticalTabsPrefChange(lazy.verticalTabsEnabled);
+  },
+
+  /**
+   * Adjust for a change to the verticalTabs pref.
+   */
+  handleVerticalTabsPrefChange(isEnabled) {
+    if (isEnabled) {
+      Services.prefs.setStringPref(VISIBILITY_SETTING_PREF, "always-show");
+    } else {
       Services.prefs.setStringPref(VISIBILITY_SETTING_PREF, "hide-sidebar");
     }
   },

browser/components/sidebar/browser-sidebar.js

@@ -1980,6 +1980,7 @@ XPCOMUtils.defineLazyPreferenceGetter(
     }
   }
 );
+
 XPCOMUtils.defineLazyPreferenceGetter(
   SidebarController,
   "sidebarRevampVisibility",
@@ -1989,19 +1990,21 @@ XPCOMUtils.defineLazyPreferenceGetter(
     if (!SidebarController.inPopup && !SidebarController.uninitializing) {
       SidebarController.toggleExpandOnHover(newValue === "expand-on-hover");
       SidebarController.recordVisibilitySetting(newValue);
-      SidebarController._state.revampVisibility = newValue;
+      if (SidebarController._state) {
-      SidebarController._state.updateVisibility(
+        const isVerticalTabs = SidebarController.sidebarVerticalTabsEnabled;
-        (newValue != "hide-sidebar" &&
+        SidebarController._state.revampVisibility = newValue;
-          SidebarController.sidebarVerticalTabsEnabled) ||
+        SidebarController._state.updateVisibility(
-          !SidebarController.sidebarVerticalTabsEnabled,
+          (newValue != "hide-sidebar" && isVerticalTabs) || !isVerticalTabs,
-        false,
+          false,
-        false,
+          false,
-        newValue !== "expand-on-hover"
+          newValue !== "expand-on-hover"
-      );
+        );
+      }
       SidebarController.updateToolbarButton();
     }
   }
 );
+
 XPCOMUtils.defineLazyPreferenceGetter(
   SidebarController,
   "sidebarVerticalTabsEnabled",
@@ -2010,10 +2013,6 @@ XPCOMUtils.defineLazyPreferenceGetter(
   (_aPreference, _previousValue, newValue) => {
     if (!SidebarController.uninitializing && !SidebarController.inPopup) {
       SidebarController.recordTabsLayoutSetting(newValue);
-      Services.prefs.setStringPref(
-        SidebarController.VISIBILITY_PREF,
-        newValue ? "always-show" : "hide-sidebar"
-      );
     }
   }
 );

browser/components/sidebar/tests/browser/browser_sidebar_collapsed_close_tab_button.js

@@ -5,11 +5,13 @@
 
 add_setup(async () => {
   await SpecialPowers.pushPrefEnv({
-    set: [
+    set: [["sidebar.verticalTabs", true]],
-      ["sidebar.verticalTabs", true],
-      ["sidebar.visibility", "always-show"],
-    ],
   });
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "always-show",
+    "Sanity check the visibilty pref when verticalTabs are enabled"
+  );
 });
 registerCleanupFunction(async () => {
   await SpecialPowers.popPrefEnv();

browser/components/sidebar/tests/browser/browser_sidebar_context_menu.js

@@ -154,6 +154,11 @@ add_task(async function test_toggle_vertical_tabs_from_a_tab() {
   await SpecialPowers.pushPrefEnv({
     set: [["sidebar.verticalTabs", false]],
   });
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "hide-sidebar",
+    "Sanity check the visibilty pref is updated when verticalTabs are disabled"
+  );
 
   info("Enable vertical tabs from a tab.");
   const tabContextMenu = document.getElementById("tabContextMenu");
@@ -170,6 +175,11 @@ add_task(async function test_toggle_vertical_tabs_from_a_tab() {
     () => gBrowser.tabContainer.verticalMode,
     "Vertical tabs are enabled."
   );
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "always-show",
+    "Sanity check the visibilty pref is updated when verticalTabs are enabled"
+  );
 
   info("Disable vertical tabs from a tab.");
   await openAndWaitForContextMenu(tabContextMenu, gBrowser.selectedTab, () => {
@@ -184,6 +194,11 @@ add_task(async function test_toggle_vertical_tabs_from_a_tab() {
     () => !gBrowser.tabContainer.verticalMode,
     "Vertical tabs are disabled."
   );
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "hide-sidebar",
+    "Sanity check the visibilty pref is updated when verticalTabs are disabled"
+  );
 
   await SpecialPowers.popPrefEnv();
 });
@@ -192,6 +207,11 @@ add_task(async function test_toggle_vertical_tabs_from_tab_strip() {
   await SpecialPowers.pushPrefEnv({
     set: [["sidebar.verticalTabs", false]],
   });
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "hide-sidebar",
+    "Sanity check the visibilty pref when verticalTabs are disabled"
+  );
 
   info("Enable vertical tabs from the toolbar.");
   const toolbarContextMenu = document.getElementById("toolbar-context-menu");
@@ -217,6 +237,11 @@ add_task(async function test_toggle_vertical_tabs_from_tab_strip() {
     () => gBrowser.tabContainer.verticalMode,
     "Vertical tabs are enabled."
   );
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "always-show",
+    "Sanity check the visibilty pref when verticalTabs are enabled"
+  );
 
   // Open customize sidebar panel from context menu
   await openAndWaitForContextMenu(
@@ -250,6 +275,11 @@ add_task(async function test_toggle_vertical_tabs_from_tab_strip() {
     () => !gBrowser.tabContainer.verticalMode,
     "Vertical tabs are disabled."
   );
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "hide-sidebar",
+    "Sanity check the visibilty pref when verticalTabs are disabled"
+  );
 
   window.SidebarController.hide();
   await SpecialPowers.popPrefEnv();

browser/components/sidebar/tests/browser/browser_sidebar_prefs.js

@@ -169,6 +169,11 @@ add_task(async function test_flip_revamp_pref() {
   ok(BrowserTestUtils.isVisible(verticalTabs), "Vertical tabs slot is visible");
 
   ok(sidebar, "Revamped sidebar is shown initially.");
+  Assert.equal(
+    Services.prefs.getStringPref("sidebar.visibility"),
+    "always-show",
+    "Sanity check the visibilty pref when verticalTabs are enabled"
+  );
 
   await SpecialPowers.pushPrefEnv({ set: [["sidebar.revamp", false]] });
 

browser/components/sidebar/tests/browser/browser_toolbar_sidebar_button.js

@@ -19,9 +19,14 @@ add_setup(async () => {
     set: [
       [SIDEBAR_TAB_DIRECTION_PREF, true],
       [SIDEBAR_BUTTON_INTRODUCED_PREF, false],
-      [SIDEBAR_VISIBILITY_PREF, "always-show"],
     ],
   });
+  Assert.equal(
+    Services.prefs.getStringPref(SIDEBAR_VISIBILITY_PREF),
+    "always-show",
+    "Sanity check the visibilty pref when verticalTabs are enabled"
+  );
+
   let navbarDefaults = gAreas.get("nav-bar").get("defaultPlacements");
   let hadSavedState = !!CustomizableUI.getTestOnlyInternalProp("gSavedState");
   if (!hadSavedState) {
@@ -179,6 +184,13 @@ add_task(async function test_states_for_hide_sidebar() {
   await SpecialPowers.pushPrefEnv({
     set: [[SIDEBAR_TAB_DIRECTION_PREF, false]],
   });
+
+  Assert.equal(
+    Services.prefs.getStringPref(SIDEBAR_VISIBILITY_PREF),
+    "hide-sidebar",
+    "Sanity check the visibilty pref when verticalTabs are disabled"
+  );
+
   const win = await BrowserTestUtils.openNewBrowserWindow();
   const { SidebarController } = win;
   const { sidebarContainer, sidebarMain, toolbarButton } = SidebarController;
@@ -221,12 +233,8 @@ add_task(async function test_states_for_hide_sidebar() {
   };
 
   // Hide the sidebar
-  info("Check default hidden state.");
+  info("Check the launcher is initially hidden");
-  await checkStates({ hidden: false });
-  info("Hide sidebar using the toolbar button.");
-  EventUtils.synthesizeMouseAtCenter(toolbarButton, {}, win);
   await checkStates({ hidden: true });
-
   info("Show sidebar using the toolbar button.");
   EventUtils.synthesizeMouseAtCenter(toolbarButton, {}, win);
   await checkStates({ hidden: false });
@@ -297,7 +305,7 @@ add_task(async function test_states_for_hide_sidebar_vertical() {
     );
   };
 
-  // Hide the sidebar
+  // Check initial sidebar state - it should be hidden
   info("Check default hidden state.");
   await checkStates({ hidden: true, expanded: false });
   info("Show expanded sidebar using the toolbar button.");