Bug 1724668 - Ensure Pocket panel closes if location changes with locationSpecific CustomizableUI prop. r=Gijs

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

browser/base/content/browser.js

@@ -2018,16 +2018,6 @@ var gBrowserInit = {
       PanicButtonNotifier.init();
     });
 
-    gBrowser.tabContainer.addEventListener("TabSelect", function() {
-      for (let panel of document.querySelectorAll(
-        "panel[tabspecific='true']"
-      )) {
-        if (panel.state == "open") {
-          panel.hidePopup();
-        }
-      }
-    });
-
     if (BrowserHandler.kiosk) {
       // We don't modify popup windows for kiosk mode
       if (!gURLBar.readOnly) {
@@ -5422,6 +5412,25 @@ var XULBrowserWindow = {
       );
     }
 
+    let closeOpenPanels = selector => {
+      for (let panel of document.querySelectorAll(selector)) {
+        if (panel.state == "open") {
+          panel.hidePopup();
+        }
+      }
+    };
+
+    // If the location is changed due to switching tabs,
+    // ensure we close any open tabspecific panels.
+    if (aIsSimulated) {
+      closeOpenPanels("panel[tabspecific='true']");
+    }
+
+    // Ensure we close any remaining open locationspecific panels
+    if (!isSameDocument) {
+      closeOpenPanels("panel[locationspecific='true']");
+    }
+
     // About pages other than about:reader are not currently supported by
     // screenshots (see Bug 1620992).
     Services.obs.notifyObservers(

browser/base/content/test/tabPrompts/browser_closeTabSpecificPanels.js

@@ -12,7 +12,9 @@ add_task(async function() {
   let tab2 = BrowserTestUtils.addTab(gBrowser, "http://mochi.test:8888/#1");
   let specificPanel = document.createXULElement("panel");
   specificPanel.setAttribute("tabspecific", "true");
+  specificPanel.setAttribute("noautohide", "true");
   let generalPanel = document.createXULElement("panel");
+  generalPanel.setAttribute("noautohide", "true");
   let anchor = document.getElementById(CustomizableUI.AREA_NAVBAR);
 
   anchor.appendChild(specificPanel);

browser/components/customizableui/CustomizableUI.jsm

@@ -1843,6 +1843,9 @@ var CustomizableUIInternal = {
       if (aWidget.tabSpecific) {
         node.setAttribute("tabspecific", aWidget.tabSpecific);
       }
+      if (aWidget.locationSpecific) {
+        node.setAttribute("locationspecific", aWidget.locationSpecific);
+      }
 
       let shortcut;
       if (aWidget.shortcutId) {
@@ -2894,6 +2897,7 @@ var CustomizableUIInternal = {
       defaultArea: null,
       shortcutId: null,
       tabSpecific: false,
+      locationSpecific: false,
       tooltiptext: null,
       l10nId: null,
       showInPrivateBrowsing: true,
@@ -2937,6 +2941,7 @@ var CustomizableUIInternal = {
       "showInPrivateBrowsing",
       "overflows",
       "tabSpecific",
+      "locationSpecific",
       "localized",
     ];
     for (let prop of kOptBoolProps) {
@@ -4008,6 +4013,10 @@ var CustomizableUI = {
    *                  as the "$shortcut" variable to the fluent message.
    * - showInPrivateBrowsing: whether to show the widget in private browsing
    *                          mode (optional, default: true)
+   * - tabSpecific:      If true, closes the panel if the tab changes.
+   * - locationSpecific: If true, closes the panel if the location changes.
+   *                     This is similar to tabSpecific, but also if the location
+   *                     changes in the same tab, we may want to close the panel.
    *
    * @param aProperties the specifications for the widget.
    * @return a wrapper around the created widget (see getWidget)

browser/components/customizableui/content/panelUI.js

@@ -468,6 +468,9 @@ const PanelUI = {
       if (aAnchor.getAttribute("tabspecific")) {
         tempPanel.setAttribute("tabspecific", true);
       }
+      if (aAnchor.getAttribute("locationspecific")) {
+        tempPanel.setAttribute("locationspecific", true);
+      }
       if (this._disableAnimations) {
         tempPanel.setAttribute("animate", "false");
       }

browser/components/customizableui/test/browser.ini

@@ -163,6 +163,7 @@ skip-if = os == "mac" # no toggle-able menubar on macOS.
 skip-if = verify
 [browser_palette_labels.js]
 [browser_panel_keyboard_navigation.js]
+[browser_panel_locationSpecific.js]
 [browser_panel_toggle.js]
 [browser_panelUINotifications.js]
 [browser_panelUINotifications_fullscreen.js]

browser/components/customizableui/test/browser_panel_locationSpecific.js

@@ -0,0 +1,83 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/*
+ * This test creates multiple panels, one that has been tagged as location specific
+ * and one that isn't. When the location changes, the specific panel should close.
+ * The non-specific panel should remain open.
+ *
+ */
+
+function synthesizeKeys(input) {
+  for (const key of input.split("")) {
+    EventUtils.synthesizeKey(key, {});
+  }
+}
+
+add_task(async function() {
+  let specificPanel = document.createXULElement("panel");
+  specificPanel.setAttribute("locationspecific", "true");
+  specificPanel.setAttribute("noautohide", "true");
+  specificPanel.height = "100px";
+  specificPanel.width = "100px";
+
+  let generalPanel = document.createXULElement("panel");
+  generalPanel.setAttribute("noautohide", "true");
+  generalPanel.height = "100px";
+  generalPanel.width = "100px";
+
+  let anchor = document.getElementById(CustomizableUI.AREA_NAVBAR);
+
+  anchor.appendChild(specificPanel);
+  anchor.appendChild(generalPanel);
+  is(specificPanel.state, "closed", "specificPanel starts as closed");
+  is(generalPanel.state, "closed", "generalPanel starts as closed");
+
+  let specificPanelPromise = BrowserTestUtils.waitForEvent(
+    specificPanel,
+    "popupshown"
+  );
+
+  specificPanel.openPopupAtScreen(0, 0);
+
+  await specificPanelPromise;
+  is(specificPanel.state, "open", "specificPanel has been opened");
+
+  let generalPanelPromise = BrowserTestUtils.waitForEvent(
+    generalPanel,
+    "popupshown"
+  );
+
+  generalPanel.openPopupAtScreen(100, 0);
+
+  await generalPanelPromise;
+  is(generalPanel.state, "open", "generalPanel has been opened");
+
+  let specificPanelHiddenPromise = BrowserTestUtils.waitForEvent(
+    specificPanel,
+    "popuphidden"
+  );
+
+  // Trigger a url change through Ctrl+l
+  EventUtils.synthesizeKey("l", { ctrlKey: true });
+  synthesizeKeys("http://mochi.test:8888/#0");
+  EventUtils.synthesizeKey("KEY_Enter", {});
+
+  await specificPanelHiddenPromise;
+
+  is(
+    specificPanel.state,
+    "closed",
+    "specificPanel panel is closed after location change"
+  );
+  is(
+    generalPanel.state,
+    "open",
+    "generalPanel is still open after location change"
+  );
+
+  specificPanel.remove();
+  generalPanel.remove();
+});

browser/components/pocket/content/SaveToPocket.jsm

@@ -45,8 +45,8 @@ var PocketCustomizableWidget = {
       l10nId: "save-to-pocket-button",
       type: "view",
       viewId: "PanelUI-savetopocket",
-      // This closes any open Pocket panels if you change tabs.
+      // This closes any open Pocket panels if you change location.
-      tabSpecific: true,
+      locationSpecific: true,
       onViewShowing(aEvent) {
         let panelView = aEvent.target;
         let panelNode = panelView.querySelector(

browser/components/pocket/test/browser_pocket_button_icon_state.js

@@ -8,22 +8,33 @@ ChromeUtils.defineModuleGetter(
   "chrome://pocket/content/SaveToPocket.jsm"
 );
 
-add_task(async function() {
+function test_runner(test) {
-  let tab = await BrowserTestUtils.openNewForegroundTab(
+  let testTask = async () => {
-    gBrowser,
+    // Before each
-    "https://example.com/browser/browser/components/pocket/test/test.html"
+    const sandbox = sinon.createSandbox();
-  );
 
-  // We're faking a logged in test, so initially we need to fake the logged in state.
+    // We're faking logged in tests, so initially we need to fake the logged in state.
-  const loggedInStub = sinon
+    sandbox.stub(pktApi, "isUserLoggedIn").callsFake(() => true);
-    .stub(pktApi, "isUserLoggedIn")
-    .callsFake(() => true);
-  // Also we cannot actually make remote requests, so make sure we stub any functions
-  // we need that that make requests to api.getpocket.com.
-  const addLinkStub = sinon.stub(pktApi, "addLink").callsFake(() => true);
 
+    // Also we cannot actually make remote requests, so make sure we stub any functions
+    // we need that that make requests to api.getpocket.com.
+    sandbox.stub(pktApi, "addLink").callsFake(() => true);
+
+    try {
+      await test({ sandbox });
+    } finally {
+      // After each
+      sandbox.restore();
+    }
+  };
+
+  // Copy the name of the test function to identify the test
+  Object.defineProperty(testTask, "name", { value: test.name });
+  add_task(testTask);
+}
+
+async function isPocketPanelShown() {
   info("clicking on pocket button in toolbar");
-  let pocketButton = document.getElementById("save-to-pocket-button");
   // The panel is created on the fly, so we can't simply wait for focus
   // inside it.
   let pocketPanelShowing = BrowserTestUtils.waitForEvent(
@@ -31,9 +42,18 @@ add_task(async function() {
     "popupshown",
     true
   );
-  pocketButton.click();
+  return pocketPanelShowing;
-  await pocketPanelShowing;
+}
 
+async function isPocketPanelHidden() {
+  let pocketPanelHidden = BrowserTestUtils.waitForEvent(
+    document,
+    "popuphidden"
+  );
+  return pocketPanelHidden;
+}
+
+function fakeSavingPage() {
   // Because we're not actually logged into a remote Pocket account,
   // and because we're not actually saving anything,
   // we fake it, instead, by calling the function we care about.
@@ -41,26 +61,78 @@ add_task(async function() {
   // This fakes the button from just opened, to also pocketed,
   // we currently expect both from a save.
   SaveToPocket.updateToolbarNodeState(window);
+}
 
+function checkPanelOpen() {
+  let pocketButton = document.getElementById("save-to-pocket-button");
   // The Pocket button should be set to open.
   is(pocketButton.open, true, "Pocket button is open");
   is(pocketButton.getAttribute("pocketed"), "true", "Pocket item is pocketed");
+}
 
-  let pocketPanelHidden = BrowserTestUtils.waitForEvent(
+function checkPanelClosed() {
-    document,
+  let pocketButton = document.getElementById("save-to-pocket-button");
-    "popuphidden"
+  // Something should have closed the Pocket panel, icon should no longer be red.
-  );
-
-  // Mochitests start with an open tab, so use that to trigger a tab change.
-  await BrowserTestUtils.switchTab(gBrowser, gBrowser.tabs[0]);
-
-  await pocketPanelHidden;
-
-  // Opening a new tab should have closed the Pocket panel, icon should no longer be red.
   is(pocketButton.open, false, "Pocket button is closed");
   is(pocketButton.getAttribute("pocketed"), "", "Pocket item is not pocketed");
+}
+
+function synthesizeKeys(input) {
+  for (const key of input.split("")) {
+    EventUtils.synthesizeKey(key, {});
+  }
+}
+
+test_runner(async function test_pocketButtonState_changeTabs({ sandbox }) {
+  let tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "https://example.com/browser/browser/components/pocket/test/test.html"
+  );
+
+  let pocketPanelShown = isPocketPanelShown();
+  let pocketButton = document.getElementById("save-to-pocket-button");
+  pocketButton.click();
+  await pocketPanelShown;
+  fakeSavingPage();
+
+  // Testing the panel states.
+  checkPanelOpen();
+
+  let pocketPanelHidden = isPocketPanelHidden();
+  // Mochitests start with an open tab, so use that to trigger a tab change.
+  await BrowserTestUtils.switchTab(gBrowser, gBrowser.tabs[0]);
+  await pocketPanelHidden;
+
+  // Testing the panel states.
+  checkPanelClosed();
+
+  BrowserTestUtils.removeTab(tab);
+});
+
+test_runner(async function test_pocketButtonState_changeLocation({ sandbox }) {
+  let tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "https://example.com/browser/browser/components/pocket/test/test.html"
+  );
+
+  let pocketPanelShown = isPocketPanelShown();
+  let pocketButton = document.getElementById("save-to-pocket-button");
+  pocketButton.click();
+  await pocketPanelShown;
+  fakeSavingPage();
+
+  // Testing the panel states.
+  checkPanelOpen();
+
+  let pocketPanelHidden = isPocketPanelHidden();
+  // Trigger a url change through Ctrl+l
+  EventUtils.synthesizeKey("l", { ctrlKey: true });
+  synthesizeKeys("about:robots");
+  EventUtils.synthesizeKey("KEY_Enter", {});
+  await pocketPanelHidden;
+
+  // Testing the panel states.
+  checkPanelClosed();
 
-  loggedInStub.restore();
-  addLinkStub.restore();
   BrowserTestUtils.removeTab(tab);
 });