Merge pull request #1292 from jonathanKingston/fix-missing-favicon

Add in missing favicon. Fixes #1285

.github/ISSUE_TEMPLATE.md

@@ -0,0 +1,28 @@
+<!--
+  Feel free to ignore this Issue template if you just want to ask or suggest something. If you experience an Issue then please provide all asked informations.
+
+  Note: If "Firefox will: Never remember history" in the Firefox Preferences/Options under "Privacy & Security > History" is selected, then Multi-Account Containers will not work, since Containers aren't available in Private Windows.
+-->
+- Is "Firefox will: Never remember history" in the Firefox Preferences/Options under "Privacy & Security > History" selected? Yes/No:
+- Are you using Firefox in a Private Window? Yes/No:
+- Can you see a grayed out but ticked Checkbox with the description "Enable Container Tabs" in the Firefox Preferences/Options under "Tabs"? Yes/No:
+- Multi-Account Containers Version:
+- Operating System + Version:
+- Firefox Version:
+- Other installed Add-ons + Version + Enabled/Disabled-Status:
+<!-- To be able to Copy&Paste the full list of your Add-ons navigate to "about:support" and scroll down to "Extensions" -->
+
+
+### Actual behavior
+..
+
+### Expected behavior
+..
+
+### Steps to reproduce
+1. ..
+2. ..
+3. ..
+
+### Notes
+..

CONTRIBUTING.md

@@ -9,9 +9,9 @@ Everyone is welcome to contribute to containers. Reach out to team members if yo
 
 If you find a bug with containers, please file a issue.
 
-Check first if the bug might already exist: https://github.com/mozilla/testpilot-containers/issues
+Check first if the bug might already exist: https://github.com/mozilla/multi-account-containers/issues
 
