Remove usage of browser.contextMenus API events: onShown, onHidden

Remove usage of browser.contextMenus API events that don't exist in the chrome.contextMenus API.
msmolens · Aug 19, 2023 · 33ede55 · 33ede55
1 parent ea899b0
commit 33ede55
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 181 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+## [1.7.0] - 2023-08-20
+
+### Changed
+
+- Remove usage of browser.contextMenus API events that don't exist in the
+  chrome.contextMenus API.
+
 ## [1.6.0] - 2023-07-16
 
 ### Changed
@@ -112,7 +119,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - Initial release.
 
-[Unreleased]: https://github.com/msmolens/treetop/compare/v1.6.0...HEAD
+[Unreleased]: https://github.com/msmolens/treetop/compare/v1.7.0...HEAD
+[1.7.0]: https://github.com/msmolens/treetop/releases/tag/v1.7.0
 [1.6.0]: https://github.com/msmolens/treetop/releases/tag/v1.6.0
 [1.5.0]: https://github.com/msmolens/treetop/releases/tag/v1.5.0
 [1.4.1]: https://github.com/msmolens/treetop/releases/tag/v1.4.1

diff --git a/_locales/en/messages.json b/_locales/en/messages.json
@@ -136,6 +136,10 @@
     "message": "Error handling bookmark removal.",
     "description": "Message shown when an error occurs when handling bookmark removal."
   },
+  "errorHandlingContextMenu": {
+    "message": "Error handling context menu.",
+    "description": "Message shown when an error occurs when handling a context menu event."
+  },
   "errorHandlingHistoryVisit": {
     "message": "Error handling history visit.",
     "description": "Message shown when an error occurs when handling a history visit event."
@@ -148,10 +152,6 @@
     "message": "Error handling menu click.",
     "description": "Message shown when an error occurs when handling a menu click event."
   },
