address feedback

src/js/background/backgroundLogic.js

@@ -1,3 +1,4 @@
+/* global MAC_CONSTANTS */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -11,33 +12,35 @@ const backgroundLogic = {
     "about:home",
     "about:blank"
   ]),
-  NUMBER_OF_KEYBOARD_SHORTCUTS: 10,
+  // Use shared constants for counts
+  // NOTE: Keep in sync with MAC_CONSTANTS.NUMBER_OF_KEYBOARD_SHORTCUTS
   unhideQueue: [],
 
   init() {
     browser.commands.onCommand.addListener(async function (command) {
       if (command === "sort_tabs") {
         backgroundLogic.sortTabs();
-        return;
+      } else if (command.startsWith(MAC_CONSTANTS.OPEN_CONTAINER_PREFIX)) {
-      }
+        for (let i = 0; i < MAC_CONSTANTS.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) {
-
+          const key = MAC_CONSTANTS.OPEN_CONTAINER_PREFIX + i;
-      for (let i=0; i < backgroundLogic.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) {
+          const cookieStoreId = identityState.keyboardShortcut[key];
-        const key = "open_container_" + i;
+          if (command === key) {
-        const reopenKey = "reopen_in_container_" + i;
+            if (cookieStoreId !== "none") {
-        const cookieStoreId = identityState.keyboardShortcut[key];
+              browser.tabs.create({cookieStoreId});
-
+            }
-        if (cookieStoreId === "none") {
+            break;
-          continue;
+          }
         }
-
+      } else if (command.startsWith(MAC_CONSTANTS.REOPEN_IN_CONTAINER_PREFIX)) {
-        if (command === key) {
+        for (let i = 0; i < MAC_CONSTANTS.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) {
-          browser.tabs.create({cookieStoreId});
+          const key = MAC_CONSTANTS.REOPEN_IN_CONTAINER_PREFIX + i;
-          return;
+          const cookieStoreId = identityState.keyboardShortcut[key];
-        }
+          if (command === key) {
-
+            if (cookieStoreId !== "none") {
-        if (command === reopenKey) {
+              backgroundLogic.reopenInContainer(cookieStoreId);
-          backgroundLogic.reopenInContainer(cookieStoreId);
+            }
-          return;
+            break;
+          }
         }
       }
     });
@@ -81,7 +84,7 @@ const backgroundLogic = {
   },
 
   async reopenInContainer(cookieStoreId) {
-   const currentTab = await browser.tabs.query({ active: true, currentWindow: true })
+    const currentTab = await browser.tabs.query({active: true, currentWindow: true});
 
     if (currentTab.length > 0) {
       const tab = currentTab[0];
@@ -112,14 +115,14 @@ const backgroundLogic = {
   },
 
   updateTranslationInManifest() {
-    for (let index = 0; index < 10; index++) {
+    for (let index = 0; index < MAC_CONSTANTS.NUMBER_OF_KEYBOARD_SHORTCUTS; index++) {
       const adjustedIndex = index + 1; // We want to start from 1 instead of 0 in the UI.
       browser.commands.update({
-        name: `open_container_${index}`,
+        name: `${MAC_CONSTANTS.OPEN_CONTAINER_PREFIX}${index}`,
         description: browser.i18n.getMessage("containerShortcut", `${adjustedIndex}`)
       });
       browser.commands.update({
-        name: `reopen_in_container_${index}`,
+        name: `${MAC_CONSTANTS.REOPEN_IN_CONTAINER_PREFIX}${index}`,
         description: browser.i18n.getMessage("reopenInContainerShortcut", `${adjustedIndex}`)
       });
     }
@@ -231,9 +234,9 @@ const backgroundLogic = {
     const protocol = new URL(url).protocol;
     // We can't open these we just have to throw them away
     if (protocol === "about:"
-        || protocol === "chrome:"
+      || protocol === "chrome:"
-        || protocol === "moz-extension:"
+      || protocol === "moz-extension:"
-        || protocol === "file:") {
+      || protocol === "file:") {
       return false;
     }
     return true;
@@ -250,7 +253,7 @@ const backgroundLogic = {
   async getTabs(options) {
     const requiredArguments = ["cookieStoreId", "windowId"];
     this.checkArgs(requiredArguments, options, "getTabs");
-    const { cookieStoreId, windowId } = options;
+    const {cookieStoreId, windowId} = options;
 
     const list = [];
     const tabs = await browser.tabs.query({
@@ -294,7 +297,7 @@ const backgroundLogic = {
   async moveTabsToWindow(options) {
     const requiredArguments = ["cookieStoreId", "windowId"];
     this.checkArgs(requiredArguments, options, "moveTabsToWindow");
-    const { cookieStoreId, windowId } = options;
+    const {cookieStoreId, windowId} = options;
 
     const list = await browser.tabs.query({
       cookieStoreId,
@@ -305,7 +308,7 @@ const backgroundLogic = {
 
     // Nothing to do
     if (list.length === 0 &&
-        containerState.hiddenTabs.length === 0) {
+      containerState.hiddenTabs.length === 0) {
       return;
     }
     let newWindowObj;
@@ -316,7 +319,7 @@ const backgroundLogic = {
       // Pin the default tab in the new window so existing pinned tabs can be moved after it.
       // From the docs (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/move):
       //   Note that you can't move pinned tabs to a position after any unpinned tabs in a window, or move any unpinned tabs to a position before any pinned tabs.
-      await browser.tabs.update(newWindowObj.tabs[0].id, { pinned: true });
+      await browser.tabs.update(newWindowObj.tabs[0].id, {pinned: true});
 
       browser.tabs.move(list.map((tab) => tab.id), {
         windowId: newWindowObj.id,
@@ -324,8 +327,7 @@ const backgroundLogic = {
       });
     } else {
       // As we get a blank tab here we will need to await the tabs creation
-      newWindowObj = await browser.windows.create({
+      newWindowObj = await browser.windows.create({});
-      });
       hiddenDefaultTabToClose = true;
     }
 
@@ -386,7 +388,7 @@ const backgroundLogic = {
     const identities = await browser.contextualIdentities.query({});
     const identitiesOutput = {};
     const identitiesPromise = identities.map(async (identity) => {
-      const { cookieStoreId } = identity;
+      const {cookieStoreId} = identity;
       const containerState = await identityState.storageArea.get(cookieStoreId);
       const openTabs = await browser.tabs.query({
         cookieStoreId,
@@ -445,7 +447,7 @@ const backgroundLogic = {
 
       if (!map.has(tab.cookieStoreId)) {
         const userContextId = backgroundLogic.getUserContextIdFromCookieStoreId(tab.cookieStoreId);
-        map.set(tab.cookieStoreId, { order: userContextId, tabs: [] });
+        map.set(tab.cookieStoreId, {order: userContextId, tabs: []});
       }
       map.get(tab.cookieStoreId).tabs.push(tab);
     }
@@ -464,7 +466,7 @@ const backgroundLogic = {
     const sortMap = new Map([...map.entries()].sort((a, b) => a[1].order > b[1].order));
 
     // Let's move tabs.
-    for (const { tabs } of sortMap.values()) {
+    for (const {tabs} of sortMap.values()) {
       for (const tab of tabs) {
         ++pos;
         browser.tabs.move(tab.id, {
@@ -488,7 +490,7 @@ const backgroundLogic = {
   async hideTabs(options) {
     const requiredArguments = ["cookieStoreId", "windowId"];
     this.checkArgs(requiredArguments, options, "hideTabs");
-    const { cookieStoreId, windowId } = options;
+    const {cookieStoreId, windowId} = options;
 
     const userContextId = backgroundLogic.getUserContextIdFromCookieStoreId(cookieStoreId);
 
@@ -528,7 +530,7 @@ const backgroundLogic = {
   },
 
   cookieStoreId(userContextId) {
-    if(userContextId === 0) return "firefox-default";
+    if (userContextId === 0) return "firefox-default";
     return `firefox-container-${userContextId}`;
   }
 };

src/js/background/constants.js

@@ -0,0 +1,6 @@
+// Shared constants for background scripts
+window.MAC_CONSTANTS = {
+  OPEN_CONTAINER_PREFIX: "open_container_",
+  REOPEN_IN_CONTAINER_PREFIX: "reopen_in_container_",
+  NUMBER_OF_KEYBOARD_SHORTCUTS: 10,
+};

src/js/background/identityState.js

@@ -1,3 +1,4 @@
+/* global MAC_CONSTANTS */
 window.identityState = {
   keyboardShortcut: {},
   storageArea: {
@@ -50,18 +51,23 @@ window.identityState = {
 
     async loadKeyboardShortcuts () {
       const identities = await browser.contextualIdentities.query({});
-      for (let i=0; i < backgroundLogic.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) {
+      for (let i=0; i < MAC_CONSTANTS.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) {
-        const key = "open_container_" + i;
+        const openKey = MAC_CONSTANTS.OPEN_CONTAINER_PREFIX + i;
-        const storageObject = await this.area.get(key);
+        const reopenKey = MAC_CONSTANTS.REOPEN_IN_CONTAINER_PREFIX + i;
-        if (storageObject[key]){
+        const storageObject = await this.area.get(openKey);
-          identityState.keyboardShortcut[key] = storageObject[key];
+
+        if (storageObject[openKey]){
+          identityState.keyboardShortcut[openKey] = storageObject[openKey];
+          identityState.keyboardShortcut[reopenKey] = storageObject[openKey];
           continue;
         }
         if (identities[i]) {
-          identityState.keyboardShortcut[key] = identities[i].cookieStoreId;
+          identityState.keyboardShortcut[openKey] = identities[i].cookieStoreId;
+          identityState.keyboardShortcut[reopenKey] = identities[i].cookieStoreId;
           continue;
         }
-        identityState.keyboardShortcut[key] = "none";
+        identityState.keyboardShortcut[openKey] = "none";
+        identityState.keyboardShortcut[reopenKey] = "none";
       }
       return identityState.keyboardShortcut;
     },

src/js/background/index.html

@@ -15,6 +15,7 @@
 -->
     <script type="text/javascript" src="../utils.js"></script>
     <script type="text/javascript" src="../proxified-containers.js"></script>
+    <script type="text/javascript" src="constants.js"></script>
     <script type="text/javascript" src="backgroundLogic.js"></script>
     <script type="text/javascript" src="mozillaVpnBackground.js"></script>
     <script type="text/javascript" src="assignManager.js"></script>

src/manifest.json

@@ -111,63 +111,33 @@
       "description": "__MSG_containerShortcut__"
     },
     "reopen_in_container_0": {
-      "suggested_key": {
-        "default": "Alt+Shift+1"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_1": {
-      "suggested_key": {
-        "default": "Alt+Shift+2"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_2": {
-      "suggested_key": {
-        "default": "Alt+Shift+3"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_3": {
-      "suggested_key": {
-        "default": "Alt+Shift+4"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_4": {
-      "suggested_key": {
-        "default": "Alt+Shift+5"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_5": {
-      "suggested_key": {
-        "default": "Alt+Shift+6"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_6": {
-      "suggested_key": {
-        "default": "Alt+Shift+7"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_7": {
-      "suggested_key": {
-        "default": "Alt+Shift+8"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_8": {
-      "suggested_key": {
-        "default": "Alt+Shift+9"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     },
     "reopen_in_container_9": {
-      "suggested_key": {
-        "default": "Alt+Shift+0"
-      },
       "description": "__MSG_reopenInContainerShortcut__"
     }
   },