Bug 1439246 - Clear browser and page actions when inactive tabs navigate draft
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Sun, 18 Feb 2018 21:03:36 +0100
changeset 769044 9f48c81216b357543cbb33a57321833087c824c9
parent 768313 16e69d4df5dcef04f1d3f4b6fcf9284d321bce6f
push id103023
push userbmo:oriol-bugzilla@hotmail.com
push dateSun, 18 Mar 2018 00:38:11 +0000
bugs1439246
milestone61.0a1
Bug 1439246 - Clear browser and page actions when inactive tabs navigate MozReview-Commit-ID: 5uGHmXEiMQS
browser/components/extensions/ext-browser.js
browser/components/extensions/ext-pageAction.js
browser/components/extensions/test/browser/browser_ext_browserAction_context.js
browser/components/extensions/test/browser/browser_ext_pageAction_context.js
browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js
browser/components/extensions/test/browser/head.js
--- a/browser/components/extensions/ext-browser.js
+++ b/browser/components/extensions/ext-browser.js
@@ -136,22 +136,20 @@ global.TabContext = class extends EventE
       let nativeTab = event.target;
       this.emit("tab-select", nativeTab);
       this.emit("location-change", nativeTab);
     }
   }
 
   onLocationChange(browser, webProgress, request, locationURI, flags) {
     let gBrowser = browser.ownerGlobal.gBrowser;
-    if (browser === gBrowser.selectedBrowser) {
-      let tab = gBrowser.getTabForBrowser(browser);
-      // fromBrowse will be false in case of e.g. a hash change or history.pushState
-      let fromBrowse = !(flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
-      this.emit("location-change", tab, fromBrowse);
-    }
+    let tab = gBrowser.getTabForBrowser(browser);
+    // fromBrowse will be false in case of e.g. a hash change or history.pushState
+    let fromBrowse = !(flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
+    this.emit("location-change", tab, fromBrowse);
   }
 
   shutdown() {
     windowTracker.removeListener("progress", this);
     windowTracker.removeListener("TabSelect", this);
   }
 };
 
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -185,28 +185,27 @@ this.pageAction = class extends Extensio
     }
     this.browserPageAction.setIconURL(iconURL, window);
   }
 
   // Checks whether the tab action is shown when the specified tab becomes active.
   // Does pattern matching if necessary, and caches the result as a tab-specific value.
   // @param {XULElement} tab
   //        The tab to be checked
-  // @param [optional] {boolean} ignoreCache
-  //        If true, forces pattern matching to be reevaluated, even if there is a cached value.
-  isShown(tab, ignoreCache = false) {
+  // @return boolean
+  isShown(tab) {
     let tabData = this.tabContext.get(tab);
 
     // If there is a "show" value, return it. Can be due to show(), hide() or empty show_matches.
     if (tabData.show !== undefined) {
       return tabData.show;
     }
 
     // Otherwise pattern matching must have been configured. Do it, caching the result.
-    if (ignoreCache || tabData.patternMatching === undefined) {
+    if (tabData.patternMatching === undefined) {
       let uri = tab.linkedBrowser.currentURI;
       tabData.patternMatching = tabData.showMatches.matches(uri) && !tabData.hideMatches.matches(uri);
     }
     return tabData.patternMatching;
   }
 
   getIconData(icons) {
     let getIcon = size => {
@@ -279,29 +278,48 @@ this.pageAction = class extends Extensio
                                                      popup.panel);
       TelemetryStopwatch.finish(popupOpenTimingHistogram, this);
     } else {
       TelemetryStopwatch.cancel(popupOpenTimingHistogram, this);
       this.emit("click", tab);
     }
   }
 
+  /**
+   * Updates the `tabData` for any location change, however it only updates the button
+   * when the selected tab has a location change, or the selected tab has changed.
+   *
+   * @param {string} eventType
+   *        The type of the event, should be "location-change".
+   * @param {XULElement} tab
+   *        The tab whose location changed, or which has become selected.
+   * @param {boolean} [fromBrowse]
+   *        - `true` if navigation occurred in `tab`.
+   *        - `false` if the location changed but no navigation occurred, e.g. due to
+               a hash change or `history.pushState`.
+   *        - Omitted if TabSelect has occurred, tabData does not need to be updated.
+   */
   handleLocationChange(eventType, tab, fromBrowse) {
-    // fromBrowse can have three values:
-    // - true: navigation occurred in the active tab
-    // - false: the location changed in the active tab but no navigation occurred
-    // - undefined: an inactive tab has become active
     if (fromBrowse === true) {
+      // Clear tab data on navigation.
       this.tabContext.clear(tab);
+    } else if (fromBrowse === false) {
+      // Clear pattern matching cache when URL changes.
+      let tabData = this.tabContext.get(tab);
+      if (tabData.patternMatching !== undefined) {
+        tabData.patternMatching = undefined;
+      }
     }
 
-    // isShown will do pattern matching (if necessary) and store the result
-    // so that updateButton knows whether the page action should be shown.
-    this.isShown(tab, fromBrowse !== undefined);
-    this.updateButton(tab.ownerGlobal);
+    if (tab.selected) {
+      // isShown will do pattern matching (if necessary) and store the result
+      // so that updateButton knows whether the page action should be shown.
+      this.isShown(tab);
+      this.updateButton(tab.ownerGlobal);
+    }
   }
 
   getAPI(context) {
     let {extension} = context;
 
     const {tabManager} = extension;
     const pageAction = this;
 
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_context.js
@@ -592,8 +592,98 @@ add_task(async function testPropertyRemo
           browser.browserAction.setBadgeBackgroundColor({color: null});
           await expectGlobals(details[0]);
           expect(details[0]);
         },
       ];
     },
   });
 });