-[Open an issue](https://github.com/mozilla/testpilot-containers/issues/new)
+[Open an issue](https://github.com/mozilla/multi-account-containers/issues/new)
 
 1. Visit about:support
 2. Click "Copy raw data to clipboard" and paste into the bug. Alternatively copy the following sections into the issue:

package.json

@@ -2,41 +2,39 @@
   "name": "testpilot-containers",
   "title": "Multi-Account Containers",
   "description": "Containers helps you keep all the parts of your online life contained in different tabs. Custom labels and color-coded tabs help keep different activities — like online shopping, travel planning, or checking work email — separate.",
-  "version": "6.0.0",
+  "version": "6.0.1",
   "author": "Andrea Marchesini, Luke Crouch and Jonathan Kingston",
   "bugs": {
-    "url": "https://github.com/mozilla/testpilot-containers/issues"
+    "url": "https://github.com/mozilla/multi-account-containers/issues"
   },
   "dependencies": {},
   "devDependencies": {
-    "addons-linter": "^0.15.14",
+    "addons-linter": "^1.3.2",
     "chai": "^4.1.2",
-    "deploy-txp": "^1.0.7",
     "eslint": "^3.17.1",
     "eslint-plugin-no-unsanitized": "^2.0.0",
     "eslint-plugin-promise": "^3.4.0",
-    "htmllint-cli": "^0.0.5",
+    "htmllint-cli": "0.0.7",
     "jsdom": "^11.6.2",
     "json": "^9.0.6",
     "mocha": "^5.0.0",
     "npm-run-all": "^4.0.0",
-    "sinon": "^4.2.2",
+    "sinon": "^4.4.0",
     "sinon-chai": "^2.14.0",
     "stylelint": "^7.9.0",
     "stylelint-config-standard": "^16.0.0",
     "stylelint-order": "^0.3.0",
     "web-ext": "^2.2.2"
   },
-  "homepage": "https://github.com/mozilla/testpilot-containers#readme",
+  "homepage": "https://github.com/mozilla/multi-account-containers#readme",
   "license": "MPL-2.0",
   "main": "index.js",
   "repository": {
     "type": "git",
-    "url": "git+https://github.com/mozilla/testpilot-containers.git"
+    "url": "git+https://github.com/mozilla/multi-account-containers.git"
   },
   "scripts": {
     "build": "npm test && cd src && web-ext build --overwrite-dest",
-    "deploy": "deploy-txp",
     "lint": "npm-run-all lint:*",
     "lint:addon": "addons-linter src --self-hosted",
     "lint:css": "stylelint src/css/*.css",

src/css/popup.css

@@ -45,6 +45,7 @@ body {
   --small-text-size: 0.833rem; /* 10px */
   --small-radius: 3px;
   --icon-button-size: calc(calc(var(--block-line-separation-size) * 2) + 1.66rem); /* 20px */
+  --inactive-opacity: 0.3;
 }
 
 @media (min-resolution: 1dppx) {
@@ -537,7 +538,7 @@ span ~ .panel-header-text {
 }
 
 #current-tab > label > input:checked {
-  background-image: url("chrome://global/skin/in-content/check.svg#check-native");
+  background-image: url("/img/check.svg");
   background-position: -1px -1px;
   background-size: var(--icon-size);
 }
@@ -578,6 +579,11 @@ span ~ .panel-header-text {
   max-inline-size: 204px;
 }
 
+.disable-edit-containers {
+  opacity: var(--inactive-opacity);
+  pointer-events: none;
+}
+
 .userContext-wrapper {
   align-items: center;
   display: flex;

src/img/check.svg

@@ -0,0 +1,6 @@
+<!-- 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/. -->
+<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
+  <path fill="#3c3c3c" d="M6 14a1 1 0 0 1-.707-.293l-3-3a1 1 0 0 1 1.414-1.414l2.157 2.157 6.316-9.023a1 1 0 0 1 1.639 1.146l-7 10a1 1 0 0 1-.732.427A.863.863 0 0 1 6 14z"/>
+</svg>

src/js/background/assignManager.js

@@ -147,22 +147,44 @@ const assignManager = {
       || (messageHandler.lastCreatedTab
         && messageHandler.lastCreatedTab.id === tab.id);
     const openTabId = removeTab ? tab.openerTabId : tab.id;
-    
-    // we decided to cancel the request at this point, register it as canceled request as early as possible
-    if (!this.canceledRequests[options.requestId]) {
-      this.canceledRequests[options.requestId] = true;
-      // register a cleanup for handled requestIds
-      // all relevant requests that come in that timeframe with the same requestId will be canceled
+
+    if (!this.canceledRequests[tab.id]) {
+      // we decided to cancel the request at this point, register canceled request
+      this.canceledRequests[tab.id] = {
+        requestIds: {
+          [options.requestId]: true
+        },
+        urls: {
+          [options.url]: true
+        }
+      };
+
+      // since webRequest onCompleted and onErrorOccurred are not 100% reliable (see #1120)
+      // we register a timer here to cleanup canceled requests, just to make sure we don't
+      // end up in a situation where certain urls in a tab.id stay canceled
       setTimeout(() => {
-        delete this.canceledRequests[options.requestId];
+        if (this.canceledRequests[tab.id]) {
+          delete this.canceledRequests[tab.id];
+        }
       }, 2000);
     } else {
-      // if we see a request for the same requestId at this point then this is a redirect that we have to cancel to prevent opening two tabs
-      return {
-        cancel: true
-      };
+      let cancelEarly = false;
+      if (this.canceledRequests[tab.id].requestIds[options.requestId] ||
+          this.canceledRequests[tab.id].urls[options.url]) {
+        // same requestId or url from the same tab
+        // this is a redirect that we have to cancel early to prevent opening two tabs
+        cancelEarly = true;
+      }
+      // we decided to cancel the request at this point, register canceled request
+      this.canceledRequests[tab.id].requestIds[options.requestId] = true;
+      this.canceledRequests[tab.id].urls[options.url] = true;
+      if (cancelEarly) {
+        return {
+          cancel: true
+        };
+      }
     }
-    
+
     this.reloadPageInContainer(
       options.url,
       userContextId,
@@ -203,6 +225,19 @@ const assignManager = {
     browser.webRequest.onBeforeRequest.addListener((options) => {
       return this.onBeforeRequest(options);
     },{urls: ["<all_urls>"], types: ["main_frame"]}, ["blocking"]);
+
+    // Clean up canceled requests
+    browser.webRequest.onCompleted.addListener((options) => {
+      if (this.canceledRequests[options.tabId]) {
+        delete this.canceledRequests[options.tabId];
+      }
+    },{urls: ["<all_urls>"], types: ["main_frame"]});
+    browser.webRequest.onErrorOccurred.addListener((options) => {
+      if (this.canceledRequests[options.tabId]) {
+        delete this.canceledRequests[options.tabId];
+      }
+    },{urls: ["<all_urls>"], types: ["main_frame"]});
+
   },
 
   async _onClickedHandler(info, tab) {

src/js/popup.js

@@ -162,7 +162,7 @@ const Logic = {
   async clearBrowserActionBadge() {
     const extensionInfo = await getExtensionInfo();
     const storage = await browser.storage.local.get({browserActionBadgesClicked: []});
-    browser.browserAction.setBadgeBackgroundColor({color: ""});
+    browser.browserAction.setBadgeBackgroundColor({color: null});
     browser.browserAction.setBadgeText({text: ""});
     storage.browserActionBadgesClicked.push(extensionInfo.version);
     // use set and spread to create a unique array
@@ -212,6 +212,27 @@ const Logic = {
     return false;
   },
 
+  async numTabs() {
+    const activeTabs = await browser.tabs.query({windowId: browser.windows.WINDOW_ID_CURRENT});
+    return activeTabs.length;
+  },
+
+  _disableMoveTabs(message) {
+    const moveTabsEl = document.querySelector("#container-info-movetabs");
+    const fragment = document.createDocumentFragment();
+    const incompatEl = document.createElement("div");
+
+    moveTabsEl.classList.remove("clickable");
+    moveTabsEl.setAttribute("title", message);
+
+    fragment.appendChild(incompatEl);
+    incompatEl.setAttribute("id", "container-info-movetabs-incompat");
+    incompatEl.textContent = message;
+    incompatEl.classList.add("container-info-tab-row");
+
+    moveTabsEl.parentNode.insertBefore(fragment, moveTabsEl.nextSibling);
+  },
+
   async refreshIdentities() {
     const [identities, state] = await Promise.all([
       browser.contextualIdentities.query({}),
@@ -485,8 +506,10 @@ Logic.registerPanel(P_CONTAINERS_LIST, {
       Logic.showPanel(P_CONTAINER_EDIT, { name: Logic.generateIdentityName() });
     });
 
-    Logic.addEnterHandler(document.querySelector("#edit-containers-link"), () => {
-      Logic.showPanel(P_CONTAINERS_EDIT);
+    Logic.addEnterHandler(document.querySelector("#edit-containers-link"), (e) => {
+      if (!e.target.classList.contains("disable-edit-containers")){
+        Logic.showPanel(P_CONTAINERS_EDIT);
+      }
     });
 
     Logic.addEnterHandler(document.querySelector("#sort-containers-link"), async function () {
@@ -666,6 +689,13 @@ Logic.registerPanel(P_CONTAINERS_LIST, {
     document.addEventListener("mousedown", () => {
       document.removeEventListener("focus", focusHandler);
     });
+    /*  If no container is present disable the Edit Containers button */
+    const editContainer = document.querySelector("#edit-containers-link");
+    if (Logic.identities().length === 0) {
+      editContainer.classList.add("disable-edit-containers");
+    } else {
+      editContainer.classList.remove("disable-edit-containers");
+    }
 
     return Promise.resolve();
   },
@@ -698,37 +728,31 @@ Logic.registerPanel(P_CONTAINER_INFO, {
     });
 
     // Check if the user has incompatible add-ons installed
+    let incompatible = false;
     try {
-      const incompatible = await browser.runtime.sendMessage({
+      incompatible = await browser.runtime.sendMessage({
         method: "checkIncompatibleAddons"
       });
-      const moveTabsEl = document.querySelector("#container-info-movetabs");
-      if (incompatible) {
-        const fragment = document.createDocumentFragment();
-        const incompatEl = document.createElement("div");
-
-        moveTabsEl.classList.remove("clickable");
-        moveTabsEl.setAttribute("title", "Moving container tabs is incompatible with Pulse, PageShot, and SnoozeTabs.");
-
-        fragment.appendChild(incompatEl);
-        incompatEl.setAttribute("id", "container-info-movetabs-incompat");
-        incompatEl.textContent = "Incompatible with other Experiments.";
-        incompatEl.classList.add("container-info-tab-row");
-
-        moveTabsEl.parentNode.insertBefore(fragment, moveTabsEl.nextSibling);
-      } else {
-        Logic.addEnterHandler(moveTabsEl, async function () {
-          await browser.runtime.sendMessage({
-            method: "moveTabsToWindow",
-            windowId: browser.windows.WINDOW_ID_CURRENT,
-            cookieStoreId: Logic.currentIdentity().cookieStoreId,
-          });
-          window.close();
-        });
-      }
     } catch (e) {
       throw new Error("Could not check for incompatible add-ons.");
     }
+    const moveTabsEl = document.querySelector("#container-info-movetabs");
+    const numTabs = await Logic.numTabs();
+    if (incompatible) {
+      Logic._disableMoveTabs("Moving container tabs is incompatible with Pulse, PageShot, and SnoozeTabs.");
+      return;
+    } else if (numTabs === 1) {
+      Logic._disableMoveTabs("Cannot move a tab from a single-tab window.");
+      return; 
+    }
+    Logic.addEnterHandler(moveTabsEl, async function () {
+      await browser.runtime.sendMessage({
+        method: "moveTabsToWindow",
+        windowId: browser.windows.WINDOW_ID_CURRENT,
+        cookieStoreId: Logic.currentIdentity().cookieStoreId,
+      });
+      window.close();
+    });
   },
 
   // This method is called when the panel is shown.
@@ -1001,6 +1025,11 @@ Logic.registerPanel(P_CONTAINER_EDIT, {
 
     document.querySelector("#edit-container-panel-name-input").value = identity.name || "";
     document.querySelector("#edit-container-panel-usercontext-input").value = userContextId || NEW_CONTAINER_ID;
+    const containerName = document.querySelector("#edit-container-panel-name-input");
+    window.requestAnimationFrame(() => {
+      containerName.select();
+      containerName.focus();
+    });
     [...document.querySelectorAll("[name='container-color']")].forEach(colorInput => {
       colorInput.checked = colorInput.value === identity.color;
     });

src/js/utils.js

@@ -1,4 +1,4 @@
-const DEFAULT_FAVICON = "moz-icon://goat?size=16";
+const DEFAULT_FAVICON = "/img/blank-favicon.svg";
 
 // TODO use export here instead of globals
 window.Utils = {

src/manifest.json

@@ -1,7 +1,7 @@
 {
   "manifest_version": 2,
   "name": "Firefox Multi-Account Containers",
-  "version": "6.0.0",
+  "version": "6.0.1",
 
   "description": "Multi-Account Containers helps you keep all the parts of your online life contained in different tabs. Custom labels and color-coded tabs help keep different activities — like online shopping, travel planning, or checking work email — separate.",
   "icons": {
@@ -16,7 +16,7 @@
     }
   },
 
-  "homepage_url": "https://testpilot.firefox.com/",
+  "homepage_url": "https://github.com/mozilla/multi-account-containers#readme",
 
   "permissions": [
     "<all_urls>",

test/browser.mock.js

@@ -19,6 +19,9 @@ module.exports = () => {
       },
       onCompleted: {
         addListener: sinon.stub()
+      },
+      onErrorOccurred: {
+        addListener: sinon.stub()
       }
     },
     windows: {

test/helper.js

@@ -18,19 +18,21 @@ module.exports = {
       });
     },
 
-    async openNewTab(tab, options = {isAsync: true}) {
+    async openNewTab(tab, options = {}) {
+      if (options.resetHistory) {
+        background.browser.tabs.create.resetHistory();
+        background.browser.tabs.remove.resetHistory();
+      }
       background.browser.tabs.get.resolves(tab);
-      background.browser.webRequest.onBeforeRequest.addListener.yield({
+      background.browser.tabs.onCreated.addListener.yield(tab);
+      const [promise] = background.browser.webRequest.onBeforeRequest.addListener.yield({
         frameId: 0,
         tabId: tab.id,
         url: tab.url,
         requestId: options.requestId
       });
-      background.browser.tabs.onCreated.addListener.yield(tab);
-      if (!options.isAsync) {
-        return;
-      }
-      await nextTick();
+
+      return promise;
     }
   },
 

test/issues/940.test.js

@@ -22,8 +22,7 @@ describe("#940", () => {
         active: true
       };
       helper.browser.openNewTab(newTab, {
-        requestId: 1,
-        isAsync: false
+        requestId: 1
       });
 
       // other addon sees the same request
@@ -40,4 +39,142 @@ describe("#940", () => {
       background.browser.tabs.create.should.have.been.calledOnce;
     });
   });
+
+  describe("when redirects change requestId midflight", () => {
+    let promiseResults;
+    beforeEach(async () => {
+      // init
+      const activeTab = {
+        id: 1,
+        cookieStoreId: "firefox-container-1",
+        url: "https://www.youtube.com",
+        index: 0
+      };
+      await helper.browser.initializeWithTab(activeTab);
+      // assign the activeTab.url
+      await helper.popup.clickElementById("container-page-assigned");
+
+      // http://youtube.com
+      const newTab = {
+        id: 2,
+        cookieStoreId: "firefox-default",
+        url: "http://youtube.com",
+        index: 1,
+        active: true
+      };
+      const promise1 = helper.browser.openNewTab(newTab, {
+        requestId: 1
+      });
+
+      // https://youtube.com
+      const [promise2] = background.browser.webRequest.onBeforeRequest.addListener.yield({
+        frameId: 0,
+        tabId: newTab.id,
+        url: "https://youtube.com",
+        requestId: 1
+      });
+
+      // https://www.youtube.com
+      const [promise3] = background.browser.webRequest.onBeforeRequest.addListener.yield({
+        frameId: 0,
+        tabId: newTab.id,
+        url: "https://www.youtube.com",
+        requestId: 1
+      });
+
+      // https://www.youtube.com
+      const [promise4] = background.browser.webRequest.onBeforeRequest.addListener.yield({
+        frameId: 0,
+        tabId: newTab.id,
+        url: "https://www.youtube.com",
+        requestId: 2
+      });
+
+      promiseResults = await Promise.all([promise1, promise2, promise3, promise4]);
+    });
+
+    it("should not open two confirm pages", async () => {
+      // http://youtube.com is not assigned, no cancel, no reopening
+      expect(promiseResults[0]).to.deep.equal({});
+
+      // https://youtube.com is not assigned, no cancel, no reopening
+      expect(promiseResults[1]).to.deep.equal({});
+
+      // https://www.youtube.com is assigned, this triggers reopening, cancel
+      expect(promiseResults[2]).to.deep.equal({
+        cancel: true
+      });
+
+      // https://www.youtube.com is assigned, this was a redirect, cancel early, no reopening
+      expect(promiseResults[3]).to.deep.equal({
+        cancel: true
+      });
+
+      background.browser.tabs.create.should.have.been.calledOnce;
+    });
+
+    it("should uncancel after webRequest.onCompleted", async () => {
+      const [promise1] = background.browser.webRequest.onCompleted.addListener.yield({
+        tabId: 2
+      });
+      await promise1;
+
+      const [promise2] = background.browser.webRequest.onBeforeRequest.addListener.yield({
+        frameId: 0,
+        tabId: 2,
+        url: "https://www.youtube.com",
+        requestId: 123
+      });
+      await promise2;
+
+      background.browser.tabs.create.should.have.been.calledTwice;
+    });
+
+    it("should uncancel after webRequest.onErrorOccurred", async () => {
+      const [promise1] = background.browser.webRequest.onErrorOccurred.addListener.yield({
+        tabId: 2
+      });
+      await promise1;
+
+      // request to assigned url in same tab
+      const [promise2] = background.browser.webRequest.onBeforeRequest.addListener.yield({
+        frameId: 0,
+        tabId: 2,
+        url: "https://www.youtube.com",
+        requestId: 123
+      });
+      await promise2;
+
+      background.browser.tabs.create.should.have.been.calledTwice;
+    });
+
+    it("should uncancel after 2 seconds", async () => {
+      await new Promise(resolve => setTimeout(resolve, 2000));
+      // request to assigned url in same tab
+      const [promise2] = background.browser.webRequest.onBeforeRequest.addListener.yield({
+        frameId: 0,
+        tabId: 2,
+        url: "https://www.youtube.com",
+        requestId: 123
+      });
+      await promise2;
+
+      background.browser.tabs.create.should.have.been.calledTwice;
+    }).timeout(2002);
+
+    it("should not influence the canceled url in other tabs", async () => {
+      const newTab = {
+        id: 123,
+        cookieStoreId: "firefox-default",
+        url: "https://www.youtube.com",
+        index: 10,
+        active: true
+      };
+      await helper.browser.openNewTab(newTab, {
+        requestId: 321
+      });
+
+      background.browser.tabs.create.should.have.been.calledTwice;
+    });
+  });
 });

test/setup.js

@@ -91,11 +91,11 @@ global.buildPopupDom = async (options = {}) => {
 global.afterEach(() => {
   if (global.background) {
     global.background.dom.window.close();
-    delete global.background.dom;
+    delete global.background;
   }
 
   if (global.popup) {
     global.popup.dom.window.close();
-    delete global.popup.dom;
+    delete global.popup;
   }
 });