Browse Source

fix: incognito mode installation + UI warning + related edge cases (#963)

* fix: openerTabId can't be used across incognito

* feat: warn about incognito changes

* fix: focus & unminimize the confirmation tab's window

* fix: focus & unminimize the window in TabOpen if needed

* fix: TabOpen edge cases

* fix: use lastFocusedWindow to process incognito windows

* refactor: simplify bg/hotkeys
tophf 5 years ago
parent
commit
ee2b2d5f2f

+ 3 - 0
src/_locales/en/messages.yml

@@ -491,6 +491,9 @@ msgImported:
     Message shown after import. There is an argument referring to the count of
     Message shown after import. There is an argument referring to the count of
     scripts imported.
     scripts imported.
   message: $1 item(s) are imported.
   message: $1 item(s) are imported.
+msgIncognitoChanges:
+  description: Message shown in popup and installation tab when opened in an incognito window.
+  message: Changes you make in the incognito mode also apply to your main profile.
 msgInstalled:
 msgInstalled:
   description: Message shown when a script is installed.
   description: Message shown when a script is installed.
   message: Script installed.
   message: Script installed.

+ 4 - 6
src/background/utils/hotkeys.js

@@ -1,4 +1,3 @@
-import { getActiveTab } from '#/common';
 import { postInitialize } from './init';
 import { postInitialize } from './init';
 import { commands } from './message';
 import { commands } from './message';
 
 
@@ -8,10 +7,9 @@ const ROUTES = {
 };
 };
 
 
 postInitialize.push(() => {
 postInitialize.push(() => {
-  browser.commands.onCommand.addListener(async (cmd) => {
-    const tab = await getActiveTab();
-    const optionsUrl = browser.runtime.getURL(browser.runtime.getManifest().options_ui.page);
-    const url = `${optionsUrl}${ROUTES[cmd] || ''}`;
-    commands.TabOpen({ url, insert: true }, { tab });
+  browser.commands.onCommand.addListener((cmd) => {
+    commands.TabOpen({
+      url: `${browser.runtime.getManifest().options_ui.page}${ROUTES[cmd] || ''}`,
+    });
   });
   });
 });
 });

+ 9 - 4
src/background/utils/requests.js