-  "errorHandlingMenuShown": {
-    "message": "Error handling menu shown.",
-    "description": "Message shown when an error occurs when handling a menu shown event."
-  },
   "errorLoadingPreferences": {
     "message": "Error loading preferences.",
     "description": "Message shown when an error occurs when loading the extension's preferences."

diff --git a/src/manifest.json b/src/manifest.json
@@ -2,7 +2,7 @@
   "manifest_version": 2,
   "name": "Treetop",
   "description": "__MSG_extensionDescription__",
-  "version": "1.6.0",
+  "version": "1.7.0",
   "author": "Max Smolens",
   "homepage_url": "https://github.com/msmolens/treetop",
   "default_locale": "en",

diff --git a/src/treetop/Treetop.svelte b/src/treetop/Treetop.svelte
@@ -426,25 +426,19 @@
   //
 
   function onContextMenu(event: Event) {
-    // Record the target element of the context menu
-    if (menuManager && event.target instanceof HTMLElement) {
-      menuManager.activeElement = event.target;
-    }
-  }
-
-  async function asyncOnMenuShown(info: Menus.OnShownInfoType, tab: Tabs.Tab) {
-    await menuManager?.handleMenuShown(info, tab);
-  }
-
-  function onMenuShown(info: Menus.OnShownInfoType, tab: Tabs.Tab) {
-    asyncOnMenuShown(info, tab).catch((err) => {
+    asyncOnContextMenu(event).catch((err) => {
       console.error(err);
-      handleError(browser.i18n.getMessage('errorHandlingMenuShown'));
+      handleError(browser.i18n.getMessage('errorHandlingContextMenu'));
     });
   }
 
-  function onMenuHidden() {
-    menuManager?.handleMenuHidden();
+  async function asyncOnContextMenu(event: Event) {
+    // Record the target element of the context menu
+    if (menuManager && event.target instanceof HTMLElement) {
+      menuManager.activeElement = event.target;
+
+      await menuManager.updateEnabled();
+    }
   }
 
   async function asyncOnMenuClicked(info: Menus.OnClickData, tab?: Tabs.Tab) {
@@ -545,8 +539,6 @@
 
     // Register menu event handlers
     document.addEventListener('contextmenu', onContextMenu);
-    browser.contextMenus.onShown.addListener(onMenuShown);
-    browser.contextMenus.onHidden.addListener(onMenuHidden);
     browser.contextMenus.onClicked.addListener(onMenuClicked);
 
     // Initialize menu manager
@@ -578,8 +570,6 @@
 
     // Unregister menu event handlers
     document.removeEventListener('contextmenu', onContextMenu);
-    browser.contextMenus.onShown.removeListener(onMenuShown);
-    browser.contextMenus.onHidden.removeListener(onMenuHidden);
     browser.contextMenus.onClicked.removeListener(onMenuClicked);
 
     // Destroy menu manager

diff --git a/src/treetop/menus/MenuManager.ts b/src/treetop/menus/MenuManager.ts
@@ -11,71 +11,13 @@ export class MenuManager {
 
   private readonly menuItems = new Map<string, MenuItem>();
 
-  // Track shown menu instance ID
-  // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/onShown
-  private lastMenuInstanceId = 0;
-  private nextMenuInstanceId = 1;
-
   /**
    * Register a menu item.
    */
   registerMenuItem(id: string, item: MenuItem): void {
     this.menuItems.set(id, item);
   }
 
-  /**
-   * Update whether menu items are enabled when the menu is shown.
-   * The menu must have been opened in the 'link' context in the current Treetop
-   * tab.
-   */
-  async handleMenuShown(
-    info: Menus.OnShownInfoType,
-    tab: Tabs.Tab,
-  ): Promise<void> {
-    if (!info.contexts.includes('link') || info.viewType !== 'tab') {
-      return;
-    }
-
-    // Get match pattern for the extension's origin
-    // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#Host_permissions
-    const origin = browser.runtime.getURL('');
-    if (!info.pageUrl || !info.pageUrl.startsWith(origin)) {
-      return;
-    }
-
-    // Store menu instance ID before calling async function
-    const menuInstanceId = this.nextMenuInstanceId;
-    ++this.nextMenuInstanceId;
-    this.lastMenuInstanceId = menuInstanceId;
-
-    const currentTab = await browser.tabs.getCurrent();
-    if (currentTab.id !== tab.id) {
-      return;
-    }
-
-    // Check whether the same menu is still shown
-    if (menuInstanceId !== this.lastMenuInstanceId) {
-      return;
-    }
-
-    // Update whether menu items are enabled based on the target bookmark
-    const nodeId = this.activeElement?.dataset.nodeId;
-    if (nodeId) {
-      await this.updateEnabled(nodeId);
-    }
-  }
-
-  /**
-   * Clean up state when the menu is hidden.
-   */
-  handleMenuHidden(): void {
-    // Reset the menu instance ID
-    this.lastMenuInstanceId = 0;
-
-    // Clear the active element
-    this.activeElement = null;
-  }
-
   /**
    * Call a registered menu item handler when a menu item is clicked.
    * The menu must have been clicked in the current Treetop tab.
@@ -103,12 +45,20 @@ export class MenuManager {
       const item = this.menuItems.get(info.menuItemId as string);
       item?.onClicked(nodeId);
     }
+
+    // Clear the active element
+    this.activeElement = null;
   }
 
   /**
    * Update whether menu items are enabled based on the target node.
    */
-  async updateEnabled(nodeId: string): Promise<void> {
+  async updateEnabled(): Promise<void> {
+    const nodeId = this.activeElement?.dataset.nodeId;
+    if (!nodeId) {
+      return;
+    }
+
     for (const [id, item] of this.menuItems.entries()) {
       const enabled = item.enabled(nodeId);
       await browser.contextMenus.update(id, { enabled });

diff --git a/test/treetop/Treetop.svelte.test.ts b/test/treetop/Treetop.svelte.test.ts
@@ -44,8 +44,6 @@ describe('Treetop', () => {
     //
 
     // Menu event handlers
-    mockBrowser.contextMenus.onShown.addListener.expect(expect.any(Function));
-    mockBrowser.contextMenus.onHidden.addListener.expect(expect.any(Function));
     mockBrowser.contextMenus.onClicked.addListener.expect(expect.any(Function));
 
     //
@@ -88,12 +86,6 @@ describe('Treetop', () => {
     // onDestroy
     //
 
-    mockBrowser.contextMenus.onShown.removeListener.expect(
-      expect.any(Function),
-    );
-    mockBrowser.contextMenus.onHidden.removeListener.expect(
-      expect.any(Function),
-    );
     mockBrowser.contextMenus.onClicked.removeListener.expect(
       expect.any(Function),
     );

diff --git a/test/treetop/menus/MenuManager.test.ts b/test/treetop/menus/MenuManager.test.ts
@@ -23,17 +23,6 @@ class TestMenuItem extends MenuItem {
   }
 }
 
-const createOnShownInfo = (origin: string): Menus.OnShownInfoType => {
-  return {
-    menuIds: [faker.datatype.number()],
-    contexts: ['link'],
-    viewType: 'tab',
-    editable: true,
-    pageUrl: `${origin}/${faker.random.word()}`,
-    targetElementId: faker.datatype.number(),
-  };
-};
-
 const createOnClickData = (menuItemId: string): Menus.OnClickData => {
   return {
     menuItemId,
@@ -68,112 +57,38 @@ beforeEach(() => {
   menuManager = new MenuManager();
 });
 
-describe('handleMenuShown', () => {
-  let origin: string;
-  let info: Menus.OnShownInfoType;
-  let tab: Tabs.Tab;
-
+describe('updateEnabled', () => {
   beforeEach(() => {
     const callback: OnClickedCallback = (nodeId) => {
       void nodeId;
     };
 
     menuManager.registerMenuItem('test1', new TestMenuItem(callback, true));
     menuManager.registerMenuItem('test2', new TestMenuItem(callback, false));
-
-    origin = faker.internet.url();
-    info = createOnShownInfo(origin);
-    tab = createTab();
   });
 
   it('updates whether menu items are enabled', async () => {
-    mockBrowser.runtime.getURL.expect('').andReturn(origin);
-    mockBrowser.tabs.getCurrent.expect.andResolve(tab);
     mockBrowser.contextMenus.update.expect('test1', { enabled: true });
     mockBrowser.contextMenus.update.expect('test2', { enabled: false });
     mockBrowser.contextMenus.refresh.expect;
 
     // @ts-ignore
     menuManager.activeElement = createElement();
 
-    await menuManager.handleMenuShown(info, tab);
-  });
-
-  it("no-op if context is not 'link'", async () => {
-    info.contexts = ['page'];
-
-    // @ts-ignore
-    menuManager.activeElement = createElement();
-
-    await menuManager.handleMenuShown(info, tab);
-  });
-
-  it("no-op if viewType is not 'tab'", async () => {
-    info.viewType = 'popup';
-
-    // @ts-ignore
-    menuManager.activeElement = createElement();
-
-    await menuManager.handleMenuShown(info, tab);
-  });
-
-  it("no-op if pageUrl isn't provided", async () => {
-    delete info.pageUrl;
-
-    mockBrowser.runtime.getURL.expect('').andReturn(origin);
-
-    // @ts-ignore
-    menuManager.activeElement = createElement();
-
-    await menuManager.handleMenuShown(info, tab);
-  });
-
-  it("no-op if pageUrl doesn't match", async () => {
-    info.pageUrl = faker.internet.url();
-
-    mockBrowser.runtime.getURL.expect('').andReturn(origin);
-
-    // @ts-ignore
-    menuManager.activeElement = createElement();
-
-    await menuManager.handleMenuShown(info, tab);
-  });
-
-  it('no-op if not in current tab', async () => {
-    mockBrowser.runtime.getURL.expect('').andReturn(origin);
-
-    const otherTab = createTab();
-    mockBrowser.tabs.getCurrent.expect.andResolve(otherTab);
-
-    // @ts-ignore
-    menuManager.activeElement = createElement();
-
-    await menuManager.handleMenuShown(info, tab);
+    await menuManager.updateEnabled();
   });
 
   it("no-op if active element isn't provided", async () => {
     menuManager.activeElement = null;
 
-    mockBrowser.runtime.getURL.expect('').andReturn(origin);
-    mockBrowser.tabs.getCurrent.expect.andResolve(tab);
-
-    await menuManager.handleMenuShown(info, tab);
+    await menuManager.updateEnabled();
   });
 
   it("no-op if nodeId isn't available", async () => {
     // @ts-ignore
     menuManager.activeElement = createElement(false);
 
-    mockBrowser.runtime.getURL.expect('').andReturn(origin);
-    mockBrowser.tabs.getCurrent.expect.andResolve(tab);
-
-    await menuManager.handleMenuShown(info, tab);
-  });
-});
-
-describe('handleMenuShown', () => {
-  it('succeeds', () => {
-    menuManager.handleMenuHidden();
+    await menuManager.updateEnabled();
   });
 });