fix and simplify openURL + onTabReady + message from popup

background/background.js

@@ -49,10 +49,26 @@ window.API_METHODS = Object.assign(window.API_METHODS || {}, {
 
   openEditor,
 
-  // exposed for stuff that requires followup sendMessage() like popup::openSettings
-  // that would fail otherwise if another extension forced the tab to open
-  // in the foreground thus auto-closing the popup (in Chrome)
-  openURL,
+  /* Same as openURL, the only extra prop in `opts` is `message` - it'll be sent when the tab is ready,
+  which is needed in the popup, otherwise another extension could force the tab to open in foreground
+  thus auto-closing the popup (in Chrome at least) and preventing the sendMessage code from running */
+  openURL(opts) {
+    const {message} = opts;
+    return openURL(opts) // will pass the resolved value untouched when `message` is absent or falsy
+      .then(message && (tab => tab.status === 'complete' ? tab : onTabReady(tab)))
+      .then(message && (tab => msg.sendTab(tab.id, opts.message)));
+    function onTabReady(tab) {
+      return new Promise((resolve, reject) =>
+        setTimeout(function ping(numTries = 10, delay = 100) {
+          msg.sendTab(tab.id, {method: 'ping'})
+            .catch(() => false)
+            .then(pong => pong
+              ? resolve(tab)
+              : numTries && setTimeout(ping, delay, numTries - 1, delay * 1.5) ||
+                reject('timeout'));
+        }));
+    }
+  },
 
   optionsCustomizeHotkeys() {
     return browser.runtime.openOptionsPage()

js/messaging.js

@@ -1,4 +1,4 @@
-/* exported getActiveTab onTabReady stringAsRegExp openURL
+/* exported getTab getActiveTab onTabReady stringAsRegExp openURL ignoreChromeError
   getStyleWithNoCode tryRegExp sessionStorageHash download deepEqual
   closeCurrentTab capitalize CHROME_HAS_BORDER_BUG */
 /* global promisify */
@@ -125,70 +125,6 @@ function getActiveTab() {
     .then(tabs => tabs[0]);
 }
 
-/**
- * Resolves when the [just created] tab is ready for communication.
- * @param {Number|Tab} tabOrId
- * @returns {Promise<?Tab>}
- */
-function onTabReady(tabOrId) {
-  let tabId, tab;
-  if (Number.isInteger(tabOrId)) {
-    tabId = tabOrId;
-  } else {
-    tab = tabOrId;
-    tabId = tab && tab.id;
-  }
-  if (!tab) {
-    return getTab(tabId).then(onTabReady);
-  }
-  if (tab.status === 'complete') {
-    if (!FIREFOX || tab.url !== 'about:blank') {
-      return Promise.resolve(tab);
-    } else {
-      return new Promise(resolve => {
-        chrome.webNavigation.getFrame({tabId, frameId: 0}, frame => {
-          ignoreChromeError();
-          if (frame) {
-            onTabReady(tab).then(resolve);
-          } else {
-            setTimeout(() => onTabReady(tabId).then(resolve));
-          }
-        });
-      });
-    }
-  }
-  return new Promise((resolve, reject) => {
-    chrome.webNavigation.onCommitted.addListener(onCommitted);
-    chrome.webNavigation.onErrorOccurred.addListener(onErrorOccurred);
-    chrome.tabs.onRemoved.addListener(onTabRemoved);
-    chrome.tabs.onReplaced.addListener(onTabReplaced);
-    function onCommitted(info) {
-      if (info.tabId !== tabId) return;
-      unregister();
-      getTab(tab.id).then(resolve);
-    }
-    function onErrorOccurred(info) {
-      if (info.tabId !== tabId) return;
-      unregister();
-      reject();
-    }
-    function onTabRemoved(removedTabId) {
-      if (removedTabId !== tabId) return;
-      unregister();
-      reject();
-    }
-    function onTabReplaced(addedTabId, removedTabId) {
-      onTabRemoved(removedTabId);
-    }
-    function unregister() {
-      chrome.webNavigation.onCommitted.removeListener(onCommitted);
-      chrome.webNavigation.onErrorOccurred.removeListener(onErrorOccurred);
-      chrome.tabs.onRemoved.removeListener(onTabRemoved);
-      chrome.tabs.onReplaced.removeListener(onTabReplaced);
-    }
-  });
-}
-
 function urlToMatchPattern(url, ignoreSearch) {
   // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns
   if (!/^(http|https|ws|wss|ftp|data|file)$/.test(url.protocol)) {

popup/popup.js

@@ -1,4 +1,4 @@
-/* global configDialog hotkeys onTabReady msg
+/* global configDialog hotkeys msg
   getActiveTab FIREFOX URLS API onDOMready $ $$ prefs
   setupLivePrefs template t $create animateElement
   tryJSONparse CHROME_HAS_BORDER_BUG */
@@ -84,7 +84,7 @@ function initTabUrls() {
   return getActiveTab()
     .then((tab = {}) =>
       FIREFOX && tab.status === 'loading' && tab.url === 'about:blank'
-        ? onTabReady(tab)
+        ? waitForTabUrlFF(tab)
         : tab)
     .then(tab => new Promise(resolve =>
       chrome.webNavigation.getAllFrames({tabId: tab.id}, frames =>
@@ -626,18 +626,12 @@ Object.assign(handleEvent, {
 
   openURLandHide(event) {
     event.preventDefault();
-    const message = tryJSONparse(this.dataset.sendMessage);
     getActiveTab()
       .then(activeTab => API.openURL({
         url: this.href || this.dataset.href,
-        index: activeTab.index + 1
+        index: activeTab.index + 1,
+        message: tryJSONparse(this.dataset.sendMessage),
       }))
-      .then(tab => {
-        if (message) {
-          return onTabReady(tab)
-            .then(() => msg.sendTab(tab.id, message));
-        }
-      })
       .then(window.close);
   },
 
@@ -704,3 +698,18 @@ function handleDelete(id) {
     installed.appendChild(template.noStyles.cloneNode(true));
   }
 }
+
+function waitForTabUrlFF(tab) {
+  return new Promise(resolve => {
+    browser.tabs.onUpdated.addListener(...[
+      function onUpdated(tabId, info, updatedTab) {
+        if (info.url && tabId === tab.id) {
+          chrome.tabs.onUpdated.removeListener(onUpdated);
+          resolve(updatedTab);
+        }
+      },
+      ...'UpdateFilter' in browser.tabs ? [{tabId: tab.id}] : [],
+      // TODO: remove both spreads and tabId check when strict_min_version >= 61
+    ]);
+  });
+}