@@ -371,14 +371,19 @@ async function confirmInstall({ code, from, url }, { tab = {} }) {
   if (!isUserScript(code)) throw i18n('msgInvalidScript');
   if (!isUserScript(code)) throw i18n('msgInvalidScript');
   cache.put(url, code, 3000);
   cache.put(url, code, 3000);
   const confirmKey = getUniqId();
   const confirmKey = getUniqId();
-  const tabId = tab.id;
-  cache.put(`confirm-${confirmKey}`, { url, from, tabId });
-  browser.tabs.create({
+  const { id: tabId, incognito } = tab;
+  cache.put(`confirm-${confirmKey}`, { incognito, url, from, tabId });
+  const { windowId } = await browser.tabs.create({
     url: `/confirm/index.html#${confirmKey}`,
     url: `/confirm/index.html#${confirmKey}`,
     index: tab.index + 1 || undefined,
     index: tab.index + 1 || undefined,
     active: !!tab.active,
     active: !!tab.active,
-    ...tabId >= 0 && ua.openerTabIdSupported ? { openerTabId: tabId } : {},
+    ...tabId >= 0 && ua.openerTabIdSupported && !incognito && {
+      openerTabId: tabId,
+    },
   });
   });
+  if (windowId !== tab.windowId) {
+    await browser.windows.update(windowId, { focused: true });
+  }
 }
 }
 
 
 const whitelist = [
 const whitelist = [

+ 32 - 14
src/background/utils/tabs.js

@@ -1,5 +1,6 @@
 import { getActiveTab, sendTabCmd, getFullUrl } from '#/common';
 import { getActiveTab, sendTabCmd, getFullUrl } from '#/common';
 import ua from '#/common/ua';
 import ua from '#/common/ua';
+import { extensionRoot } from './init';
 import { commands } from './message';
 import { commands } from './message';
 
 
 const openers = {};
 const openers = {};
@@ -7,29 +8,44 @@ const openers = {};
 Object.assign(commands, {
 Object.assign(commands, {
   /** @return {Promise<{ id: number }>} */
   /** @return {Promise<{ id: number }>} */
   async TabOpen({
   async TabOpen({
-    url, active, container, insert = true, pinned,
+    url,
+    active = true,
+    container,
+    insert = true,
+    pinned,
   }, src = {}) {
   }, src = {}) {
     // src.tab may be absent when invoked from popup (e.g. edit/create buttons)
     // src.tab may be absent when invoked from popup (e.g. edit/create buttons)
     const srcTab = src.tab || await getActiveTab() || {};
     const srcTab = src.tab || await getActiveTab() || {};
     // src.url may be absent when invoked directly as commands.TabOpen
     // src.url may be absent when invoked directly as commands.TabOpen
-    const isInternal = !src.url || src.url.startsWith(window.location.protocol);
+    const srcUrl = src.url;
+    const isInternal = !srcUrl || srcUrl.startsWith(extensionRoot);
     // only incognito storeId may be specified when opening in an incognito window
     // only incognito storeId may be specified when opening in an incognito window
+    const { incognito, windowId } = srcTab;
     let storeId = srcTab.cookieStoreId;
     let storeId = srcTab.cookieStoreId;
-    const { incognito } = srcTab;
-    // Chrome can't open chrome-extension:// in incognito windows because VM uses `spanning` mode
-    const sameWindow = ua.isFirefox || !incognito;
-    storeId = storeId && !incognito && getContainerId(isInternal ? 0 : container) || storeId;
-    const { id } = await browser.tabs.create({
-      active: active !== false,
+    if (storeId && !incognito) {
+      storeId = getContainerId(isInternal ? 0 : container) || storeId;
+    }
+    if (!url.startsWith('blob:')) {
+      // URL needs to be expanded to check the protocol for 'chrome' below
+      if (!isInternal) url = getFullUrl(url, srcUrl);
+      else if (!/^\w+:/.test(url)) url = browser.runtime.getURL(url);
+    }
+    const { id, windowId: newWindowId } = await browser.tabs.create({
+      url,
+      // normalizing as boolean because the API requires strict types
+      active: !!active,
       pinned: !!pinned,
       pinned: !!pinned,
-      url: isInternal || url.startsWith('blob:') ? url : getFullUrl(url, src.url),
-      windowId: sameWindow ? srcTab.windowId : undefined,
       ...storeId && { cookieStoreId: storeId },
       ...storeId && { cookieStoreId: storeId },
-      ...insert && { index: srcTab.index + 1 },
-      // XXX openerTabId seems buggy on Chrome, https://crbug.com/967150
-      // It seems to do nothing even set successfully with `browser.tabs.update`.
-      ...ua.openerTabIdSupported && sameWindow && { openerTabId: srcTab.id },
+      // Chrome can't open chrome-xxx: URLs in incognito windows
+      ...!incognito || ua.isFirefox || !/^(chrome[-\w]*):/.test(url) && {
+        windowId,
+        ...insert && { index: srcTab.index + 1 },
+        ...ua.openerTabIdSupported && { openerTabId: srcTab.id },
+      },
     });
     });
+    if (active && newWindowId !== windowId) {
+      await browser.windows.update(newWindowId, { focused: true });
+    }
     openers[id] = srcTab.id;
     openers[id] = srcTab.id;
     return { id };
     return { id };
   },
   },
@@ -41,6 +57,8 @@ Object.assign(commands, {
 });
 });
 
 
 // Firefox Android does not support `openerTabId` field, it fails if this field is passed
 // Firefox Android does not support `openerTabId` field, it fails if this field is passed
+// XXX openerTabId seems buggy on Chrome, https://crbug.com/967150
+// It seems to do nothing even set successfully with `browser.tabs.update`.
 ua.ready.then(() => {
 ua.ready.then(() => {
   Object.defineProperties(ua, {
   Object.defineProperties(ua, {
     openerTabIdSupported: {
     openerTabIdSupported: {

+ 3 - 0
src/common/browser.js

@@ -107,6 +107,9 @@ if (!global.browser?.runtime?.sendMessage) {
       executeScript: wrapAsync,
       executeScript: wrapAsync,
     },
     },
     webRequest: true,
     webRequest: true,
+    windows: {
+      update: wrapAsync,
+    },
   };
   };
   global.browser = wrapAPIs(chrome, meta);
   global.browser = wrapAPIs(chrome, meta);
 } else if (process.env.DEBUG && !global.chrome.app) {
 } else if (process.env.DEBUG && !global.chrome.app) {

+ 5 - 2
src/common/index.js

@@ -143,8 +143,11 @@ export function decodeFilename(filename) {
 }
 }
 
 
 export async function getActiveTab() {
 export async function getActiveTab() {
-  return (await browser.tabs.query({ active: true, currentWindow: true }))[0]
-    || (await browser.tabs.query({ active: true }))[0];
+  const [tab] = await browser.tabs.query({
+    active: true,
+    lastFocusedWindow: true, // also gets incognito windows
+  });
+  return tab;
 }
 }
 
 
 export function makePause(ms) {
 export function makePause(ms) {

+ 11 - 1
src/confirm/views/app.vue

@@ -6,7 +6,7 @@
           <span v-text="i18n('labelInstall')"></span> - <span v-text="i18n('extName')"></span>
           <span v-text="i18n('labelInstall')"></span> - <span v-text="i18n('extName')"></span>
         </h1>
         </h1>
         <div class="flex-auto"></div>
         <div class="flex-auto"></div>
-        <div>
+        <div class="text-right">
           <dropdown class="confirm-options" align="right">
           <dropdown class="confirm-options" align="right">
             <button slot="toggle" v-text="i18n('buttonInstallOptions')"></button>
             <button slot="toggle" v-text="i18n('buttonInstallOptions')"></button>
             <label>
             <label>
@@ -23,6 +23,7 @@
           <button v-text="i18n('buttonConfirmInstallation')"
           <button v-text="i18n('buttonConfirmInstallation')"
           :disabled="!installable" @click="installScript"></button>
           :disabled="!installable" @click="installScript"></button>
           <button v-text="i18n('buttonClose')" @click="close"></button>
           <button v-text="i18n('buttonClose')" @click="close"></button>
+          <div class="incognito" v-if="info.incognito" v-text="i18n('msgIncognitoChanges')"/>
         </div>
         </div>
       </div>
       </div>
       <div class="flex">
       <div class="flex">
@@ -242,6 +243,15 @@ export default {
 </script>
 </script>
 
 
 <style>
 <style>
+.page-confirm {
+  .incognito {
+    padding: .25em 0;
+    color: red;
+    @media (prefers-color-scheme: dark) {
+      color: orange;
+    }
+  }
+}
 .confirm-options {
 .confirm-options {
   label {
   label {
     display: block;
     display: block;

+ 4 - 3
src/popup/index.js

@@ -47,9 +47,10 @@ Object.assign(handlers, {
 });
 });
 
 
 getActiveTab()
 getActiveTab()
-.then(async ({ id, url }) => {
-  store.currentTab = { id, url };
-  browser.runtime.connect({ name: `${id}` });
+.then(async (tab) => {
+  const { url } = tab;
+  store.currentTab = tab;
+  browser.runtime.connect({ name: `${tab.id}` });
   if (/^https?:\/\//i.test(url)) {
   if (/^https?:\/\//i.test(url)) {
     const matches = url.match(/:\/\/([^/]*)/);
     const matches = url.match(/:\/\/([^/]*)/);
     const domain = matches[1];
     const domain = matches[1];

+ 3 - 0
src/popup/style.css

@@ -34,6 +34,9 @@ body {
   /* hardcoded popup height in Chrome */
   /* hardcoded popup height in Chrome */
   max-height: 600px;
   max-height: 600px;
   overflow: hidden;
   overflow: hidden;
+  .incognito {
+    padding: $padding $padding 0 $leftPaneWidth;
+  }
 }
 }
 
 
 footer {
 footer {

+ 3 - 0
src/popup/views/app.vue

@@ -94,6 +94,9 @@
         </div>
         </div>
       </div>
       </div>
     </div>
     </div>
+    <div class="incognito"
+       v-if="store.currentTab && store.currentTab.incognito"
+       v-text="i18n('msgIncognitoChanges')"/>
     <footer>
     <footer>
       <span @click="onVisitWebsite" v-text="i18n('visitWebsite')" />
       <span @click="onVisitWebsite" v-text="i18n('visitWebsite')" />
     </footer>
     </footer>