Backed out 2 changesets (bug 1858762, bug 1867336) as requested by jhirsch.

Backed out changeset 5df38003afc9 (bug 1867336)
Backed out changeset 7ec919f740b9 (bug 1858762)

browser/components/shopping/ShoppingSidebarParent.sys.mjs

@@ -160,7 +160,7 @@ class ShoppingSidebarManagerClass {
 
     if (!this.isActive) {
       document.querySelectorAll("shopping-sidebar").forEach(sidebar => {
-        sidebar.toggleAttribute("closed", true);
+        sidebar.hidden = true;
       });
     }
 
@@ -196,18 +196,18 @@ class ShoppingSidebarManagerClass {
     if (isProduct && this.isActive) {
       if (!sidebar) {
         sidebar = document.createXULElement("shopping-sidebar");
-        sidebar.removeAttribute("closed");
+        sidebar.hidden = false;
         let splitter = document.createXULElement("splitter");
         splitter.classList.add("sidebar-splitter");
         browserPanel.appendChild(splitter);
         browserPanel.appendChild(sidebar);
       } else {
         actor?.updateProductURL(aLocationURI, aFlags);
-        sidebar.removeAttribute("closed");
+        sidebar.hidden = false;
       }
-    } else if (sidebar && !sidebar.hasAttribute("closed")) {
+    } else if (sidebar && !sidebar.hidden) {
       actor?.updateProductURL(null);
-      sidebar.toggleAttribute("closed", true);
+      sidebar.hidden = true;
     }
 
     this._updateBCActiveness(aBrowser);
@@ -215,7 +215,7 @@ class ShoppingSidebarManagerClass {
 
     if (
       sidebar &&
-      !sidebar.hasAttribute("closed") &&
+      !sidebar.hidden &&
       lazy.ShoppingUtils.isProductPageNavigation(aLocationURI, aFlags)
     ) {
       Glean.shopping.surfaceDisplayed.record();
@@ -258,7 +258,7 @@ class ShoppingSidebarManagerClass {
       browsingContext.isActive =
         !document.hidden &&
         aBrowser == gBrowser.selectedBrowser &&
-        !sidebar.hasAttribute("closed");
+        !sidebar.hidden;
     } catch (ex) {
       // The setter can throw and we do need to run the rest of this
       // code in that case.

browser/components/shopping/content/shopping-sidebar.js

@@ -9,9 +9,6 @@
 {
   const SHOPPING_SIDEBAR_WIDTH_PREF =
     "browser.shopping.experience2023.sidebarWidth";
-  const SHOPPING_SIDEBAR_WIDTH_VAR = "--shopping-sidebar-width";
-  const SHOPPINGS_SIDEBAR_WIDTH_TRANSLATE_X_VAR =
-    "--shopping-sidebar-width-translate-x";
   class ShoppingSidebar extends MozXULElement {
     #browser;
     #initialized;
@@ -55,10 +52,7 @@
         0
       );
       if (previousWidth > 0) {
-        this.style.setProperty(
-          SHOPPING_SIDEBAR_WIDTH_VAR,
-          `${previousWidth}px`
-        );
+        this.style.width = `${previousWidth}px`;
       }
 
       this.resizeObserver = new ResizeObserver(this.resizeObserverFn);
@@ -69,14 +63,6 @@
 
     resizeObserverFn() {
       Services.prefs.setIntPref(SHOPPING_SIDEBAR_WIDTH_PREF, this.scrollWidth);
-
-      /* Setting `--shopping-sidebar-width` directly would cause the resize observer to loop.
-       * Update `shopping-sidebar-width-translate-x` instead to prevent this
-       * after opening or closing the sidebar. */
-      this.style.setProperty(
-        SHOPPINGS_SIDEBAR_WIDTH_TRANSLATE_X_VAR,
-        `${this.scrollWidth}px`
-      );
     }
   }
 

browser/components/shopping/tests/browser/browser_shopping_urlbar.js

@@ -133,11 +133,6 @@ add_task(async function test_button_toggles_sidebars() {
     sidebar = browserPanel.querySelector("shopping-sidebar");
     ok(BrowserTestUtils.is_visible(sidebar), "Shopping sidebar should be open");
 
-    let closedSidebarPromise = BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_hidden(sidebar),
-      "Sidebar should be closed"
-    );
-
     // close
     shoppingButton.click();
     await BrowserTestUtils.waitForMutationCondition(
@@ -148,7 +143,10 @@ add_task(async function test_button_toggles_sidebars() {
       () => shoppingButton.getAttribute("shoppingsidebaropen") == "false"
     );
 
-    await closedSidebarPromise;
+    ok(
+      BrowserTestUtils.is_hidden(sidebar),
+      "Shopping sidebar should be closed"
+    );
   });
 });
 
@@ -203,15 +201,6 @@ add_task(async function test_button_toggles_all_windows() {
     "Shopping sidebar should be open in new window"
   );
 
-  let closedSidebarA = BrowserTestUtils.waitForCondition(
-    () => BrowserTestUtils.is_hidden(sidebarA),
-    "sidebar A should be closed"
-  );
-  let closedSidebarB = BrowserTestUtils.waitForCondition(
-    () => BrowserTestUtils.is_hidden(sidebarA),
-    "sidebar B should be closed"
-  );
-
   // close
   shoppingButton.click();
   await BrowserTestUtils.waitForMutationCondition(
@@ -222,8 +211,14 @@ add_task(async function test_button_toggles_all_windows() {
     () => shoppingButton.getAttribute("shoppingsidebaropen") == "false"
   );
 
-  await closedSidebarA;
-  await closedSidebarB;
+  ok(
+    BrowserTestUtils.is_hidden(sidebarA),
+    "Shopping sidebar should be closed in current window"
+  );
+  ok(
+    BrowserTestUtils.is_hidden(sidebarB),
+    "Shopping sidebar should be closed in new window"
+  );
 
   BrowserTestUtils.removeTab(tab);
   await BrowserTestUtils.closeWindow(newWindow);

browser/themes/shared/browser-shared.css

@@ -1163,35 +1163,8 @@ panel toolbarseparator {
 /* Shopping sidebar */
 
 shopping-sidebar {
-  /* `--shopping-sidebar-width` and `--shopping-sidebar-width-translate-x`
-   * have the same default values, but they should still be separate vars
-   * (see shopping-sidebar.js). */
-  --shopping-sidebar-width: 320px;
-  --shopping-sidebar-width-translate-x: var(--shopping-sidebar-width);
-
   border-inline-start: 1px solid var(--chrome-content-separator-color);
   min-width: 16.92em;
   max-width: 32.31em;
-  width: var(--shopping-sidebar-width);
-  visibility: visible;
-  inset-inline-end: 0;
-
-  &[closed] {
-    transform: translateX(var(--shopping-sidebar-width-translate-x));
-    visibility: collapse;
-    position: absolute;
-    /* Because of absolute positioning, height would normally be set to 0 and break animations.
-     * Set height to 100% so that animations are smooth and the sidebar maintains its original size. */
-    height: 100%;
-
-    &:-moz-locale-dir(rtl) {
-      transform: translateX(calc(-1 * var(--shopping-sidebar-width-translate-x)));
-    }
-  }
-}
-
-@media not (prefers-reduced-motion) {
-  shopping-sidebar {
-    transition: visibility 0.3s, transform 0.3s ease-in-out;
-  }
+  width: 26.67em;
 }

toolkit/components/shopping/test/browser/browser_shopping_integration.js

@@ -15,9 +15,9 @@ add_task(async function test_sidebar_navigation() {
   await BrowserTestUtils.withNewTab(PRODUCT_TEST_URL, async browser => {
     let sidebar = gBrowser.getPanel(browser).querySelector("shopping-sidebar");
     Assert.ok(sidebar, "Sidebar should exist");
-    await BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_visible(sidebar),
-      "Sidebar should be visible"
+    Assert.ok(
+      BrowserTestUtils.is_visible(sidebar),
+      "Sidebar should be visible."
     );
     info("Waiting for sidebar to update.");
     await promiseSidebarUpdated(sidebar, PRODUCT_TEST_URL);
@@ -33,15 +33,14 @@ add_task(async function test_sidebar_navigation() {
       BrowserTestUtils.browserLoaded(browser, false, OTHER_PRODUCT_TEST_URL),
       promiseSidebarUpdated(sidebar, OTHER_PRODUCT_TEST_URL),
     ]);
-    let openedSidebarPromise = BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_visible(sidebar),
-      "Sidebar should be visible"
-    );
     BrowserTestUtils.startLoadingURIString(browser, OTHER_PRODUCT_TEST_URL);
     info("Loading another product.");
     await loadedPromise;
     Assert.ok(sidebar, "Sidebar should exist.");
-    await openedSidebarPromise;
+    Assert.ok(
+      BrowserTestUtils.is_visible(sidebar),
+      "Sidebar should be visible."
+    );
     info("Verifying another product.");
     await verifyProductInfo(sidebar, {
       productURL: OTHER_PRODUCT_TEST_URL,
@@ -55,25 +54,16 @@ add_task(async function test_sidebar_navigation() {
       false,
       "https://example.com/1"
     );
-    let closedSidebarPromise = BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_hidden(sidebar),
-      "Shopping sidebar should be hidden"
-    );
-
     BrowserTestUtils.startLoadingURIString(browser, "https://example.com/1");
     info("Go to a non-product.");
     await loadedPromise;
-    await closedSidebarPromise;
+    Assert.ok(BrowserTestUtils.is_hidden(sidebar));
 
     // Navigate using pushState:
     loadedPromise = BrowserTestUtils.waitForLocationChange(
       gBrowser,
       PRODUCT_TEST_URL
     );
-    openedSidebarPromise = BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_visible(sidebar),
-      "Sidebar should be visible"
-    );
     info("Navigate to the first product using pushState.");
     await SpecialPowers.spawn(browser, [PRODUCT_TEST_URL], urlToUse => {
       content.history.pushState({}, null, urlToUse);
@@ -83,7 +73,10 @@ add_task(async function test_sidebar_navigation() {
     info("Waiting for the sidebar to have updated.");
     await promiseSidebarUpdated(sidebar, PRODUCT_TEST_URL);
     Assert.ok(sidebar, "Sidebar should exist");
-    await openedSidebarPromise;
+    Assert.ok(
+      BrowserTestUtils.is_visible(sidebar),
+      "Sidebar should be visible."
+    );
 
     info("Waiting to verify the first product a second time.");
     await verifyProductInfo(sidebar, {
@@ -143,9 +136,9 @@ add_task(async function test_button_visible_when_opted_out() {
         .getPanel(browser)
         .querySelector("shopping-sidebar");
       Assert.ok(sidebar, "Sidebar should exist");
-      await BrowserTestUtils.waitForCondition(
-        () => BrowserTestUtils.is_visible(sidebar),
-        "Sidebar should be visible"
+      Assert.ok(
+        BrowserTestUtils.is_visible(sidebar),
+        "Sidebar should be visible."
       );
       info("Waiting for sidebar to update.");
       await promiseSidebarUpdated(sidebar, PRODUCT_TEST_URL);
@@ -204,9 +197,9 @@ add_task(async function test_sidebar_button_open_close() {
   await BrowserTestUtils.withNewTab(PRODUCT_TEST_URL, async browser => {
     let sidebar = gBrowser.getPanel(browser).querySelector("shopping-sidebar");
     Assert.ok(sidebar, "Sidebar should exist");
-    await BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_visible(sidebar),
-      "Sidebar should be visible"
+    Assert.ok(
+      BrowserTestUtils.is_visible(sidebar),
+      "Sidebar should be visible."
     );
     let shoppingButton = document.getElementById("shopping-sidebar-button");
     ok(
@@ -224,25 +217,16 @@ add_task(async function test_sidebar_button_open_close() {
       letterGrade: "B",
     });
 
-    let closedSidebarPromise = BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_hidden(sidebar),
-      "Sidebar should be hidden"
-    );
-
     // close the sidebar
     shoppingButton.click();
-
-    await closedSidebarPromise;
-
-    let openedSidebarPromise = BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_visible(sidebar),
-      "Sidebar should be visible"
-    );
+    ok(BrowserTestUtils.is_hidden(sidebar), "Sidebar should be hidden");
 
     // reopen the sidebar
     shoppingButton.click();
-
-    await openedSidebarPromise;
+    Assert.ok(
+      BrowserTestUtils.is_visible(sidebar),
+      "Sidebar should be visible."
+    );
 
     info("Waiting for sidebar to update.");
     await promiseSidebarUpdated(sidebar, PRODUCT_TEST_URL);
@@ -272,9 +256,9 @@ add_task(async function test_no_reliability_available() {
 
     Assert.ok(sidebar, "Sidebar should exist");
 
-    await BrowserTestUtils.waitForCondition(
-      () => BrowserTestUtils.is_visible(sidebar),
-      "Sidebar should be visible"
+    Assert.ok(
+      BrowserTestUtils.is_visible(sidebar),
+      "Sidebar should be visible."
     );
     info("Waiting for sidebar to update.");
     await promiseSidebarUpdated(sidebar, NEEDS_ANALYSIS_TEST_URL);