+
+add_task(async function testNavigationClearsData() {
+  let url = "http://example.com/";
+  let default_title = "Default title";
+  let tab_title = "Tab title";
+
+  let {Management: {global: {tabTracker}}} = ChromeUtils.import("resource://gre/modules/Extension.jsm", {});
+  let extension, tabs = [];
+  async function addTab(...args) {
+    let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, ...args);
+    tabs.push(tab);
+    return tab;
+  }
+  async function sendMessage(method, param, expect, msg) {
+    extension.sendMessage({method, param, expect, msg});
+    await extension.awaitMessage("done");
+  }
+  async function expectTabSpecificData(tab, msg) {
+    let tabId = tabTracker.getId(tab);
+    await sendMessage("getBadgeText", {tabId}, "foo", msg);
+    await sendMessage("getTitle", {tabId}, tab_title, msg);
+  }
+  async function expectDefaultData(tab, msg) {
+    let tabId = tabTracker.getId(tab);
+    await sendMessage("getBadgeText", {tabId}, "", msg);
+    await sendMessage("getTitle", {tabId}, default_title, msg);
+  }
+  async function setTabSpecificData(tab) {
+    let tabId = tabTracker.getId(tab);
+    await expectDefaultData(tab, "Expect default data before setting tab-specific data.");
+    await sendMessage("setBadgeText", {tabId, text: "foo"});
+    await sendMessage("setTitle", {tabId, title: tab_title});
+    await expectTabSpecificData(tab, "Expect tab-specific data after setting it.");
+  }
+
+  info("Load a tab before installing the extension");
+  let tab1 = await addTab(url, true, true);
+
+  extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      browser_action: {default_title},
+    },
+    background: function() {
+      browser.test.onMessage.addListener(async ({method, param, expect, msg}) => {
+        let result = await browser.browserAction[method](param);
+        if (expect !== undefined) {
+          browser.test.assertEq(expect, result, msg);
+        }
+        browser.test.sendMessage("done");
+      });
+    },
+  });
+  await extension.startup();
+
+  info("Set tab-specific data to the existing tab.");
+  await setTabSpecificData(tab1);
+
+  info("Add a hash. Does not cause navigation.");
+  await navigateTab(tab1, url + "#hash");
+  await expectTabSpecificData(tab1, "Adding a hash does not clear tab-specific data");
+
+  info("Remove the hash. Causes navigation.");
+  await navigateTab(tab1, url);
+  await expectDefaultData(tab1, "Removing hash clears tab-specific data");
+
+  info("Open a new tab, set tab-specific data to it.");
+  let tab2 = await addTab("about:newtab", false, false);
+  await setTabSpecificData(tab2);
+
+  info("Load a page in that tab.");
+  await navigateTab(tab2, url);
+  await expectDefaultData(tab2, "Loading a page clears tab-specific data.");
+
+  info("Set tab-specific data.");
+  await setTabSpecificData(tab2);
+
+  info("Push history state. Does not cause navigation.");
+  await historyPushState(tab2, url + "/path");
+  await expectTabSpecificData(tab2, "history.pushState() does not clear tab-specific data");
+
+  info("Navigate when the tab is not selected");
+  gBrowser.selectedTab = tab1;
+  await navigateTab(tab2, url);
+  await expectDefaultData(tab2, "Navigating clears tab-specific data, even when not selected.");
+
+  for (let tab of tabs) {
+    await BrowserTestUtils.removeTab(tab);
+  }
+  await extension.unload();
+});
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
@@ -200,54 +200,46 @@ add_task(async function testTabSwitchCon
 });
 
 add_task(async function testNavigationClearsData() {
   let url = "http://example.com/";
   let default_title = "Default title";
   let tab_title = "Tab title";
 
   let {Management: {global: {tabTracker}}} = ChromeUtils.import("resource://gre/modules/Extension.jsm", {});
-  let extension, tab, tabId, tabs = [];
+  let extension, tabs = [];
   async function addTab(...args) {
-    tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, ...args);
-    tabId = tabTracker.getId(tab);
+    let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, ...args);
     tabs.push(tab);
-  }
-  async function locationChange(url, task) {
-    let locationChanged = BrowserTestUtils.waitForLocationChange(gBrowser, url);
-    await ContentTask.spawn(tab.linkedBrowser, url, task);
-    await locationChanged;
-  }
-  function setUrl(url) {
-    return locationChange(url, (url) => { content.location.href = url; });
-  }
-  function historyPushState(url) {
-    return locationChange(url, (url) => { content.history.pushState(null, null, url); });
+    return tab;
   }
   async function sendMessage(method, param, expect, msg) {
     extension.sendMessage({method, param, expect, msg});
     await extension.awaitMessage("done");
   }
