Fix multi-window cases

Previously all code would get the 'active tab' from the first window, even if another window was focused. Also when activating a tab it would only make it active in its window, without necessarily focusing that same window. This adds methods that handle both these cases properly.

extension/background/intentRunner.js

@@ -1,4 +1,4 @@
-/* globals log, intentParser, telemetry, catcher, searching */
+/* globals log, intentParser, telemetry, catcher, searching, browserUtil */
 // This gets used elsewhere as a namespace for the intent modules:
 this.intents = {};
 
@@ -45,7 +45,7 @@ this.intentRunner = (function() {
 
     async failedAutoplay(tab) {
       this.keepPopup();
-      await browser.tabs.update(tab.id, { active: true });
+      this.makeTabActive(tab);
       await browser.runtime.sendMessage({ type: "displayAutoplayFailure" });
     }
 
@@ -111,6 +111,14 @@ this.intentRunner = (function() {
       });
     }
 
+    activeTab() {
+      return browserUtil.activeTab();
+    }
+
+    makeTabActive(tab) {
+      return browserUtil.makeTabActive(tab);
+    }
+
     onError(message) {
       // Can be overridden
     }

extension/background/main.js

@@ -1,4 +1,4 @@
-/* globals intentParser, intentRunner, intentExamples, log, intents, telemetry, util, buildSettings, settings */
+/* globals intentParser, intentRunner, intentExamples, log, intents, telemetry, util, buildSettings, settings, browserUtil */
 
 this.main = (function() {
   const exports = {};
@@ -56,7 +56,8 @@ this.main = (function() {
       }
       return browser.tabs.sendMessage(recorderTabId, message);
     } else if (message.type === "makeRecorderActive") {
-      browser.tabs.update(recorderTabId, { active: true });
+      // FIXME: consider focusing the window too
+      browserUtil.makeTabActive(recorderTabId);
       return null;
     }
     log.error(
@@ -116,13 +117,14 @@ this.main = (function() {
       }
     }
     let tab;
-    const activeTabId = (await browser.tabs.query({ active: true }))[0].id;
+    const activeTab = await browserUtil.activeTab();
     const existing = await browser.tabs.query({ url: RECORDER_URL });
     if (existing.length) {
       if (existing.length > 1) {
         browser.tabs.remove(existing.slice(1).map(e => e.id));
       }
       tab = existing[0];
+      // FIXME: make sure window is focused?
       await browser.tabs.update(tab.id, {
         url: RECORDER_URL,
         active: true,
@@ -145,7 +147,7 @@ this.main = (function() {
       } catch (e) {}
       await util.sleep(100);
     }
-    await browser.tabs.update(activeTabId, { active: true });
+    await browserUtil.makeTabActive(activeTab);
   }
 
   async function launchOnboarding() {

extension/background/serviceList.js

@@ -1,4 +1,4 @@
-/* globals content, util, settings */
+/* globals content, util, settings, browserUtil */
 
 this.services = {};
 
@@ -132,7 +132,7 @@ this.serviceList = (function() {
       if (!this.tab) {
         throw new Error("No tab to activate");
       }
-      await browser.tabs.update(this.tab.id, { active: true });
+      this.context.makeTabActive(this.tab.id);
     }
 
     get matchPatterns() {
@@ -169,7 +169,7 @@ this.serviceList = (function() {
         }
       }
       if (activate) {
-        await browser.tabs.update(tabs[best].id, { active: activate });
+        await this.context.makeTabActive(tabs[best]);
       }
       return { created: false, tab: tabs[best] };
     }
@@ -231,7 +231,7 @@ this.serviceList = (function() {
   };
 
   exports.detectServiceFromActiveTab = async function(services) {
-    const tab = (await browser.tabs.query({ active: true }))[0];
+    const tab = await browserUtil.activeTab();
     for (const name in services) {
       const service = services[name];
       if (tab.url.startsWith(service.baseUrl)) {

extension/browserUtil.js

@@ -0,0 +1,24 @@
+this.browserUtil = (function() {
+  const exports = {};
+  exports.activeTab = async function activeTab() {
+    return (await browser.tabs.query({
+      active: true,
+      lastFocusedWindow: true,
+    }))[0];
+  };
+
+  exports.makeTabActive = async function makeTabActive(tab) {
+    let tabId;
+    if (typeof tab === "string" || typeof tab === "number") {
+      // then it's a tab ID
+      tabId = tab;
+      tab = await browser.tabs.get(tabId);
+    } else {
+      tabId = tab.id;
+    }
+    await browser.tabs.update(tabId, { active: true });
+    await browser.windows.update(tab.windowId, { focused: true });
+  };
+
+  return exports;
+})();

extension/intents/aboutPage/aboutPage.js

@@ -49,13 +49,11 @@ intents.aboutPage = (function() {
     (show | open |) comments
     `,
     async run(context) {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
-      // FIXME: get the "best" URL using rel=self or og:url, etc.
+      const activeTab = await context.activeTab();
       const url = (await pageMetadata.getMetadata(activeTab.id)).canonical;
       let results = (await hnSearchResults(url)).concat(
         await redditSearchResults(url)
       );
-      console.log("results", results);
       results = results.filter(r => {
         return r.num_comments > 0;
       });
@@ -65,7 +63,6 @@ intents.aboutPage = (function() {
         e.displayMessage = "Nobody has commented on this article";
         throw e;
       }
-      console.log("Comment results:", results);
       if (results.length > 1) {
         context.displayText(
           `${results.length} comment threads found. Use "next comments" to see more`
@@ -85,7 +82,7 @@ intents.aboutPage = (function() {
     previous (result | comments | comment) [move=previous]
     `,
     async run(context) {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+      const activeTab = await context.activeTab();
       const results = tabResults.get(activeTab.id);
       if (!results) {
         const e = new Error("No comment results for this tab");

extension/intents/bookmarks/bookmarks.js

@@ -57,7 +57,7 @@ this.intents.bookmarks = (function() {
       const id = matches[0].item;
       const url = bookmarksById.get(id);
       if (context.parameters.tab === "this") {
-        const activeTab = (await browser.tabs.query({ active: true }))[0];
+        const activeTab = await context.activeTab();
         await browser.tabs.update(activeTab.id, { url });
       } else {
         await browser.tabs.create({ url });

extension/intents/find/find.js

@@ -8,8 +8,8 @@ this.intents.find = (function() {
     (find | bring me to) (my | the |) [query] (tab |)
     go (to | to the |) [query] tab
     `,
-    async run(desc) {
-      const query = desc.slots.query;
+    async run(context) {
+      const query = context.slots.query;
       log.info("the most likely query text is:", query);
 
       // Fuse options
@@ -63,9 +63,7 @@ this.intents.find = (function() {
         throw e;
       }
       const topMatch = parseInt(matches[0].item);
-      await browser.tabs.update(topMatch, {
-        active: true,
-      });
+      await context.makeTabActive(topMatch);
     },
   });
 })();

extension/intents/navigation/navigation.js

@@ -75,7 +75,7 @@ this.intents.navigation = (function() {
     `,
     examples: ["Translate this page"],
     async run(context) {
-      const tab = (await browser.tabs.query({ active: true }))[0];
+      const tab = await context.activeTab();
       const translation = `https://translate.google.com/translate?hl=&sl=auto&tl=en&u=${encodeURIComponent(
         tab.url
       )}`;
@@ -90,7 +90,7 @@ this.intents.navigation = (function() {
     `,
     examples: ["Translate selection"],
     async run(context) {
-      const tab = (await browser.tabs.query({ active: true }))[0];
+      const tab = await context.activeTab();
       const selection = await pageMetadata.getSelection(tab.id);
       if (!selection || !selection.text) {
         const e = new Error("No text selected");

extension/intents/notes/notes.js

@@ -27,7 +27,7 @@ this.intents.notes = (function() {
     (write | add | make) (notes |) (here | this page | this tab)
     `,
     async run(context) {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+      const activeTab = await context.activeTab();
       const tabId = activeTab.id;
       await content.lazyInject(tabId, SCRIPT);
       const failureMessage = await browser.tabs.sendMessage(tabId, {
@@ -50,7 +50,7 @@ this.intents.notes = (function() {
     `,
     async run(context) {
       await checkHasTab();
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+      const activeTab = await context.activeTab();
       const metadata = await pageMetadata.getMetadata(activeTab.id);
       const success = await browser.tabs.sendMessage(writingTabId, {
         type: "addLink",
@@ -96,7 +96,7 @@ this.intents.notes = (function() {
         e.displayMessage = "You have not set a tab to write";
         throw e;
       }
-      await browser.tabs.update(writingTabId, { active: true });
+      await context.makeTabActive(writingTabId);
     },
   });
 })();

extension/intents/read/read.js

@@ -1,10 +1,10 @@
-/* globals util, content, log */
+/* globals util, content, log, browserUtil */
 
 this.intents.read = (function() {
   const exports = {};
 
   exports.stopReading = async function() {
-    const activeTab = (await browser.tabs.query({ active: true }))[0];
+    const activeTab = await browserUtil.activeTab();
     if (!activeTab) {
       return false;
     }
@@ -43,8 +43,8 @@ this.intents.read = (function() {
     match: `
     read (this |) (tab | page |)
     `,
-    async run(desc) {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+    async run(context) {
+      const activeTab = await context.activeTab();
       if (!activeTab.url.startsWith("about:reader")) {
         try {
           await browser.tabs.toggleReaderMode();
@@ -83,8 +83,8 @@ this.intents.read = (function() {
     match: `
     stop reading (this |) (tab | page |)
     `,
-    async run(desc) {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+    async run(context) {
+      const activeTab = await context.activeTab();
       if (!activeTab.url.startsWith("about:reader")) {
         const e = new Error(`Not a Reader Mode page`);
         e.displayMessage = "Page isn't narrating";

extension/intents/search/search.js

@@ -95,7 +95,7 @@ this.intents.search = (function() {
       }
       if (lastSearchIndex >= lastSearchInfo.searchResults.length - 1) {
         const tabId = await openSearchTab();
-        await browser.tabs.update(tabId, { active: true });
+        await context.makeTabActive(tabId);
         return;
       }
       lastSearchIndex++;
@@ -110,7 +110,7 @@ this.intents.search = (function() {
         // eslint-disable-next-line require-atomic-updates
         lastTabId = tab.id;
       } else {
-        await browser.tabs.update(lastTabId, { url: item.url, active: true });
+        await context.makeTabActive(lastTabId);
       }
     },
   });
@@ -123,7 +123,7 @@ this.intents.search = (function() {
     `,
     async run(context) {
       const tabId = await openSearchTab();
-      await browser.tabs.update(tabId, { active: true });
+      await context.makeTabActive(tabId);
     },
   });
 })();

extension/intents/tabs/tabs.js

@@ -6,8 +6,8 @@ this.intents.tabs = (function() {
     close tab
     close this tab
     `,
-    async run(desc) {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+    async run(context) {
+      const activeTab = await context.activeTab();
       await browser.tabs.remove(activeTab.id);
     },
   });

extension/manifest.json.ejs

@@ -30,6 +30,7 @@
       "catcher.js",
       "settings.js",
       "js/vendor/fuse.js",
+      "browserUtil.js",
       "background/pageMetadata.js",
       "background/main.js",
       "background/voiceSchema.js",

extension/services/readerMode/readerMode.js

@@ -8,7 +8,7 @@ this.services.readerMode = (function() {
     }
 
     async unpause() {
-      const activeTab = (await browser.tabs.query({ active: true }))[0];
+      const activeTab = await this.context.activeTab();
       if (!activeTab.url.startsWith("about:reader")) {
         const e = new Error("Cannot unpause a non-reader tab");
         e.displayMessage = "Cannot unpause";

extension/services/spotify/spotify.js

@@ -8,13 +8,12 @@ this.services.spotify = (function() {
       if (this.tabCreated) {
         const isAudible = await this.pollTabAudible(this.tab.id, 2000);
         if (!isAudible) {
-          const activeTabId = (await browser.tabs.query({ active: true }))[0]
-            .id;
-          await browser.tabs.update(this.tab.id, { active: true });
+          const activeTabId = (await this.context.activeTab()).id;
+          this.context.makeTabActive(this.tab);
           const nowAudible = await this.pollTabAudible(this.tab.id, 1000);
           if (nowAudible) {
             if (this.tab.id !== activeTabId) {
-              await browser.tabs.update(activeTabId, { active: true });
+              this.context.makeTabActive(activeTabId);
             }
           } else {
             this.context.failedAutoplay(this.tab);

extension/services/youtube/youtube.js

@@ -71,7 +71,7 @@ this.services.youtube = (function() {
       }
       let tabs = await this.getAllTabs({ audible: true });
       if (!tabs.length) {
-        const currentTab = (await browser.tabs.query({ active: true }))[0];
+        const currentTab = await this.context.activeTab();
         if (currentTab.url.startsWith(this.baseUrl)) {
           tabs = [currentTab];
         } else {