Merge pull request #1565 from kendallcorner/bookmark-menu-updates

fixed #1564: cleanup updates requested for bookmark context menu

src/js/background/assignManager.js

@@ -240,19 +240,26 @@ const assignManager = {
         delete this.canceledRequests[options.tabId];
       }
     },{urls: ["<all_urls>"], types: ["main_frame"]});
-    this.getPermissions();
+    this.resetBookmarksMenuItem();
   },
 
-  async getPermissions() {
-    const {permissions} = await browser.permissions.getAll();
-    permissions.includes("bookmarks") ? this.makeBookmarksMenu() : browser.contextMenus.remove(this.OPEN_IN_CONTAINER);
-  },
-
-  makeBookmarksMenu() {
-    this.initBookmarksMenu();
-    browser.contextualIdentities.onCreated.addListener(this.contextualIdentityCreated);
-    browser.contextualIdentities.onUpdated.addListener(this.contextualIdentityUpdated);
-    browser.contextualIdentities.onRemoved.addListener(this.contextualIdentityRemoved);
+  async resetBookmarksMenuItem() {
+    const hasPermission = await browser.permissions.contains({permissions: ["bookmarks"]});
+    if (this.hadBookmark === hasPermission) {
+      return;
+    }
+    this.hadBookmark = hasPermission;
+    if (hasPermission) {
+      this.initBookmarksMenu();
+      browser.contextualIdentities.onCreated.addListener(this.contextualIdentityCreated);
+      browser.contextualIdentities.onUpdated.addListener(this.contextualIdentityUpdated);
+      browser.contextualIdentities.onRemoved.addListener(this.contextualIdentityRemoved);
+    } else {
+      this.removeBookmarksMenu();
+      browser.contextualIdentities.onCreated.removeListener(this.contextualIdentityCreated);
+      browser.contextualIdentities.onUpdated.removeListener(this.contextualIdentityUpdated);
+      browser.contextualIdentities.onRemoved.removeListener(this.contextualIdentityRemoved);
+    }
   },
 
   contextualIdentityCreated(changeInfo) {
@@ -324,7 +331,7 @@ const assignManager = {
         if(openInReaderMode) {
           try {
             const parsed = new URL(bookmark.url);
-            bookmark.url = parsed.searchParams.get("url") + parsed.hash; // can't believe const lets me do this ...
+            bookmark.url = parsed.searchParams.get("url") + parsed.hash;
           } catch (err) {
             return err.message;
           }
@@ -521,6 +528,14 @@ const assignManager = {
         icons: { "16": `img/usercontext.svg#${identity.icon}` }
       });
     }
+  },
+
+  async removeBookmarksMenu() {
+    browser.contextMenus.remove(this.OPEN_IN_CONTAINER);
+    const identities = await browser.contextualIdentities.query({});
+    for (const identity of identities) {
+      browser.contextMenus.remove(identity.cookieStoreId);
+    }
   }
 };
 

src/js/background/messageHandler.js

@@ -11,7 +11,7 @@ const messageHandler = {
 
       switch (m.method) {
       case "resetBookmarksContext":
-        response = assignManager.getPermissions();
+        response = assignManager.resetBookmarksMenuItem();
         break;
       case "deleteContainer":
         response = backgroundLogic.deleteContainer(m.message.userContextId);

src/js/options.js

@@ -1,39 +1,23 @@
 
-function requestPermissions() {
+async function requestPermissions() {
   const checkbox = document.querySelector("#bookmarksPermissions");
   if (checkbox.checked) {
-    browser.permissions.request({permissions: ["bookmarks"]}).
-    then((response) => {
-      if (response) {
-        browser.runtime.sendMessage({ method: "resetBookmarksContext" });
-      } else {
-        checkbox.checked = false;
-      }
-    }).
-    catch((err) => {
-      return err.message;
-    });
+    const granted = await browser.permissions.request({permissions: ["bookmarks"]});
+    if (!granted) { 
+      checkbox.checked = false; 
+      return;
+    }
   } else {
-    browser.permissions.remove({permissions: ["bookmarks"]}).
-    then(() => {
-      browser.runtime.sendMessage({ method: "resetBookmarksContext" });
-    }).
-    catch((err) => {
-      return err.message;
-    });
+    await browser.permissions.remove({permissions: ["bookmarks"]});
   }
+  browser.runtime.sendMessage({ method: "resetBookmarksContext" });
 }
 
-function restoreOptions() {
-  browser.permissions.getAll()
-    .then((permissions) => {
-      if (permissions.permissions.includes("bookmarks")) {
-        document.querySelector("#bookmarksPermissions").checked = true;
-      }
-    }).
-    catch((err) => {
-      return err.message;
-    });
+async function restoreOptions() {
+  const hasPermission = await browser.permissions.contains({permissions: ["bookmarks"]});
+  if (hasPermission) {
+    document.querySelector("#bookmarksPermissions").checked = true;
+  }
 }
 
 

src/options.html

@@ -2,13 +2,15 @@
 
 <html>
   <head>
-    <meta charset="utf-8"/>
+    <meta charset="utf-8">
   </head>
 
   <body>
     <form>
-      <input type="checkbox" id="bookmarksPermissions"/>
-      <label>Enable Bookmark Menus</label>
+      <label>
+        <input type="checkbox" id="bookmarksPermissions">
+        Enable Bookmark Menus
+      </label>
       <p>This setting allows you to open a bookmark or folder of bookmarks in a container.</p>
     </form>
     <script src="js/options.js"></script>

test/browser.mock.js

@@ -93,7 +93,7 @@ module.exports = () => {
       getURL: sinon.stub().returns("moz-extension://multi-account-containers/confirm-page.html")
     },
     permissions: {
-      getAll: sinon.stub().returns({"permissions": ["bookmarks"]})
+      contains: sinon.stub().returns(true)
     }
   };