From 5298551650a13d56492ec4827c75a0da87a46948 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 13:13:34 +0700 Subject: [PATCH 01/14] Setup with global types --- package-lock.json | 19 +++++++++++++++++++ package.json | 1 + src/background.ts | 1 + src/background/messenger/api.ts | 21 +++++++++++++++++++++ src/background/messenger/registration.ts | 22 ++++++++++++++++++++++ tsconfig.json | 1 + 6 files changed, 65 insertions(+) create mode 100644 src/background/messenger/api.ts create mode 100644 src/background/messenger/registration.ts diff --git a/package-lock.json b/package-lock.json index 147e50a705..fa99dc5e6e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -108,6 +108,7 @@ "webext-content-scripts": "^0.9.0", "webext-detect-page": "^3.0.2", "webext-dynamic-content-scripts": "^8.0.0", + "webext-messenger": "^0.3.0", "webext-patterns": "^1.1.1", "webext-polyfill-kinda": "^0.1.0", "webextension-polyfill-ts": "^0.26.0" @@ -40080,6 +40081,15 @@ "url": "https://github.com/sponsors/fregante" } }, + "node_modules/webext-messenger": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.0.tgz", + "integrity": "sha512-qQt0w/2ljzPBWyBHVZ65LthyPfSei/Gv1DL5prjn13wImnobpXYe8ZYbSCb1l2wUu0a6JoDRu5mrYCkdVSkVqA==", + "dependencies": { + "webext-detect-page": "^3.0.2", + "webextension-polyfill": "^0.8.0" + } + }, "node_modules/webext-patterns": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/webext-patterns/-/webext-patterns-1.1.1.tgz", @@ -72222,6 +72232,15 @@ "webext-additional-permissions": "^2.0.1" } }, + "webext-messenger": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.0.tgz", + "integrity": "sha512-qQt0w/2ljzPBWyBHVZ65LthyPfSei/Gv1DL5prjn13wImnobpXYe8ZYbSCb1l2wUu0a6JoDRu5mrYCkdVSkVqA==", + "requires": { + "webext-detect-page": "^3.0.2", + "webextension-polyfill": "^0.8.0" + } + }, "webext-patterns": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/webext-patterns/-/webext-patterns-1.1.1.tgz", diff --git a/package.json b/package.json index e3f3434eb0..44885b6208 100644 --- a/package.json +++ b/package.json @@ -129,6 +129,7 @@ "webext-content-scripts": "^0.9.0", "webext-detect-page": "^3.0.2", "webext-dynamic-content-scripts": "^8.0.0", + "webext-messenger": "^0.3.0", "webext-patterns": "^1.1.1", "webext-polyfill-kinda": "^0.1.0", "webextension-polyfill-ts": "^0.26.0" diff --git a/src/background.ts b/src/background.ts index b9f9463207..3dfa920ba7 100644 --- a/src/background.ts +++ b/src/background.ts @@ -37,6 +37,7 @@ import "./background/dataStore"; import "./background/devtools"; import "./background/browserAction"; import "./background/permissionPrompt"; +import "./background/messenger/registration"; import initGoogle from "@/contrib/google/initGoogle"; import initFrames from "@/background/iframes"; diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts new file mode 100644 index 0000000000..560ec97bed --- /dev/null +++ b/src/background/messenger/api.ts @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2021 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +/* Do not register handlers in this file */ +import { forbidContext } from "@/utils/expectContext"; + +forbidContext("background"); diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts new file mode 100644 index 0000000000..683e303ed8 --- /dev/null +++ b/src/background/messenger/registration.ts @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2021 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +/* Use this file exclusively to register the handlers from `webext-messenger` */ + +import { expectContext } from "@/utils/expectContext"; + +expectContext("background"); diff --git a/tsconfig.json b/tsconfig.json index b11b8474a9..576fe185a7 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,6 +9,7 @@ "esModuleInterop": true, "resolveJsonModule": true, "baseUrl": ".", + "skipLibCheck": false, // TODO: Drop these lines to make TS stricter https://github.com/pixiebrix/pixiebrix-extension/issues/775 "strictNullChecks": false, From 3b1a13efbe6e368ada84e924a63672aea81649ca Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 13:14:58 +0700 Subject: [PATCH 02/14] containsPermissions --- src/background/messenger/api.ts | 3 +++ src/background/messenger/registration.ts | 11 +++++++++++ src/contrib/zapier/pushOptions.tsx | 3 ++- .../editor/toolbar/PermissionsToolbar.tsx | 2 +- .../activateExtension/useEnsurePermissions.ts | 2 +- .../pages/installed/useExtensionPermissions.ts | 3 ++- .../pages/marketplace/useEnsurePermissions.ts | 2 +- src/pages/marketplace/useInstall.ts | 2 +- src/services/useDependency.ts | 3 ++- src/utils/permissions.ts | 18 +----------------- 10 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 560ec97bed..4b24de77f5 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -17,5 +17,8 @@ /* Do not register handlers in this file */ import { forbidContext } from "@/utils/expectContext"; +import { getMethod } from "webext-messenger"; forbidContext("background"); + +export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index 683e303ed8..574114ac74 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -17,6 +17,17 @@ /* Use this file exclusively to register the handlers from `webext-messenger` */ +import { registerMethods } from "webext-messenger"; import { expectContext } from "@/utils/expectContext"; expectContext("background"); + +declare global { + interface MessengerMethods { + CONTAINS_PERMISSIONS: typeof browser.permissions.contains; + } +} + +registerMethods({ + CONTAINS_PERMISSIONS: browser.permissions.contains, +}); diff --git a/src/contrib/zapier/pushOptions.tsx b/src/contrib/zapier/pushOptions.tsx index ee2f71fe8a..0508636cd7 100644 --- a/src/contrib/zapier/pushOptions.tsx +++ b/src/contrib/zapier/pushOptions.tsx @@ -35,7 +35,8 @@ import { pixieServiceFactory } from "@/services/locator"; import { getBaseURL } from "@/services/baseService"; import { ZAPIER_PERMISSIONS, ZAPIER_PROPERTIES } from "@/contrib/zapier/push"; import { ObjectField } from "@/components/fields/FieldTable"; -import { containsPermissions, requestPermissions } from "@/utils/permissions"; +import { requestPermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; import AsyncButton from "@/components/AsyncButton"; import { getErrorMessage } from "@/errors"; diff --git a/src/devTools/editor/toolbar/PermissionsToolbar.tsx b/src/devTools/editor/toolbar/PermissionsToolbar.tsx index 6f2de535f7..2d62330658 100644 --- a/src/devTools/editor/toolbar/PermissionsToolbar.tsx +++ b/src/devTools/editor/toolbar/PermissionsToolbar.tsx @@ -27,7 +27,7 @@ import { Permissions } from "webextension-polyfill-ts"; import { useAsyncEffect } from "use-async-effect"; import { useDebounce } from "use-debounce"; import { useToasts } from "react-toast-notifications"; -import { containsPermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; type PermissionsState = { hasPermissions: boolean; diff --git a/src/options/pages/activateExtension/useEnsurePermissions.ts b/src/options/pages/activateExtension/useEnsurePermissions.ts index 470484538c..79f4af7891 100644 --- a/src/options/pages/activateExtension/useEnsurePermissions.ts +++ b/src/options/pages/activateExtension/useEnsurePermissions.ts @@ -23,7 +23,7 @@ import { useAsyncState } from "@/hooks/common"; import { locator } from "@/background/locator"; import { collectPermissions, ensureAllPermissions } from "@/permissions"; import { resolveDefinitions } from "@/registry/internal"; -import { containsPermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; import { useCallback } from "react"; import { getErrorMessage } from "@/errors"; import { reportEvent } from "@/telemetry/events"; diff --git a/src/options/pages/installed/useExtensionPermissions.ts b/src/options/pages/installed/useExtensionPermissions.ts index fac9f2b58b..c114dc6903 100644 --- a/src/options/pages/installed/useExtensionPermissions.ts +++ b/src/options/pages/installed/useExtensionPermissions.ts @@ -20,7 +20,8 @@ import { useCallback, useMemo, useState } from "react"; import { useAsyncEffect } from "use-async-effect"; import { castArray } from "lodash"; import { ensureAllPermissions, extensionPermissions } from "@/permissions"; -import { containsPermissions, mergePermissions } from "@/utils/permissions"; +import { mergePermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; /** * WARNING: This hook swallows errors (to simplify the behavior for the `InstalledPage` extension table. diff --git a/src/options/pages/marketplace/useEnsurePermissions.ts b/src/options/pages/marketplace/useEnsurePermissions.ts index e3289a776a..105c7e7492 100644 --- a/src/options/pages/marketplace/useEnsurePermissions.ts +++ b/src/options/pages/marketplace/useEnsurePermissions.ts @@ -23,7 +23,7 @@ import { useAsyncState } from "@/hooks/common"; import { locator } from "@/background/locator"; import { collectPermissions, ensureAllPermissions } from "@/permissions"; import { resolveRecipe } from "@/registry/internal"; -import { containsPermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; import { useCallback } from "react"; import { getErrorMessage } from "@/errors"; import { reportEvent } from "@/telemetry/events"; diff --git a/src/pages/marketplace/useInstall.ts b/src/pages/marketplace/useInstall.ts index 968147cfd9..ba43b6feda 100644 --- a/src/pages/marketplace/useInstall.ts +++ b/src/pages/marketplace/useInstall.ts @@ -24,7 +24,7 @@ import { FormikHelpers } from "formik"; import { WizardValues } from "@/options/pages/marketplace/wizardTypes"; import { selectedExtensions } from "@/options/pages/marketplace/ConfigureBody"; import { uniq } from "lodash"; -import { containsPermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; import { collectPermissions } from "@/permissions"; import { reactivate } from "@/background/navigation"; import { push } from "connected-react-router"; diff --git a/src/services/useDependency.ts b/src/services/useDependency.ts index 16ae17e0eb..9eda937ae9 100644 --- a/src/services/useDependency.ts +++ b/src/services/useDependency.ts @@ -27,7 +27,8 @@ import { castArray, head } from "lodash"; import { locator } from "@/background/locator"; import registry from "@/services/registry"; import { Service } from "@/types"; -import { containsPermissions, requestPermissions } from "@/utils/permissions"; +import { requestPermissions } from "@/utils/permissions"; +import { containsPermissions } from "@/background/messenger/api"; import { getErrorMessage } from "@/errors"; import useNotifications from "@/hooks/useNotifications"; diff --git a/src/utils/permissions.ts b/src/utils/permissions.ts index 93326177cb..f073428228 100644 --- a/src/utils/permissions.ts +++ b/src/utils/permissions.ts @@ -17,7 +17,7 @@ import { browser, Manifest, Permissions } from "webextension-polyfill-ts"; import { uniq } from "lodash"; -import { liftBackground } from "@/background/protocol"; +import { containsPermissions } from "@/background/messenger/api"; import { openPopupPrompt } from "@/background/permissionPrompt"; /** Filters out any permissions that are not part of `optional_permissions` */ @@ -42,22 +42,6 @@ export function mergePermissions( }; } -const containsPermissionsInBackground = liftBackground( - "CONTAINS_PERMISSIONS", - async (permissions: Permissions.AnyPermissions) => - browser.permissions.contains(permissions) -); - -export async function containsPermissions( - permissions: Permissions.AnyPermissions -): Promise { - if (browser.permissions) { - return browser.permissions.contains(permissions); - } - - return containsPermissionsInBackground(permissions); -} - // TODO: Make it work in content scripts as well, or any context that doesn't have the API /** An alternative API to permissions.request() that works in Firefox’ Dev Tools */ export async function requestPermissions( From 58a3d19846297f2da0a9930ede4563046212e316 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 13:15:33 +0700 Subject: [PATCH 03/14] uninstallContextMenu --- src/background/contextMenus.ts | 14 +++++--------- src/background/deployment.ts | 10 +++++----- src/background/devtools/protocol.ts | 9 --------- src/background/messenger/api.ts | 5 +++++ src/background/messenger/registration.ts | 3 +++ src/devTools/editor/hooks/useRemove.ts | 3 ++- src/extensionPoints/contextMenu.ts | 6 ++---- src/options/pages/installed/InstalledPage.tsx | 6 ++---- src/pages/marketplace/useReinstall.ts | 2 +- 9 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/background/contextMenus.ts b/src/background/contextMenus.ts index 1704daddf5..b5e9c6e216 100644 --- a/src/background/contextMenus.ts +++ b/src/background/contextMenus.ts @@ -117,7 +117,11 @@ function menuListener(info: Menus.OnClickData, tab: Tabs.Tab) { * Uninstall contextMenu for `extensionId`. Returns true if the contextMenu was removed, or false if the contextMenu was * not found. */ -export async function uninstall(extensionId: UUID): Promise { +export async function uninstallContextMenu({ + extensionId, +}: { + extensionId: UUID; +}): Promise { try { const menuItemId = extensionMenuItems.get(extensionId); @@ -141,14 +145,6 @@ export async function uninstall(extensionId: UUID): Promise { } } -/** - * Uninstall context menu and return whether or not the context menu was uninstalled. - */ -export const uninstallContextMenu = liftBackground( - "UNINSTALL_CONTEXT_MENU", - async ({ extensionId }: { extensionId: UUID }) => uninstall(extensionId) -); - export const ensureContextMenu = liftBackground( "ENSURE_CONTEXT_MENU", async ({ diff --git a/src/background/deployment.ts b/src/background/deployment.ts index fb292539a1..89c92b35b5 100644 --- a/src/background/deployment.ts +++ b/src/background/deployment.ts @@ -29,8 +29,10 @@ import { refreshRegistries } from "@/hooks/useRefresh"; import { liftBackground } from "@/background/protocol"; import * as contentScript from "@/contentScript/lifecycle"; import { selectExtensions } from "@/options/selectors"; -import { uninstallContextMenu } from "@/background/contextMenus"; -import { containsPermissions } from "@/utils/permissions"; +import { + uninstallContextMenu, + containsPermissions, +} from "@/background/messenger/api"; import { deploymentPermissions } from "@/permissions"; import { IExtension } from "@/core"; import { ExtensionOptionsState } from "@/store/extensions"; @@ -83,9 +85,7 @@ function installDeployment( extensionId: extension.id, }; - void uninstallContextMenu(identifier).catch((error) => { - reportError(error); - }); + void uninstallContextMenu(identifier).catch(reportError); returnState = reducer(returnState, actions.removeExtension(identifier)); } diff --git a/src/background/devtools/protocol.ts b/src/background/devtools/protocol.ts index 0efb0ed3d6..851f557e4a 100644 --- a/src/background/devtools/protocol.ts +++ b/src/background/devtools/protocol.ts @@ -31,7 +31,6 @@ import { } from "@/background/devtools/internal"; import { ensureContentScript } from "@/background/util"; import { isEmpty } from "lodash"; -import * as contextMenuProtocol from "@/background/contextMenus"; import { Target } from "@/background/devtools/contract"; import { DynamicDefinition } from "@/nativeEditor/dynamic"; import { RegistryId, UUID } from "@/core"; @@ -206,14 +205,6 @@ export const runReader = liftBackground( }) ); -export const uninstallContextMenu = liftBackground( - "UNINSTALL_CONTEXT_MENU", - // False positive - it's the inner method that should be async - // eslint-disable-next-line unicorn/consistent-function-scoping - () => async ({ extensionId }: { extensionId: UUID }) => - contextMenuProtocol.uninstall(extensionId) -); - export const uninstallActionPanelPanel = liftBackground( "UNINSTALL_ACTION_PANEL_PANEL", // False positive - it's the inner method that should be async diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 4b24de77f5..083210c0ff 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -22,3 +22,8 @@ import { getMethod } from "webext-messenger"; forbidContext("background"); export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); + +/** + * Uninstall context menu and return whether or not the context menu was uninstalled. + */ +export const uninstallContextMenu = getMethod("UNINSTALL_CONTEXT_MENU"); diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index 574114ac74..0ba86b15f9 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -19,15 +19,18 @@ import { registerMethods } from "webext-messenger"; import { expectContext } from "@/utils/expectContext"; +import { uninstallContextMenu } from "@/background/contextMenus"; expectContext("background"); declare global { interface MessengerMethods { CONTAINS_PERMISSIONS: typeof browser.permissions.contains; + UNINSTALL_CONTEXT_MENU: typeof uninstallContextMenu; } } registerMethods({ CONTAINS_PERMISSIONS: browser.permissions.contains, + UNINSTALL_CONTEXT_MENU: uninstallContextMenu, }); diff --git a/src/devTools/editor/hooks/useRemove.ts b/src/devTools/editor/hooks/useRemove.ts index fc5c763a53..0202d9ee88 100644 --- a/src/devTools/editor/hooks/useRemove.ts +++ b/src/devTools/editor/hooks/useRemove.ts @@ -26,6 +26,7 @@ import * as nativeOperations from "@/background/devtools"; import { optionsSlice } from "@/options/slices"; import { reportError } from "@/telemetry/logging"; import { getErrorMessage } from "@/errors"; +import { uninstallContextMenu } from "@/background/messenger/api"; /** * Remove the current element from the page and installed extensions @@ -63,7 +64,7 @@ function useRemove(element: FormState): () => void { } void Promise.allSettled([ - nativeOperations.uninstallContextMenu(port, ref), + uninstallContextMenu(ref), nativeOperations.uninstallActionPanelPanel(port, ref), ]); diff --git a/src/extensionPoints/contextMenu.ts b/src/extensionPoints/contextMenu.ts index dff5dd11c9..b00306aede 100644 --- a/src/extensionPoints/contextMenu.ts +++ b/src/extensionPoints/contextMenu.ts @@ -39,10 +39,8 @@ import { } from "@/extensionPoints/types"; import { castArray, uniq, compact, cloneDeep, isEmpty } from "lodash"; import { checkAvailable } from "@/blocks/available"; -import { - ensureContextMenu, - uninstallContextMenu, -} from "@/background/contextMenus"; +import { ensureContextMenu } from "@/background/contextMenus"; +import { uninstallContextMenu } from "@/background/messenger/api"; import { registerHandler } from "@/contentScript/contextMenus"; import { reportError } from "@/telemetry/logging"; import { Manifest } from "webextension-polyfill-ts/lib/manifest"; diff --git a/src/options/pages/installed/InstalledPage.tsx b/src/options/pages/installed/InstalledPage.tsx index d44627aaeb..1d1ad95609 100644 --- a/src/options/pages/installed/InstalledPage.tsx +++ b/src/options/pages/installed/InstalledPage.tsx @@ -24,7 +24,7 @@ import { Link, Redirect, Route } from "react-router-dom"; import { Col, Row } from "react-bootstrap"; import { ExtensionRef, IExtension, UUID } from "@/core"; import "./InstalledPage.scss"; -import { uninstallContextMenu } from "@/background/contextMenus"; +import { uninstallContextMenu } from "@/background/messenger/api"; import { reportError } from "@/telemetry/logging"; import AuthContext from "@/auth/AuthContext"; import { reportEvent } from "@/telemetry/events"; @@ -206,9 +206,7 @@ const mapDispatchToProps = (dispatch: Dispatch) => ({ // Remove from storage first so it doesn't get re-added in reactivate step below dispatch(removeExtension(ref)); // XXX: also remove remove side panel panels that are already open? - void uninstallContextMenu(ref).catch((error) => { - reportError(error); - }); + void uninstallContextMenu(ref).catch(reportError); void reactivate().catch((error: unknown) => { console.warn("Error re-activating content scripts", { error }); }); diff --git a/src/pages/marketplace/useReinstall.ts b/src/pages/marketplace/useReinstall.ts index cd131328d7..41bd6fe221 100644 --- a/src/pages/marketplace/useReinstall.ts +++ b/src/pages/marketplace/useReinstall.ts @@ -19,7 +19,7 @@ import { RecipeDefinition } from "@/types/definitions"; import { useDispatch, useSelector } from "react-redux"; import { selectExtensions } from "@/options/selectors"; import { useCallback } from "react"; -import { uninstallContextMenu } from "@/background/contextMenus"; +import { uninstallContextMenu } from "@/background/messenger/api"; import { optionsSlice } from "@/options/slices"; import { groupBy, uniq } from "lodash"; import { IExtension } from "@/core"; From 72de2a11d3b278b07d252dbe74540d4d64d3d08e Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 13:18:52 +0700 Subject: [PATCH 04/14] ensureContextMenu --- src/background/contextMenus.ts | 144 +++++++++++------------ src/background/messenger/api.ts | 1 + src/background/messenger/registration.ts | 7 +- src/extensionPoints/contextMenu.ts | 6 +- 4 files changed, 81 insertions(+), 77 deletions(-) diff --git a/src/background/contextMenus.ts b/src/background/contextMenus.ts index b5e9c6e216..e7c8e8fb3b 100644 --- a/src/background/contextMenus.ts +++ b/src/background/contextMenus.ts @@ -16,7 +16,6 @@ */ import pTimeout from "p-timeout"; -import { liftBackground } from "@/background/protocol"; import { browser, Menus, Tabs } from "webextension-polyfill-ts"; import { isBackgroundPage } from "webext-detect-page"; import { reportError } from "@/telemetry/logging"; @@ -145,93 +144,90 @@ export async function uninstallContextMenu({ } } -export const ensureContextMenu = liftBackground( - "ENSURE_CONTEXT_MENU", - async ({ - extensionId, - contexts, - title, - documentUrlPatterns, - }: SelectionMenuOptions) => { - if (!extensionId) { - throw new Error("extensionId is required"); - } +export async function ensureContextMenu({ + extensionId, + contexts, + title, + documentUrlPatterns, +}: SelectionMenuOptions) { + if (!extensionId) { + throw new Error("extensionId is required"); + } - // Handle the thundering herd of re-registrations when a contentScript.reactivate is broadcast - if (pendingRegistration.has(extensionId)) { - console.debug("contextMenu registration pending for %s", extensionId); + // Handle the thundering herd of re-registrations when a contentScript.reactivate is broadcast + if (pendingRegistration.has(extensionId)) { + console.debug("contextMenu registration pending for %s", extensionId); - // Is it OK to return immediately? Or do we need to track the common promise that all callers should see? - return; - } + // Is it OK to return immediately? Or do we need to track the common promise that all callers should see? + return; + } - pendingRegistration.add(extensionId); + pendingRegistration.add(extensionId); - const updateProperties: Menus.UpdateUpdatePropertiesType = { - type: "normal", - title, - contexts, - documentUrlPatterns, - }; + const updateProperties: Menus.UpdateUpdatePropertiesType = { + type: "normal", + title, + contexts, + documentUrlPatterns, + }; - const expectedMenuId = makeMenuId(extensionId); + const expectedMenuId = makeMenuId(extensionId); - try { - let menuId = extensionMenuItems.get(extensionId); + try { + let menuId = extensionMenuItems.get(extensionId); - if (menuId) { - try { - await browser.contextMenus.update(menuId, updateProperties); - return; - } catch (error: unknown) { - console.debug("Cannot update context menu", { error }); - } - } else { - // Just to be safe if our `extensionMenuItems` bookkeeping is off, remove any stale menu item - await browser.contextMenus.remove(expectedMenuId).catch(noop); + if (menuId) { + try { + await browser.contextMenus.update(menuId, updateProperties); + return; + } catch (error: unknown) { + console.debug("Cannot update context menu", { error }); } + } else { + // Just to be safe if our `extensionMenuItems` bookkeeping is off, remove any stale menu item + await browser.contextMenus.remove(expectedMenuId).catch(noop); + } - // The update failed, or this is a new context menu - extensionMenuItems.delete(extensionId); - - // The types of browser.contextMenus.create are wacky. I verified on Chrome that the method does take a callback - // even when using the browser polyfill - let createdMenuId: string | number; - menuId = await new Promise((resolve, reject) => { - // `browser.contextMenus.create` returns immediately with the assigned menu id - createdMenuId = browser.contextMenus.create( - { - ...updateProperties, - id: makeMenuId(extensionId), - }, - () => { - if (browser.runtime.lastError) { - reject(new Error(browser.runtime.lastError.message)); - } - - resolve(createdMenuId); + // The update failed, or this is a new context menu + extensionMenuItems.delete(extensionId); + + // The types of browser.contextMenus.create are wacky. I verified on Chrome that the method does take a callback + // even when using the browser polyfill + let createdMenuId: string | number; + menuId = await new Promise((resolve, reject) => { + // `browser.contextMenus.create` returns immediately with the assigned menu id + createdMenuId = browser.contextMenus.create( + { + ...updateProperties, + id: makeMenuId(extensionId), + }, + () => { + if (browser.runtime.lastError) { + reject(new Error(browser.runtime.lastError.message)); } - ); - }); - extensionMenuItems.set(extensionId, menuId); - } catch (error: unknown) { - if ( - getErrorMessage(error).includes("Cannot create item with duplicate id") - ) { - // Likely caused by a concurrent update. In practice, our `pendingRegistration` set and `extensionMenuItems` - // should prevent this from happening - console.debug("Error registering context menu item", { error }); - return; - } + resolve(createdMenuId); + } + ); + }); - console.error("Error registering context menu item", { error }); - throw error; - } finally { - pendingRegistration.delete(extensionId); + extensionMenuItems.set(extensionId, menuId); + } catch (error: unknown) { + if ( + getErrorMessage(error).includes("Cannot create item with duplicate id") + ) { + // Likely caused by a concurrent update. In practice, our `pendingRegistration` set and `extensionMenuItems` + // should prevent this from happening + console.debug("Error registering context menu item", { error }); + return; } + + console.error("Error registering context menu item", { error }); + throw error; + } finally { + pendingRegistration.delete(extensionId); } -); +} if (isBackgroundPage()) { browser.contextMenus.onClicked.addListener(menuListener); diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 083210c0ff..ec409bd316 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -27,3 +27,4 @@ export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); * Uninstall context menu and return whether or not the context menu was uninstalled. */ export const uninstallContextMenu = getMethod("UNINSTALL_CONTEXT_MENU"); +export const ensureContextMenu = getMethod("ENSURE_CONTEXT_MENU"); diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index 0ba86b15f9..ed18fec080 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -19,7 +19,10 @@ import { registerMethods } from "webext-messenger"; import { expectContext } from "@/utils/expectContext"; -import { uninstallContextMenu } from "@/background/contextMenus"; +import { + ensureContextMenu, + uninstallContextMenu, +} from "@/background/contextMenus"; expectContext("background"); @@ -27,10 +30,12 @@ declare global { interface MessengerMethods { CONTAINS_PERMISSIONS: typeof browser.permissions.contains; UNINSTALL_CONTEXT_MENU: typeof uninstallContextMenu; + ENSURE_CONTEXT_MENU: typeof ensureContextMenu; } } registerMethods({ CONTAINS_PERMISSIONS: browser.permissions.contains, UNINSTALL_CONTEXT_MENU: uninstallContextMenu, + ENSURE_CONTEXT_MENU: ensureContextMenu, }); diff --git a/src/extensionPoints/contextMenu.ts b/src/extensionPoints/contextMenu.ts index b00306aede..0eff73f3ee 100644 --- a/src/extensionPoints/contextMenu.ts +++ b/src/extensionPoints/contextMenu.ts @@ -39,8 +39,10 @@ import { } from "@/extensionPoints/types"; import { castArray, uniq, compact, cloneDeep, isEmpty } from "lodash"; import { checkAvailable } from "@/blocks/available"; -import { ensureContextMenu } from "@/background/contextMenus"; -import { uninstallContextMenu } from "@/background/messenger/api"; +import { + uninstallContextMenu, + ensureContextMenu, +} from "@/background/messenger/api"; import { registerHandler } from "@/contentScript/contextMenus"; import { reportError } from "@/telemetry/logging"; import { Manifest } from "webextension-polyfill-ts/lib/manifest"; From 0766857c08f504627b3f8c0067bd47b39fdbeb83 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 15:08:53 +0700 Subject: [PATCH 05/14] openPopupPrompt --- src/background/messenger/api.ts | 1 + src/background/messenger/registration.ts | 3 +++ src/background/permissionPrompt.ts | 20 ++++++++------------ src/utils/permissions.ts | 6 ++++-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index ec409bd316..ce34964f8f 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -22,6 +22,7 @@ import { getMethod } from "webext-messenger"; forbidContext("background"); export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); +export const openPopupPrompt = getMethod("OPEN_POPUP_PROMPT"); /** * Uninstall context menu and return whether or not the context menu was uninstalled. diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index ed18fec080..21186d1ad9 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -23,6 +23,7 @@ import { ensureContextMenu, uninstallContextMenu, } from "@/background/contextMenus"; +import { openPopupPrompt } from "@/background/permissionPrompt"; expectContext("background"); @@ -31,6 +32,7 @@ declare global { CONTAINS_PERMISSIONS: typeof browser.permissions.contains; UNINSTALL_CONTEXT_MENU: typeof uninstallContextMenu; ENSURE_CONTEXT_MENU: typeof ensureContextMenu; + OPEN_POPUP_PROMPT: typeof openPopupPrompt; } } @@ -38,4 +40,5 @@ registerMethods({ CONTAINS_PERMISSIONS: browser.permissions.contains, UNINSTALL_CONTEXT_MENU: uninstallContextMenu, ENSURE_CONTEXT_MENU: ensureContextMenu, + OPEN_POPUP_PROMPT: openPopupPrompt, }); diff --git a/src/background/permissionPrompt.ts b/src/background/permissionPrompt.ts index e6a28c4025..b8a510cf23 100644 --- a/src/background/permissionPrompt.ts +++ b/src/background/permissionPrompt.ts @@ -16,7 +16,6 @@ */ import { browser, Tabs } from "webextension-polyfill-ts"; -import { liftBackground } from "@/background/protocol"; import { isFirefox } from "webext-detect-page"; const POPUP_WIDTH_PX = 400; // Makes the native prompt appear centered @@ -81,16 +80,13 @@ async function detectPopupSupport( /** * Show a popup prompt and await the popup closing */ -export const openPopupPrompt = liftBackground( - "OPEN_POPUP_PROMPT", - async (openerTabId: number, url: string) => { - const { windowId } = await browser.tabs.get(openerTabId); - const openerWindow = await browser.windows.get(windowId); +export async function openPopupPrompt(openerTabId: number, url: string) { + const { windowId } = await browser.tabs.get(openerTabId); + const openerWindow = await browser.windows.get(windowId); - const popupTab = (await detectPopupSupport(openerWindow)) - ? await openPopup(url, openerWindow) - : await openTab(url, openerTabId); + const popupTab = (await detectPopupSupport(openerWindow)) + ? await openPopup(url, openerWindow) + : await openTab(url, openerTabId); - await onTabClose(popupTab.id); - } -); + await onTabClose(popupTab.id); +} diff --git a/src/utils/permissions.ts b/src/utils/permissions.ts index f073428228..911a7e8b6d 100644 --- a/src/utils/permissions.ts +++ b/src/utils/permissions.ts @@ -17,8 +17,10 @@ import { browser, Manifest, Permissions } from "webextension-polyfill-ts"; import { uniq } from "lodash"; -import { containsPermissions } from "@/background/messenger/api"; -import { openPopupPrompt } from "@/background/permissionPrompt"; +import { + containsPermissions, + openPopupPrompt, +} from "@/background/messenger/api"; /** Filters out any permissions that are not part of `optional_permissions` */ export function selectOptionalPermissions( From a22f282d14d00ca4c2ade94ca1373b9611b106cb Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 17:29:54 +0700 Subject: [PATCH 06/14] whoAmI (sendMessage) Shorten whoAmI ID --- package-lock.json | 14 +++++++------- package.json | 2 +- src/background/executor.ts | 17 ++++++++--------- src/background/messenger/api.ts | 1 + src/background/messenger/registration.ts | 3 +++ src/contentScript.ts | 6 ++---- src/contentScript/executor.ts | 22 +--------------------- 7 files changed, 23 insertions(+), 42 deletions(-) diff --git a/package-lock.json b/package-lock.json index fa99dc5e6e..f0a547a2b1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -108,7 +108,7 @@ "webext-content-scripts": "^0.9.0", "webext-detect-page": "^3.0.2", "webext-dynamic-content-scripts": "^8.0.0", - "webext-messenger": "^0.3.0", + "webext-messenger": "^0.3.2", "webext-patterns": "^1.1.1", "webext-polyfill-kinda": "^0.1.0", "webextension-polyfill-ts": "^0.26.0" @@ -40082,9 +40082,9 @@ } }, "node_modules/webext-messenger": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.0.tgz", - "integrity": "sha512-qQt0w/2ljzPBWyBHVZ65LthyPfSei/Gv1DL5prjn13wImnobpXYe8ZYbSCb1l2wUu0a6JoDRu5mrYCkdVSkVqA==", + "version": "0.3.2", + "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.2.tgz", + "integrity": "sha512-MRXCemVroEswB34VLO7GJcbCocNH03R/ftVuahRU8BLF58+Iisqeu4GlPNJwPavfGA2RMhxmQvmuv5K6FnsYiw==", "dependencies": { "webext-detect-page": "^3.0.2", "webextension-polyfill": "^0.8.0" @@ -72233,9 +72233,9 @@ } }, "webext-messenger": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.0.tgz", - "integrity": "sha512-qQt0w/2ljzPBWyBHVZ65LthyPfSei/Gv1DL5prjn13wImnobpXYe8ZYbSCb1l2wUu0a6JoDRu5mrYCkdVSkVqA==", + "version": "0.3.2", + "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.2.tgz", + "integrity": "sha512-MRXCemVroEswB34VLO7GJcbCocNH03R/ftVuahRU8BLF58+Iisqeu4GlPNJwPavfGA2RMhxmQvmuv5K6FnsYiw==", "requires": { "webext-detect-page": "^3.0.2", "webextension-polyfill": "^0.8.0" diff --git a/package.json b/package.json index 44885b6208..6eb46dc5cb 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,7 @@ "webext-content-scripts": "^0.9.0", "webext-detect-page": "^3.0.2", "webext-dynamic-content-scripts": "^8.0.0", - "webext-messenger": "^0.3.0", + "webext-messenger": "^0.3.2", "webext-patterns": "^1.1.1", "webext-polyfill-kinda": "^0.1.0", "webextension-polyfill-ts": "^0.26.0" diff --git a/src/background/executor.ts b/src/background/executor.ts index 498e620498..56bfe10810 100644 --- a/src/background/executor.ts +++ b/src/background/executor.ts @@ -19,13 +19,12 @@ import { linkChildTab, MESSAGE_CHECK_AVAILABILITY, - MESSAGE_CONTENT_SCRIPT_ECHO_SENDER, MESSAGE_CONTENT_SCRIPT_READY, MESSAGE_RUN_BLOCK as CONTENT_MESSAGE_RUN_BLOCK, RemoteBlockOptions, RunBlockAction, } from "@/contentScript/executor"; -import { browser, Tabs } from "webextension-polyfill-ts"; +import { browser, Runtime, Tabs } from "webextension-polyfill-ts"; import { liftBackground, MESSAGE_PREFIX } from "@/background/protocol"; import { ActionType, Message, RegistryId, RenderedArgs } from "@/core"; import { emitDevtools } from "@/background/devtools/internal"; @@ -37,6 +36,7 @@ import { sleep } from "@/utils"; import { partition, zip } from "lodash"; import { getLinkedApiClient } from "@/services/apiClient"; import { JsonObject } from "type-fest"; +import { MessengerMeta } from "webext-messenger"; const MESSAGE_RUN_BLOCK_OPENER = `${MESSAGE_PREFIX}RUN_BLOCK_OPENER`; const MESSAGE_RUN_BLOCK_TARGET = `${MESSAGE_PREFIX}RUN_BLOCK_TARGET`; @@ -321,13 +321,6 @@ handlers.set(MESSAGE_CONTENT_SCRIPT_READY, async (_, sender) => { emitDevtools("ContentScriptReady", { tabId, frameId }); }); -handlers.set(MESSAGE_CONTENT_SCRIPT_ECHO_SENDER, async (_, sender) => { - console.debug("Responding %s", MESSAGE_CONTENT_SCRIPT_ECHO_SENDER, { - sender, - }); - return sender; -}); - async function linkTabListener(tab: Tabs.Tab): Promise { if (tab.openerTabId) { tabToOpener.set(tab.id, tab.openerTabId); @@ -356,6 +349,12 @@ export async function activateTab(): Promise { }); } +export async function whoAmI( + this: MessengerMeta +): Promise { + return this; +} + export async function closeTab(): Promise { expectContext("contentScript"); diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index ce34964f8f..260d373e48 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -23,6 +23,7 @@ forbidContext("background"); export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); export const openPopupPrompt = getMethod("OPEN_POPUP_PROMPT"); +export const whoAmI = getMethod("ECHO_SENDER"); /** * Uninstall context menu and return whether or not the context menu was uninstalled. diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index 21186d1ad9..453fb4e460 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -24,6 +24,7 @@ import { uninstallContextMenu, } from "@/background/contextMenus"; import { openPopupPrompt } from "@/background/permissionPrompt"; +import { whoAmI } from "@/background/executor"; expectContext("background"); @@ -33,6 +34,7 @@ declare global { UNINSTALL_CONTEXT_MENU: typeof uninstallContextMenu; ENSURE_CONTEXT_MENU: typeof ensureContextMenu; OPEN_POPUP_PROMPT: typeof openPopupPrompt; + ECHO_SENDER: typeof whoAmI; } } @@ -41,4 +43,5 @@ registerMethods({ UNINSTALL_CONTEXT_MENU: uninstallContextMenu, ENSURE_CONTEXT_MENU: ensureContextMenu, OPEN_POPUP_PROMPT: openPopupPrompt, + ECHO_SENDER: whoAmI, }); diff --git a/src/contentScript.ts b/src/contentScript.ts index b89f23c7b8..a55bea7461 100644 --- a/src/contentScript.ts +++ b/src/contentScript.ts @@ -28,16 +28,14 @@ import "@/contentScript/contextMenus"; import "@/contentScript/browserAction"; import addContentScriptListener from "@/contentScript/backgroundProtocol"; import { handleNavigate } from "@/contentScript/lifecycle"; -import addExecutorListener, { - notifyReady, - whoAmI, -} from "@/contentScript/executor"; +import addExecutorListener, { notifyReady } from "@/contentScript/executor"; import "@/messaging/external"; import "@/contentScript/script"; import "@/vendors/notify"; import { updateTabInfo } from "@/contentScript/context"; import { initTelemetry } from "@/telemetry/events"; import "@/contentScript/uipath"; +import { whoAmI } from "@/background/messenger/api"; const PIXIEBRIX_SYMBOL = Symbol.for("pixiebrix-content-script"); const uuid = uuidv4(); diff --git a/src/contentScript/executor.ts b/src/contentScript/executor.ts index 8ae129813d..6f155309db 100644 --- a/src/contentScript/executor.ts +++ b/src/contentScript/executor.ts @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { browser, Runtime } from "webextension-polyfill-ts"; +import { browser } from "webextension-polyfill-ts"; import blockRegistry from "@/blocks/registry"; import { BackgroundLogger } from "@/background/logging"; import { MessageContext, RegistryId, RenderedArgs } from "@/core"; @@ -29,13 +29,11 @@ import { checkAvailable } from "@/blocks/available"; import { markReady } from "./context"; import { ENSURE_CONTENT_SCRIPT_READY } from "@/messaging/constants"; import { expectContext } from "@/utils/expectContext"; -import { ConnectionError } from "@/errors"; import { HandlerMap } from "@/messaging/protocol"; export const MESSAGE_CHECK_AVAILABILITY = `${MESSAGE_PREFIX}CHECK_AVAILABILITY`; export const MESSAGE_RUN_BLOCK = `${MESSAGE_PREFIX}RUN_BLOCK`; export const MESSAGE_CONTENT_SCRIPT_READY = `${MESSAGE_PREFIX}SCRIPT_READY`; -export const MESSAGE_CONTENT_SCRIPT_ECHO_SENDER = `${MESSAGE_PREFIX}ECHO_SENDER`; export interface RemoteBlockOptions { ctxt: unknown; @@ -105,24 +103,6 @@ export const linkChildTab = liftContentScript( { asyncResponse: false } ); -export async function whoAmI(): Promise { - const sender = await browser.runtime.sendMessage({ - type: MESSAGE_CONTENT_SCRIPT_ECHO_SENDER, - }); - - if (sender == null) { - // If you see this error, it means the wrong message handler responded to the message. - // The most likely cause of this is that background listener function was accidentally marked - // with the "async" keyword as that prevents the method from returning "undefined" to indicate - // that it did not handle the message - throw new ConnectionError( - `Internal error: received null response for ${MESSAGE_CONTENT_SCRIPT_ECHO_SENDER}. Check use of async in message listeners` - ); - } - - return sender; -} - export async function notifyReady(): Promise { markReady(); From 764737899f0fd722af3ecebc5ddceec097716eaf Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 17:40:59 +0700 Subject: [PATCH 07/14] activateTab / closeTab (sendMessage) --- src/background/executor.ts | 34 +++++------------------- src/background/messenger/api.ts | 2 ++ src/background/messenger/registration.ts | 6 ++++- src/blocks/effects/tabs.ts | 2 +- 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/background/executor.ts b/src/background/executor.ts index 56bfe10810..c092c328c6 100644 --- a/src/background/executor.ts +++ b/src/background/executor.ts @@ -42,8 +42,6 @@ const MESSAGE_RUN_BLOCK_OPENER = `${MESSAGE_PREFIX}RUN_BLOCK_OPENER`; const MESSAGE_RUN_BLOCK_TARGET = `${MESSAGE_PREFIX}RUN_BLOCK_TARGET`; const MESSAGE_RUN_BLOCK_BROADCAST = `${MESSAGE_PREFIX}RUN_BLOCK_BROADCAST`; const MESSAGE_RUN_BLOCK_FRAME_NONCE = `${MESSAGE_PREFIX}RUN_BLOCK_FRAME_NONCE`; -const MESSAGE_ACTIVATE_TAB = `${MESSAGE_PREFIX}MESSAGE_ACTIVATE_TAB`; -const MESSAGE_CLOSE_TAB = `${MESSAGE_PREFIX}MESSAGE_CLOSE_TAB`; const MESSAGE_OPEN_TAB = `${MESSAGE_PREFIX}MESSAGE_OPEN_TAB`; const TOP_LEVEL_FRAME = 0; @@ -277,16 +275,6 @@ handlers.set( } ); -handlers.set(MESSAGE_ACTIVATE_TAB, async (_, sender) => { - await browser.tabs.update(sender.tab.id, { - active: true, - }); -}); - -handlers.set(MESSAGE_CLOSE_TAB, async (_, sender) => - browser.tabs.remove(sender.tab.id) -); - handlers.set(MESSAGE_OPEN_TAB, async (request: OpenTabAction, sender) => { const tab = await browser.tabs.create(request.payload); // FIXME: include frame information here @@ -340,30 +328,22 @@ function initExecutor(): void { browser.runtime.onMessage.addListener(handlers.asListener()); } -export async function activateTab(): Promise { - expectContext("contentScript"); - - return browser.runtime.sendMessage({ - type: MESSAGE_ACTIVATE_TAB, - payload: {}, +export async function activateTab(this: MessengerMeta): Promise { + await browser.tabs.update(this.tab.id, { + active: true, }); } +export async function closeTab(this: MessengerMeta): Promise { + await browser.tabs.remove(this.tab.id); +} + export async function whoAmI( this: MessengerMeta ): Promise { return this; } -export async function closeTab(): Promise { - expectContext("contentScript"); - - return browser.runtime.sendMessage({ - type: MESSAGE_CLOSE_TAB, - payload: {}, - }); -} - export async function openTab( options: Tabs.CreateCreatePropertiesType ): Promise { diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 260d373e48..7a3bf1dfd5 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -24,6 +24,8 @@ forbidContext("background"); export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); export const openPopupPrompt = getMethod("OPEN_POPUP_PROMPT"); export const whoAmI = getMethod("ECHO_SENDER"); +export const activateTab = getMethod("ACTIVATE_TAB"); +export const closeTab = getMethod("CLOSE_TAB"); /** * Uninstall context menu and return whether or not the context menu was uninstalled. diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index 453fb4e460..6b412b6a4a 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -24,7 +24,7 @@ import { uninstallContextMenu, } from "@/background/contextMenus"; import { openPopupPrompt } from "@/background/permissionPrompt"; -import { whoAmI } from "@/background/executor"; +import { activateTab, closeTab, whoAmI } from "@/background/executor"; expectContext("background"); @@ -35,6 +35,8 @@ declare global { ENSURE_CONTEXT_MENU: typeof ensureContextMenu; OPEN_POPUP_PROMPT: typeof openPopupPrompt; ECHO_SENDER: typeof whoAmI; + ACTIVATE_TAB: typeof activateTab; + CLOSE_TAB: typeof closeTab; } } @@ -44,4 +46,6 @@ registerMethods({ ENSURE_CONTEXT_MENU: ensureContextMenu, OPEN_POPUP_PROMPT: openPopupPrompt, ECHO_SENDER: whoAmI, + ACTIVATE_TAB: activateTab, + CLOSE_TAB: closeTab, }); diff --git a/src/blocks/effects/tabs.ts b/src/blocks/effects/tabs.ts index 06b5ee845d..35d2f45713 100644 --- a/src/blocks/effects/tabs.ts +++ b/src/blocks/effects/tabs.ts @@ -18,7 +18,7 @@ import { Effect } from "@/types"; import { Schema } from "@/core"; import { propertiesToSchema } from "@/validators/generic"; -import { activateTab, closeTab } from "@/background/executor"; +import { activateTab, closeTab } from "@/background/messenger/api"; export class ActivateTabEffect extends Effect { constructor() { From d3252911f9f5d819654ee3d4eaa8a37040876606 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 17:53:16 +0700 Subject: [PATCH 08/14] markTabAsReady (sendMessage) --- src/background/executor.ts | 7 ++++--- src/background/messenger/api.ts | 1 + src/background/messenger/registration.ts | 9 ++++++++- src/contentScript/executor.ts | 7 ++----- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/background/executor.ts b/src/background/executor.ts index c092c328c6..bc4a4d4d20 100644 --- a/src/background/executor.ts +++ b/src/background/executor.ts @@ -19,7 +19,6 @@ import { linkChildTab, MESSAGE_CHECK_AVAILABILITY, - MESSAGE_CONTENT_SCRIPT_READY, MESSAGE_RUN_BLOCK as CONTENT_MESSAGE_RUN_BLOCK, RemoteBlockOptions, RunBlockAction, @@ -282,7 +281,9 @@ handlers.set(MESSAGE_OPEN_TAB, async (request: OpenTabAction, sender) => { tabToOpener.set(tab.id, sender.tab.id); }); -handlers.set(MESSAGE_CONTENT_SCRIPT_READY, async (_, sender) => { +export async function markTabAsReady(this: MessengerMeta) { + // eslint-disable-next-line @typescript-eslint/no-this-alias, unicorn/no-this-assignment -- Not applicable to this pattern + const sender = this; const tabId = sender.tab.id; const { frameId } = sender; console.debug(`Marked tab ${tabId} (frame: ${frameId}) as ready`, { @@ -307,7 +308,7 @@ handlers.set(MESSAGE_CONTENT_SCRIPT_READY, async (_, sender) => { tabReady[tabId][frameId] = true; emitDevtools("ContentScriptReady", { tabId, frameId }); -}); +} async function linkTabListener(tab: Tabs.Tab): Promise { if (tab.openerTabId) { diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 7a3bf1dfd5..8a4413b411 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -26,6 +26,7 @@ export const openPopupPrompt = getMethod("OPEN_POPUP_PROMPT"); export const whoAmI = getMethod("ECHO_SENDER"); export const activateTab = getMethod("ACTIVATE_TAB"); export const closeTab = getMethod("CLOSE_TAB"); +export const markTabAsReady = getMethod("MARK_TAB_AS_READY"); /** * Uninstall context menu and return whether or not the context menu was uninstalled. diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index 6b412b6a4a..d850b04b03 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -24,7 +24,12 @@ import { uninstallContextMenu, } from "@/background/contextMenus"; import { openPopupPrompt } from "@/background/permissionPrompt"; -import { activateTab, closeTab, whoAmI } from "@/background/executor"; +import { + activateTab, + closeTab, + markTabAsReady, + whoAmI, +} from "@/background/executor"; expectContext("background"); @@ -37,6 +42,7 @@ declare global { ECHO_SENDER: typeof whoAmI; ACTIVATE_TAB: typeof activateTab; CLOSE_TAB: typeof closeTab; + MARK_TAB_AS_READY: typeof markTabAsReady; } } @@ -48,4 +54,5 @@ registerMethods({ ECHO_SENDER: whoAmI, ACTIVATE_TAB: activateTab, CLOSE_TAB: closeTab, + MARK_TAB_AS_READY: markTabAsReady, }); diff --git a/src/contentScript/executor.ts b/src/contentScript/executor.ts index 6f155309db..b47a8c60ce 100644 --- a/src/contentScript/executor.ts +++ b/src/contentScript/executor.ts @@ -30,10 +30,10 @@ import { markReady } from "./context"; import { ENSURE_CONTENT_SCRIPT_READY } from "@/messaging/constants"; import { expectContext } from "@/utils/expectContext"; import { HandlerMap } from "@/messaging/protocol"; +import { markTabAsReady } from "@/background/messenger/api"; export const MESSAGE_CHECK_AVAILABILITY = `${MESSAGE_PREFIX}CHECK_AVAILABILITY`; export const MESSAGE_RUN_BLOCK = `${MESSAGE_PREFIX}RUN_BLOCK`; -export const MESSAGE_CONTENT_SCRIPT_READY = `${MESSAGE_PREFIX}SCRIPT_READY`; export interface RemoteBlockOptions { ctxt: unknown; @@ -110,10 +110,7 @@ export async function notifyReady(): Promise { void browser.runtime.sendMessage({ type: ENSURE_CONTENT_SCRIPT_READY }); // Informs the standard background listener to track this tab - await browser.runtime.sendMessage({ - type: MESSAGE_CONTENT_SCRIPT_READY, - payload: {}, - }); + await markTabAsReady(); } function addExecutorListener(): void { From 9b7f347f60fd8f56e643c6731be46862b3731252 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 18:02:00 +0700 Subject: [PATCH 09/14] Related: drop unnecessary notifyReady abstraction --- src/contentScript.ts | 15 +++++++++++---- src/contentScript/executor.ts | 13 ------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/contentScript.ts b/src/contentScript.ts index a55bea7461..de88ac25ab 100644 --- a/src/contentScript.ts +++ b/src/contentScript.ts @@ -28,14 +28,15 @@ import "@/contentScript/contextMenus"; import "@/contentScript/browserAction"; import addContentScriptListener from "@/contentScript/backgroundProtocol"; import { handleNavigate } from "@/contentScript/lifecycle"; -import addExecutorListener, { notifyReady } from "@/contentScript/executor"; +import addExecutorListener from "@/contentScript/executor"; import "@/messaging/external"; import "@/contentScript/script"; import "@/vendors/notify"; -import { updateTabInfo } from "@/contentScript/context"; +import { markReady, updateTabInfo } from "@/contentScript/context"; import { initTelemetry } from "@/telemetry/events"; import "@/contentScript/uipath"; -import { whoAmI } from "@/background/messenger/api"; +import { markTabAsReady, whoAmI } from "@/background/messenger/api"; +import { ENSURE_CONTENT_SCRIPT_READY } from "./messaging/constants"; const PIXIEBRIX_SYMBOL = Symbol.for("pixiebrix-content-script"); const uuid = uuidv4(); @@ -73,7 +74,13 @@ async function init(): Promise { try { // Notify the background script know we're ready to execute remote actions - await notifyReady(); + markReady(); + + // Inform `ensureContentScript` that the content script has loaded, if it's listening + void browser.runtime.sendMessage({ type: ENSURE_CONTENT_SCRIPT_READY }); + + // Informs the standard background listener to track this tab + await markTabAsReady(); console.info(`contentScript ready in ${Date.now() - start}ms`); } catch (error: unknown) { console.error("Error pinging the background script", error); diff --git a/src/contentScript/executor.ts b/src/contentScript/executor.ts index b47a8c60ce..ba1441dd61 100644 --- a/src/contentScript/executor.ts +++ b/src/contentScript/executor.ts @@ -26,11 +26,8 @@ import { } from "@/contentScript/backgroundProtocol"; import { Availability } from "@/blocks/types"; import { checkAvailable } from "@/blocks/available"; -import { markReady } from "./context"; -import { ENSURE_CONTENT_SCRIPT_READY } from "@/messaging/constants"; import { expectContext } from "@/utils/expectContext"; import { HandlerMap } from "@/messaging/protocol"; -import { markTabAsReady } from "@/background/messenger/api"; export const MESSAGE_CHECK_AVAILABILITY = `${MESSAGE_PREFIX}CHECK_AVAILABILITY`; export const MESSAGE_RUN_BLOCK = `${MESSAGE_PREFIX}RUN_BLOCK`; @@ -103,16 +100,6 @@ export const linkChildTab = liftContentScript( { asyncResponse: false } ); -export async function notifyReady(): Promise { - markReady(); - - // Inform `ensureContentScript` that the content script has loaded, if it's listening - void browser.runtime.sendMessage({ type: ENSURE_CONTENT_SCRIPT_READY }); - - // Informs the standard background listener to track this tab - await markTabAsReady(); -} - function addExecutorListener(): void { expectContext("contentScript"); From edc3c05a895c57d333350a8272aacdaad2f3f209 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 18:30:54 +0700 Subject: [PATCH 10/14] Lint --- src/background/messenger/api.ts | 10 +++++++--- src/background/messenger/registration.ts | 3 +-- src/extensionPoints/contextMenu.ts | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 8a4413b411..8ff690288a 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -15,13 +15,17 @@ * along with this program. If not, see . */ -/* Do not register handlers in this file */ -import { forbidContext } from "@/utils/expectContext"; +/* Do not use `registerMethod` in this file */ import { getMethod } from "webext-messenger"; +import { forbidContext } from "@/utils/expectContext"; forbidContext("background"); -export const containsPermissions = getMethod("CONTAINS_PERMISSIONS"); +// Chrome offers this API in more contexts than Firefox, so it skips the messenger entirely +export const containsPermissions = browser.permissions + ? browser.permissions.contains + : getMethod("CONTAINS_PERMISSIONS"); + export const openPopupPrompt = getMethod("OPEN_POPUP_PROMPT"); export const whoAmI = getMethod("ECHO_SENDER"); export const activateTab = getMethod("ACTIVATE_TAB"); diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index d850b04b03..ae538925d4 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -15,8 +15,7 @@ * along with this program. If not, see . */ -/* Use this file exclusively to register the handlers from `webext-messenger` */ - +/* Do not use `getMethod` in this file; Keep only registrations here, not implementations */ import { registerMethods } from "webext-messenger"; import { expectContext } from "@/utils/expectContext"; import { diff --git a/src/extensionPoints/contextMenu.ts b/src/extensionPoints/contextMenu.ts index 0eff73f3ee..280b548004 100644 --- a/src/extensionPoints/contextMenu.ts +++ b/src/extensionPoints/contextMenu.ts @@ -40,8 +40,8 @@ import { import { castArray, uniq, compact, cloneDeep, isEmpty } from "lodash"; import { checkAvailable } from "@/blocks/available"; import { - uninstallContextMenu, ensureContextMenu, + uninstallContextMenu, } from "@/background/messenger/api"; import { registerHandler } from "@/contentScript/contextMenus"; import { reportError } from "@/telemetry/logging"; From f906fb36f8b9831ffa583e47a8e8df7f2de6c96b Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 18:52:36 +0700 Subject: [PATCH 11/14] Fix generate:headers --- src/background/messenger/api.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 8ff690288a..3e0932f13b 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -16,6 +16,7 @@ */ /* Do not use `registerMethod` in this file */ +import { browser } from "webextension-polyfill-ts"; import { getMethod } from "webext-messenger"; import { forbidContext } from "@/utils/expectContext"; From e46a25f571eb7aae98b58f018c915d0ed2e1cc81 Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 19:05:15 +0700 Subject: [PATCH 12/14] Use latest messenger --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index f0a547a2b1..f0430ccf9d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -108,7 +108,7 @@ "webext-content-scripts": "^0.9.0", "webext-detect-page": "^3.0.2", "webext-dynamic-content-scripts": "^8.0.0", - "webext-messenger": "^0.3.2", + "webext-messenger": "^0.4.0", "webext-patterns": "^1.1.1", "webext-polyfill-kinda": "^0.1.0", "webextension-polyfill-ts": "^0.26.0" @@ -40082,9 +40082,9 @@ } }, "node_modules/webext-messenger": { - "version": "0.3.2", - "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.2.tgz", - "integrity": "sha512-MRXCemVroEswB34VLO7GJcbCocNH03R/ftVuahRU8BLF58+Iisqeu4GlPNJwPavfGA2RMhxmQvmuv5K6FnsYiw==", + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.4.0.tgz", + "integrity": "sha512-DsPcXFxqrgLU9Wolhfa1z5iG+c6pI8PQif6uifRPhJhiV6b3v7ilJRH+bNTvMofvzozzqLyLY1nxzaNOlRBG7Q==", "dependencies": { "webext-detect-page": "^3.0.2", "webextension-polyfill": "^0.8.0" @@ -72233,9 +72233,9 @@ } }, "webext-messenger": { - "version": "0.3.2", - "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.3.2.tgz", - "integrity": "sha512-MRXCemVroEswB34VLO7GJcbCocNH03R/ftVuahRU8BLF58+Iisqeu4GlPNJwPavfGA2RMhxmQvmuv5K6FnsYiw==", + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/webext-messenger/-/webext-messenger-0.4.0.tgz", + "integrity": "sha512-DsPcXFxqrgLU9Wolhfa1z5iG+c6pI8PQif6uifRPhJhiV6b3v7ilJRH+bNTvMofvzozzqLyLY1nxzaNOlRBG7Q==", "requires": { "webext-detect-page": "^3.0.2", "webextension-polyfill": "^0.8.0" diff --git a/package.json b/package.json index 6eb46dc5cb..d8a9e2cf3e 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,7 @@ "webext-content-scripts": "^0.9.0", "webext-detect-page": "^3.0.2", "webext-dynamic-content-scripts": "^8.0.0", - "webext-messenger": "^0.3.2", + "webext-messenger": "^0.4.0", "webext-patterns": "^1.1.1", "webext-polyfill-kinda": "^0.1.0", "webextension-polyfill-ts": "^0.26.0" From f09545e1a92e5d36fc544d0098a843474f3157fa Mon Sep 17 00:00:00 2001 From: Federico Date: Thu, 9 Sep 2021 20:44:48 +0700 Subject: [PATCH 13/14] Add global for `webext-messenger` + soften context requirement --- src/background/messenger/api.ts | 14 +++++++++++--- src/background/messenger/registration.ts | 4 ++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/background/messenger/api.ts b/src/background/messenger/api.ts index 3e0932f13b..1cc4f70566 100644 --- a/src/background/messenger/api.ts +++ b/src/background/messenger/api.ts @@ -16,11 +16,16 @@ */ /* Do not use `registerMethod` in this file */ -import { browser } from "webextension-polyfill-ts"; import { getMethod } from "webext-messenger"; -import { forbidContext } from "@/utils/expectContext"; +import { browser } from "webextension-polyfill-ts"; +import { isBackgroundPage } from "webext-detect-page"; -forbidContext("background"); +// TODO: This should be a hard error, but due to unknown dependency routes, it can't be enforced yet +if (isBackgroundPage()) { + console.trace( + "This should not have been imported in the background page. Use the API directly instead." + ); +} // Chrome offers this API in more contexts than Firefox, so it skips the messenger entirely export const containsPermissions = browser.permissions @@ -38,3 +43,6 @@ export const markTabAsReady = getMethod("MARK_TAB_AS_READY"); */ export const uninstallContextMenu = getMethod("UNINSTALL_CONTEXT_MENU"); export const ensureContextMenu = getMethod("ENSURE_CONTEXT_MENU"); + +// Temporary, webext-messenger depends on this global +(globalThis as any).browser = browser; diff --git a/src/background/messenger/registration.ts b/src/background/messenger/registration.ts index ae538925d4..304c31e367 100644 --- a/src/background/messenger/registration.ts +++ b/src/background/messenger/registration.ts @@ -17,6 +17,7 @@ /* Do not use `getMethod` in this file; Keep only registrations here, not implementations */ import { registerMethods } from "webext-messenger"; +import { browser } from "webextension-polyfill-ts"; import { expectContext } from "@/utils/expectContext"; import { ensureContextMenu, @@ -55,3 +56,6 @@ registerMethods({ CLOSE_TAB: closeTab, MARK_TAB_AS_READY: markTabAsReady, }); + +// Temporary, webext-messenger depends on this global +(globalThis as any).browser = browser; From 368711168426977dfd6843727acf848be36b1c46 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Sat, 11 Sep 2021 20:46:44 -0400 Subject: [PATCH 14/14] Fix typescript warning by exporting type --- src/background/contextMenus.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/background/contextMenus.ts b/src/background/contextMenus.ts index e7c8e8fb3b..c0f4b384d7 100644 --- a/src/background/contextMenus.ts +++ b/src/background/contextMenus.ts @@ -39,12 +39,12 @@ const MENU_PREFIX = "pixiebrix-"; const CONTEXT_SCRIPT_INSTALL_MS = 1000; const CONTEXT_MENU_INSTALL_MS = 1000; -interface SelectionMenuOptions { +export type SelectionMenuOptions = { extensionId: UUID; title: string; contexts: Menus.ContextType[]; documentUrlPatterns: string[]; -} +}; function makeMenuId(extensionId: UUID): string { return `${MENU_PREFIX}${extensionId}`;