From 33ede55cdb639f4aeb65ed51c5a4426a18bb0c80 Mon Sep 17 00:00:00 2001 From: Max Smolens Date: Sat, 19 Aug 2023 15:56:53 -0400 Subject: [PATCH] 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. --- CHANGELOG.md | 10 ++- _locales/en/messages.json | 8 +-- src/manifest.json | 2 +- src/treetop/Treetop.svelte | 28 +++----- src/treetop/menus/MenuManager.ts | 68 +++---------------- test/treetop/Treetop.svelte.test.ts | 8 --- test/treetop/menus/MenuManager.test.ts | 93 ++------------------------ 7 files changed, 36 insertions(+), 181 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e5fb0a..38113cf 100644 --- 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 index ac4691d..05ee8bb 100644 --- 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 index 1596c45..7f6b879 100644 --- 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 index 3d2b555..3218f42 100644 --- 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 index b56ed3b..a8b7848 100644 --- a/src/treetop/menus/MenuManager.ts +++ b/src/treetop/menus/MenuManager.ts @@ -11,11 +11,6 @@ export class MenuManager { private readonly menuItems = new Map(); - // 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. */ @@ -23,59 +18,6 @@ export class MenuManager { 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 { - 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 { + async updateEnabled(): Promise { + 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 index 24c2040..5d149d1 100644 --- 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 index 007d106..f6f6802 100644 --- 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,11 +57,7 @@ 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; @@ -80,15 +65,9 @@ describe('handleMenuShown', () => { 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; @@ -96,84 +75,20 @@ describe('handleMenuShown', () => { // @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(); }); });