-  async function expectTabSpecificData(msg) {
+  async function expectTabSpecificData(tab, msg) {
+    let tabId = tabTracker.getId(tab);
     await sendMessage("isShown", {tabId}, true, msg);
     await sendMessage("getTitle", {tabId}, tab_title, msg);
   }
-  async function expectDefaultData(msg) {
+  async function expectDefaultData(tab, msg) {
+    let tabId = tabTracker.getId(tab);
     await sendMessage("isShown", {tabId}, false, msg);
     await sendMessage("getTitle", {tabId}, default_title, msg);
   }
-  async function setTabSpecificData() {
-    await expectDefaultData("Expect default data before setting tab-specific data.");
+  async function setTabSpecificData(tab) {
+    let tabId = tabTracker.getId(tab);
+    await expectDefaultData(tab, "Expect default data before setting tab-specific data.");
     await sendMessage("show", tabId);
     await sendMessage("setTitle", {tabId, title: tab_title});
-    await expectTabSpecificData("Expect tab-specific data after setting it.");
+    await expectTabSpecificData(tab, "Expect tab-specific data after setting it.");
   }
 
   info("Load a tab before installing the extension");
-  await addTab(url, true, true);
+  let tab1 = await addTab(url, true, true);
 
   extension = ExtensionTestUtils.loadExtension({
     manifest: {
       page_action: {default_title},
     },
     background: function() {
       browser.test.onMessage.addListener(async ({method, param, expect, msg}) => {
         let result = await browser.pageAction[method](param);
@@ -256,38 +248,43 @@ add_task(async function testNavigationCl
         }
         browser.test.sendMessage("done");
       });
     },
   });
   await extension.startup();
 
   info("Set tab-specific data to the existing tab.");
-  await setTabSpecificData();
+  await setTabSpecificData(tab1);
 
   info("Add a hash. Does not cause navigation.");
-  await setUrl(url + "#hash");
-  await expectTabSpecificData("Adding a hash does not clear tab-specific data");
+  await navigateTab(tab1, url + "#hash");
+  await expectTabSpecificData(tab1, "Adding a hash does not clear tab-specific data");
 
   info("Remove the hash. Causes navigation.");
-  await setUrl(url);
-  await expectDefaultData("Removing hash clears tab-specific data");
+  await navigateTab(tab1, url);
+  await expectDefaultData(tab1, "Removing hash clears tab-specific data");
 
   info("Open a new tab, set tab-specific data to it.");
-  await addTab("about:newtab", false, false);
-  await setTabSpecificData();
+  let tab2 = await addTab("about:newtab", false, false);
+  await setTabSpecificData(tab2);
 
   info("Load a page in that tab.");
-  await setUrl(url);
-  await expectDefaultData("Loading a page clears tab-specific data.");
+  await navigateTab(tab2, url);
+  await expectDefaultData(tab2, "Loading a page clears tab-specific data.");
 
   info("Set tab-specific data.");
-  await setTabSpecificData();
+  await setTabSpecificData(tab2);
 
   info("Push history state. Does not cause navigation.");
-  await historyPushState(url + "/path");
-  await expectTabSpecificData("history.pushState() does not clear tab-specific data");
+  await historyPushState(tab2, url + "/path");
+  await expectTabSpecificData(tab2, "history.pushState() does not clear tab-specific data");
+
+  info("Navigate when the tab is not selected");
+  gBrowser.selectedTab = tab1;
+  await navigateTab(tab2, url);
+  await expectDefaultData(tab2, "Navigating clears tab-specific data, even when not selected.");
 
   for (let tab of tabs) {
     await BrowserTestUtils.removeTab(tab);
   }
   await extension.unload();
 });
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js
@@ -59,24 +59,16 @@ let urls = [
 ];
 
 function getId(tab) {
   let {Management: {global: {tabTracker}}} = ChromeUtils.import("resource://gre/modules/Extension.jsm", {});
   getId = tabTracker.getId.bind(tabTracker); // eslint-disable-line no-func-assign
   return getId(tab);
 }
 
-async function historyPushState(tab, url) {
-  let locationChanged = BrowserTestUtils.waitForLocationChange(gBrowser, url);
-  await ContentTask.spawn(tab.linkedBrowser, url, (url) => {
-    content.history.pushState(null, null, url);
-  });
-  await locationChanged;
-}
-
 async function check(extension, tab, expected, msg) {
   let widgetId = makeWidgetId(extension.id);
   let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(widgetId);
   is(gBrowser.selectedTab, tab, `tab ${tab.linkedBrowser.currentURI.spec} is selected`);
   let button = document.getElementById(pageActionId);
   // Sometimes we're hidden, sometimes a parent is hidden via css (e.g. about pages)
   let hidden = button === null || button.hidden ||
     window.getComputedStyle(button).display == "none";
@@ -132,17 +124,17 @@ add_task(async function test_pageAction_
       await BrowserTestUtils.removeTab(initialTab);
       await BrowserTestUtils.removeTab(installTab);
       await extension.unload();
     }
   }
 });
 
 add_task(async function test_pageAction_history() {
-  info("Check match patterns are reevaluated when using history.pushState");
+  info("Check match patterns are reevaluated when using history.pushState or navigating");
   let url1 = "http://example.com/";
   let url2 = url1 + "path/";
   let extension = getExtension({
     "show_matches": [url1],
     "hide_matches": [url2],
   });
   await extension.startup();
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, url1, true, true);
@@ -155,17 +147,26 @@ add_task(async function test_pageAction_
   info("Use hide()");
   await sendMessage(extension, "hide", getId(tab));
   await check(extension, tab, false, "page action is still hidden");
 
   info("Use history.pushState to revert to first url");
   await historyPushState(tab, url1);
   await check(extension, tab, false, "hide() has more precedence than pattern matching");
 
+  info("Select another tab");
+  let tab2 = await BrowserTestUtils.openNewForegroundTab(gBrowser, url1, true, true);
+
+  info("Perform navigation in the old tab");
+  await navigateTab(tab, url1);
+  await sendMessage(extension, "isShown", {tabId: getId(tab)}, true,
+                    "Navigating undoes hide(), even when the tab is not selected.");
+
   await BrowserTestUtils.removeTab(tab);
+  await BrowserTestUtils.removeTab(tab2);
   await extension.unload();
 });
 
 add_task(async function test_pageAction_all_urls() {
   info("Check <all_urls> is not allowed in hide_matches");
   let extension = getExtension({
     "show_matches": ["*://mochi.test/*"],
     "hide_matches": ["<all_urls>"],
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -16,16 +16,17 @@
  *          openToolsMenu closeToolsMenu
  *          imageBuffer imageBufferFromDataURI
  *          getListStyleImage getPanelForNode
  *          awaitExtensionPanel awaitPopupResize
  *          promiseContentDimensions alterContent
  *          promisePrefChangeObserved openContextMenuInFrame
  *          promiseAnimationFrame getCustomizableUIPanelID
  *          awaitEvent BrowserWindowIterator
+ *          navigateTab historyPushState
  */
 
 // There are shutdown issues for which multiple rejections are left uncaught.
 // This bug should be fixed, but for the moment this directory is whitelisted.
 //
 // NOTE: Entire directory whitelisting should be kept to a minimum. Normally you
 //       should use "expectUncaughtRejection" to flag individual failures.
 const {PromiseTestUtils} = ChromeUtils.import("resource://testing-common/PromiseTestUtils.jsm", {});
@@ -495,8 +496,22 @@ function* BrowserWindowIterator() {
   let windowsEnum = Services.wm.getEnumerator("navigator:browser");
   while (windowsEnum.hasMoreElements()) {
     let currentWindow = windowsEnum.getNext();
     if (!currentWindow.closed) {
       yield currentWindow;
     }
   }
 }
+
+async function locationChange(tab, url, task) {
+  let locationChanged = BrowserTestUtils.waitForLocationChange(gBrowser, url);
+  await ContentTask.spawn(tab.linkedBrowser, url, task);
+  return locationChanged;
+}
+
+function navigateTab(tab, url) {
+  return locationChange(tab, url, (url) => { content.location.href = url; });
+}
+
+function historyPushState(tab, url) {
+  return locationChange(tab, url, (url) => { content.history.pushState(null, null, url); });
+}