From 60a6666222e7c2967a244c810e8b97bb8a6be89c Mon Sep 17 00:00:00 2001
From: Rob Wu <rob@robwu.nl>
Date: Wed, 30 Apr 2025 00:01:43 +0200
Subject: [PATCH] Avoid logspam: "Cannot find menu item with id ..."

The extension frequently tries to remove context menus that do not
exists, which results in errors like:

> Error: Cannot find menu item with id firefox-container-1

because starting from Firefox 136, the contextMenus.remove method
rejects if the menu item does not exist. To avoid logspam, catch it.

References:

- https://bugzilla.mozilla.org/show_bug.cgi?id=1688743
- https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/136#changes_for_add-on_developers
---
 src/js/background/assignManager.js | 25 +++++++++++++++----------
 test/common.js                     |  4 ++++
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/js/background/assignManager.js b/src/js/background/assignManager.js
index 054ec3b..3cba6fc 100644
--- a/src/js/background/assignManager.js
+++ b/src/js/background/assignManager.js
@@ -483,9 +483,7 @@ window.assignManager = {
   },
 
   contextualIdentityRemoved(changeInfo) {
-    browser.contextMenus.remove(
-      changeInfo.contextualIdentity.cookieStoreId
-    );
+    this.removeMenuItem(changeInfo.contextualIdentity.cookieStoreId);
   },
 
   async _onClickedHandler(info, tab) {
@@ -672,11 +670,11 @@ window.assignManager = {
     // See: https://bugzilla.mozilla.org/show_bug.cgi?id=1215376#c16
     // We also can't change for always private mode
     // See: https://bugzilla.mozilla.org/show_bug.cgi?id=1352102
-    browser.contextMenus.remove(this.MENU_ASSIGN_ID);
-    browser.contextMenus.remove(this.MENU_REMOVE_ID);
-    browser.contextMenus.remove(this.MENU_SEPARATOR_ID);
-    browser.contextMenus.remove(this.MENU_HIDE_ID);
-    browser.contextMenus.remove(this.MENU_MOVE_ID);
+    this.removeMenuItem(this.MENU_ASSIGN_ID);
+    this.removeMenuItem(this.MENU_REMOVE_ID);
+    this.removeMenuItem(this.MENU_SEPARATOR_ID);
+    this.removeMenuItem(this.MENU_HIDE_ID);
+    this.removeMenuItem(this.MENU_MOVE_ID);
   },
 
   async calculateContextMenu(tab) {
@@ -850,12 +848,19 @@ window.assignManager = {
   },
 
   async removeBookmarksMenu() {
-    browser.contextMenus.remove(this.OPEN_IN_CONTAINER);
+    this.removeMenuItem(this.OPEN_IN_CONTAINER);
     const identities = await browser.contextualIdentities.query({});
     for (const identity of identities) {
-      browser.contextMenus.remove(identity.cookieStoreId);
+      this.removeMenuItem(identity.cookieStoreId);
     }
   },
+
+  removeMenuItem(menuItemId) {
+    // Callers do not check whether the menu exists before attempting to remove
+    // it. contextMenus.remove rejects when the menu does not exist, so we need
+    // to catch and swallow the error to avoid logspam.
+    browser.contextMenus.remove(menuItemId).catch(() => {});
+  }
 };
 
 assignManager.init();
diff --git a/test/common.js b/test/common.js
index 085ba9b..cf3ef0b 100644
--- a/test/common.js
+++ b/test/common.js
@@ -32,6 +32,10 @@ const buildDom = async ({background = {}, popup = {}}) => {
         window.crypto = {
           getRandomValues: arr => crypto.randomBytes(arr.length),
         };
+        // By default, the mock contextMenus.remove() returns undefined;
+        // Let it return a Promise instead, so that .then() calls chained to
+        // it (in src/js/background/assignManager.js) do not fail.
+        window.browser.contextMenus.remove.resolves();
       }
     }
   };