From 958453870fa5457ed018bfeea588a7631d792da5 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sun, 21 Jan 2024 17:46:30 +0800 Subject: [PATCH 1/3] Stop using `isDevToolsPage` --- src/background/executor.ts | 4 ++-- src/contrib/google/sheets/core/useCurrentOrigin.ts | 12 +++--------- src/pageEditor/hooks/useCurrentUrl.ts | 2 +- src/pageEditor/utils.ts | 2 +- src/telemetry/BackgroundLogger.ts | 6 +++--- src/testUtils/detectPageMock.ts | 4 ---- src/utils/expectContext.ts | 8 ++++++-- 7 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/background/executor.ts b/src/background/executor.ts index b90c987fe0..3139a0528d 100644 --- a/src/background/executor.ts +++ b/src/background/executor.ts @@ -68,8 +68,8 @@ async function safelyRunBrick( } // This must follow the tab existence checks or else it returns false even if the tab simply doesn't exist - if (!(await canAccessTab(tabId))) { - throw new BusinessError("PixieBrix doesn't have access to the tab"); + if (!(await canAccessTab({ tabId, frameId }))) { + throw new BusinessError("PixieBrix doesn't have access to the page"); } throw error; diff --git a/src/contrib/google/sheets/core/useCurrentOrigin.ts b/src/contrib/google/sheets/core/useCurrentOrigin.ts index a14ba53a7d..5a2af83028 100644 --- a/src/contrib/google/sheets/core/useCurrentOrigin.ts +++ b/src/contrib/google/sheets/core/useCurrentOrigin.ts @@ -15,10 +15,11 @@ * along with this program. If not, see . */ -import { isDevToolsPage, isOptionsPage } from "webext-detect-page"; +import { isOptionsPage } from "webext-detect-page"; import { useEffect } from "react"; import notify from "@/utils/notify"; import useAsyncState from "@/hooks/useAsyncState"; +import { isPageEditor } from "@/utils/expectContext"; /** * Get the current origin for where code is running. Used to set the origin on the @@ -40,14 +41,7 @@ function useCurrentOrigin(): string | null { return browser.runtime.getURL(""); } - if ( - // Checks location.pathname against the 'devtools_page' value in manifest.json - // This won't match for dev tools panels created by the devtool - // page (i.e. the Page Editor) - isDevToolsPage() || - // Check for the page editor pagePath in the location pathname - location.pathname === "/pageEditor.html" - ) { + if (isPageEditor()) { return "devtools://devtools"; } diff --git a/src/pageEditor/hooks/useCurrentUrl.ts b/src/pageEditor/hooks/useCurrentUrl.ts index 8f04abfc5e..708551efd5 100644 --- a/src/pageEditor/hooks/useCurrentUrl.ts +++ b/src/pageEditor/hooks/useCurrentUrl.ts @@ -52,7 +52,7 @@ const startWatching = once(async () => { }); export default function useCurrentUrl(): string { - expectContext("devTools"); + expectContext("pageEditor"); const [url, setUrl] = useState(tabUrl); diff --git a/src/pageEditor/utils.ts b/src/pageEditor/utils.ts index 50384034c8..7a976d9788 100644 --- a/src/pageEditor/utils.ts +++ b/src/pageEditor/utils.ts @@ -48,7 +48,7 @@ import { CustomFormRenderer } from "@/bricks/renderers/customForm"; import MapValues from "@/bricks/transformers/controlFlow/MapValues"; export async function getCurrentURL(): Promise { - expectContext("devTools"); + expectContext("pageEditor"); const tab = await browser.tabs.get(chrome.devtools.inspectedWindow.tabId); return tab.url; diff --git a/src/telemetry/BackgroundLogger.ts b/src/telemetry/BackgroundLogger.ts index d0e3e2aefc..bdae9db2a9 100644 --- a/src/telemetry/BackgroundLogger.ts +++ b/src/telemetry/BackgroundLogger.ts @@ -17,13 +17,13 @@ import { type Logger, type MessageContext } from "@/types/loggerTypes"; import { type JsonObject } from "type-fest"; -import { isBackground, isDevToolsPage } from "webext-detect-page"; +import { isBackground } from "webext-detect-page"; import { notifyContextInvalidated, wasContextInvalidated, } from "@/errors/contextInvalidated"; import { recordLog } from "@/background/messenger/api"; -import { expectContext } from "@/utils/expectContext"; +import { expectContext, isPageEditor } from "@/utils/expectContext"; import reportError from "@/telemetry/reportError"; /** @@ -73,7 +73,7 @@ class BackgroundLogger implements Logger { } async error(error: unknown, data: JsonObject): Promise { - if (wasContextInvalidated() && !isBackground() && !isDevToolsPage()) { + if (wasContextInvalidated() && !isBackground() && !isPageEditor()) { void notifyContextInvalidated(); } diff --git a/src/testUtils/detectPageMock.ts b/src/testUtils/detectPageMock.ts index 82c35d5853..a35738def2 100644 --- a/src/testUtils/detectPageMock.ts +++ b/src/testUtils/detectPageMock.ts @@ -35,10 +35,6 @@ export function isOptionsPage() { return _context === "options"; } -export function isDevToolsPage() { - return _context === "devToolsPage"; -} - export function isBackground() { return _context === "background"; } diff --git a/src/utils/expectContext.ts b/src/utils/expectContext.ts index 076834ee6c..fd5ee2fba6 100644 --- a/src/utils/expectContext.ts +++ b/src/utils/expectContext.ts @@ -27,6 +27,10 @@ function isBrowserSidebar(): boolean { return isExtensionContext() && location.pathname === "/sidebar.html"; } +export function isPageEditor(): boolean { + return location.pathname === "/pageEditor.html"; +} + /** * Accepts 'This is my error' | new Error('This is my error') | Error; * The constructor would be used to create a custom error with the default message @@ -49,16 +53,16 @@ function createError( } // eslint-disable-next-line local-rules/persistBackgroundData -- Static -const contexts = [...contextNames, "sidebar"] as const; +const contexts = [...contextNames, "sidebar", "pageEditor"] as const; // eslint-disable-next-line local-rules/persistBackgroundData -- Functions const contextMap = new Map<(typeof contexts)[number], () => boolean>([ ["web", isWebPage], ["extension", isExtensionContext], ["background", isBackground], + ["pageEditor", isPageEditor], ["contentScript", isContentScript], ["sidebar", isBrowserSidebar], - ["devTools", () => "devtools" in chrome], ]); /** From 03b24279920513f59fe8bd678d706db21d19a56b Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sun, 21 Jan 2024 17:58:32 +0800 Subject: [PATCH 2/3] Fix `expectContext` --- src/contrib/uipath/LocalProcessOptions.tsx | 2 +- src/utils/expectContext.ts | 28 ++++++++++------------ 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/contrib/uipath/LocalProcessOptions.tsx b/src/contrib/uipath/LocalProcessOptions.tsx index b6c9da5036..5b6dd5d5db 100644 --- a/src/contrib/uipath/LocalProcessOptions.tsx +++ b/src/contrib/uipath/LocalProcessOptions.tsx @@ -42,7 +42,7 @@ const LocalProcessOptions: React.FunctionComponent = ({ }) => { // Crash the React tree, because it's a programming error to use this configuration outside the dev tools expectContext( - "devTools", + "pageEditor", "useLocalRobot only works in the Page Editor due to its `thisTab` usage", ); diff --git a/src/utils/expectContext.ts b/src/utils/expectContext.ts index fd5ee2fba6..a3471f5dd5 100644 --- a/src/utils/expectContext.ts +++ b/src/utils/expectContext.ts @@ -20,7 +20,6 @@ import { isContentScript, isExtensionContext, isWebPage, - contextNames, } from "webext-detect-page"; function isBrowserSidebar(): boolean { @@ -52,18 +51,15 @@ function createError( return new errorOrCtor(defaultMessage); } -// eslint-disable-next-line local-rules/persistBackgroundData -- Static -const contexts = [...contextNames, "sidebar", "pageEditor"] as const; - // eslint-disable-next-line local-rules/persistBackgroundData -- Functions -const contextMap = new Map<(typeof contexts)[number], () => boolean>([ - ["web", isWebPage], - ["extension", isExtensionContext], - ["background", isBackground], - ["pageEditor", isPageEditor], - ["contentScript", isContentScript], - ["sidebar", isBrowserSidebar], -]); +const contextMap = { + web: isWebPage, + extension: isExtensionContext, + background: isBackground, + pageEditor: isPageEditor, + contentScript: isContentScript, + sidebar: isBrowserSidebar, +} as const; /** * @example expectContext('extension') @@ -72,10 +68,10 @@ const contextMap = new Map<(typeof contexts)[number], () => boolean>([ * @example expectContext('extension', new Error('Wrong context and this is my custom error')) */ export function expectContext( - context: (typeof contexts)[number], + context: keyof typeof contextMap, error?: ErrorBaseType, ): void { - const isContext = contextMap.get(context); + const isContext = contextMap[context]; if (!isContext) { throw new TypeError(`Context "${context}" not found`); } @@ -95,10 +91,10 @@ export function expectContext( * @example forbidContext('extension', new Error('Wrong context and this is my custom error')) */ export function forbidContext( - context: (typeof contexts)[number], + context: keyof typeof contextMap, error?: ErrorBaseType, ): void { - const isContext = contextMap.get(context); + const isContext = contextMap[context]; if (!isContext) { throw new TypeError(`Context "${context}" not found`); } From 65ec246f750971b6b5ea4187fbbc0df07d2152a0 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sun, 21 Jan 2024 21:44:32 +0800 Subject: [PATCH 3/3] Drop unnecessary test --- src/__mocks__/@/utils/expectContext.ts | 2 ++ src/contrib/google/sheets/core/useCurrentOrigin.test.ts | 8 -------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/__mocks__/@/utils/expectContext.ts b/src/__mocks__/@/utils/expectContext.ts index 09b70ed2c8..7ccecf4f94 100644 --- a/src/__mocks__/@/utils/expectContext.ts +++ b/src/__mocks__/@/utils/expectContext.ts @@ -17,3 +17,5 @@ export function expectContext() {} export function forbidContext() {} + +export { isPageEditor } from "../../../utils/expectContext"; diff --git a/src/contrib/google/sheets/core/useCurrentOrigin.test.ts b/src/contrib/google/sheets/core/useCurrentOrigin.test.ts index 8c70e7f75a..257afb311e 100644 --- a/src/contrib/google/sheets/core/useCurrentOrigin.test.ts +++ b/src/contrib/google/sheets/core/useCurrentOrigin.test.ts @@ -57,14 +57,6 @@ describe("useCurrentOrigin", () => { expect(result.current).toBe("chrome-extension://abcxyz/"); }); - test("if devtools page, should return devtools origin", async () => { - setContext("devToolsPage"); - const { result } = renderHook(() => useCurrentOrigin()); - // Wait for origin to load (async state) - await waitForEffect(); - expect(result.current).toBe(DEVTOOLS_ORIGIN); - }); - test("if page editor page, should return devtools origin", async () => { setContext("extension"); location = {