Skip to content

Commit

Permalink
make the code a bit more clear, and try-catch around the error relate…
Browse files Browse the repository at this point in the history
…d to sub-frames
  • Loading branch information
Ben Loe committed May 17, 2024
1 parent 2aeaaf8 commit d771d0f
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 57 deletions.
5 changes: 4 additions & 1 deletion src/__mocks__/@/utils/expectContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,7 @@
export function expectContext() {}
export function forbidContext() {}

export { isPageEditor, isBrowserSidebar } from "../../../utils/expectContext";
export {
isPageEditorTopFrame,
isBrowserSidebarTopFrame,
} from "../../../utils/expectContext";
32 changes: 22 additions & 10 deletions src/contrib/automationanywhere/aaFrameProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,26 @@ export async function initCopilotMessenger(): Promise<void> {
// necessary to pass to the Co-Pilot frame.
});

// Note: This code can be run either in the sidebar or in a modal
const frame = await getConnectedTarget();

// Fetch the current data from the content script when the frame loads
const data = await getCopilotHostData(frame);
console.debug("Setting initial Co-Pilot data", {
location: window.location.href,
data,
});
setHostData(data);
try {
// Note: This code can be run either in the sidebar or in a modal. Currently,
// it sometimes also runs in nested frames in the MV3 sidebar, in which case
// the webext-messenger getTopLevelFrame() function currently cannot return
// a frameId/tabId, due to how it's implemented. It will throw an error, and
// in this specific case we don't want to be running the CoPilot Data
// initialization on a nested frame anyway, so we'll just eat the error and
// warn to console for now.
const connectedTarget = await getConnectedTarget();
// Fetch the current data from the content script when the target frame loads
const data = await getCopilotHostData(connectedTarget);
console.debug("Setting initial Co-Pilot data", {
location: window.location.href,
data,
});
setHostData(data);
} catch (error) {
console.warn(
"Error getting connected target, aborting co-pilot messenger initialization",
error,
);
}
}
4 changes: 2 additions & 2 deletions src/contrib/google/sheets/core/useCurrentOrigin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ 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";
import { isPageEditorTopFrame } from "@/utils/expectContext";
import { type Nullishable } from "@/utils/nullishUtils";

/**
Expand All @@ -42,7 +42,7 @@ function useCurrentOrigin(): Nullishable<string> {
return browser.runtime.getURL("");
}

if (isPageEditor()) {
if (isPageEditorTopFrame()) {
return "devtools://devtools";
}

Expand Down
4 changes: 2 additions & 2 deletions src/errors/contextInvalidated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { isPageEditor } from "@/utils/expectContext";
import { isPageEditorTopFrame } from "@/utils/expectContext";
import { getErrorMessage, getRootCause } from "./errorHelpers";
import { CONTEXT_INVALIDATED_ERROR } from "@/errors/knownErrorMessages";

Expand All @@ -31,7 +31,7 @@ const CONTEXT_INVALIDATED_NOTIFICATION_DURATION_MS = 20_000;
* all communication becomes impossible.
*/
export async function notifyContextInvalidated(): Promise<void> {
if (isPageEditor()) {
if (isPageEditorTopFrame()) {
// It's one of the few contexts that stay open after invalidation, but it has its own InvalidatedContextGate
return;
}
Expand Down
4 changes: 2 additions & 2 deletions src/sidebar/ConnectedSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { MOD_LAUNCHER } from "@/store/sidebar/constants";
import { ensureExtensionPointsInstalled } from "@/contentScript/messenger/api";
import { getReservedSidebarEntries } from "@/contentScript/messenger/strict/api";
import {
getConnectedTabIdMv3,
getConnectedTabIdForMV3SidebarTopFrame,
getConnectedTarget,
} from "@/sidebar/connectedTarget";
import useAsyncEffect from "use-async-effect";
Expand Down Expand Up @@ -112,7 +112,7 @@ const ConnectedSidebar: React.VFC = () => {
details: chrome.webNavigation.WebNavigationFramedCallbackDetails,
) => {
const { frameId, tabId, documentLifecycle } = details;
const connectedTabId = getConnectedTabIdMv3();
const connectedTabId = getConnectedTabIdForMV3SidebarTopFrame();
if (
documentLifecycle === "active" &&
tabId === connectedTabId &&
Expand Down
39 changes: 19 additions & 20 deletions src/sidebar/connectedTarget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,21 @@
*/

import { isMV3 } from "@/mv3/api";
import { expectContext, isBrowserSidebar } from "@/utils/expectContext";
import { expectContext, isBrowserSidebarTopFrame } from "@/utils/expectContext";
import { assertNotNullish } from "@/utils/nullishUtils";
import { once } from "lodash";
import { getTopLevelFrame } from "webext-messenger";
import { getTabUrl, Target } from "webext-tools";
import { getTopLevelFrame, type TopLevelFrame } from "webext-messenger";
import { getTabUrl } from "webext-tools";

export function getConnectedTabIdMv3(): number {
export function getConnectedTabIdForMV3SidebarTopFrame(): number {
expectContext("sidebar");

if (location.pathname !== "/sidebar.html") {
throw new Error(
`getConnectedTabIdForMV3SidebarTopFrame can only be called from the MV3 sidebar's top frame. The current location is ${location.href}.`,
);
}

const tabId = new URLSearchParams(window.location.search).get("tabId");
assertNotNullish(
tabId,
Expand All @@ -38,7 +45,7 @@ async function getConnectedTabIdMv2() {
}

export const getConnectedTabId = once(
isMV3() ? getConnectedTabIdMv3 : getConnectedTabIdMv2,
isMV3() ? getConnectedTabIdForMV3SidebarTopFrame : getConnectedTabIdMv2,
);

/**
Expand All @@ -47,21 +54,13 @@ export const getConnectedTabId = once(
*/
// TODO: Drop support for "content script iframes" because it doesn't belong to `@/sidebar/connectedTarget`
// https://github.com/pixiebrix/pixiebrix-extension/pull/7354#discussion_r1461563961
// export const getConnectedTarget =
// isMV3() && isBrowserSidebar()
// ? (): TopLevelFrame => ({ tabId: getConnectedTabIdMv3(), frameId: 0 })
// : getTopLevelFrame;

export async function getConnectedTarget(): Promise<Target> {
const isMv3 = isMV3();
const isBrowserSidebarContext = isBrowserSidebar();
if (isMv3 && isBrowserSidebarContext) {
const tabId = getConnectedTabIdMv3();
return { tabId, frameId: 0 };
}

return getTopLevelFrame();
}
export const getConnectedTarget: () => Promise<TopLevelFrame> =
isMV3() && isBrowserSidebarTopFrame()
? async () => ({
tabId: getConnectedTabIdForMV3SidebarTopFrame(),
frameId: 0,
})
: getTopLevelFrame;

export async function getConnectedTargetUrl(): Promise<string | undefined> {
return getTabUrl(await getConnectedTarget());
Expand Down
4 changes: 2 additions & 2 deletions src/tinyPages/RestrictedUrlPopupApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
DISPLAY_REASON_EXTENSION_CONSOLE,
DISPLAY_REASON_UNKNOWN,
} from "@/tinyPages/restrictedUrlPopupConstants";
import { isBrowserSidebar } from "@/utils/expectContext";
import { isBrowserSidebarTopFrame } from "@/utils/expectContext";
import { getExtensionConsoleUrl } from "@/utils/extensionUtils";
import useOnMountOnly from "@/hooks/useOnMountOnly";

Expand All @@ -39,7 +39,7 @@ async function openInActiveTab(event: React.MouseEvent<HTMLAnchorElement>) {

// TODO: Drop conditon after we drop the browser action popover since this
// component will only be shown in the sidebar
if (!isBrowserSidebar()) {
if (!isBrowserSidebarTopFrame()) {
window.close();
}
}
Expand Down
29 changes: 13 additions & 16 deletions src/utils/expectContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,19 @@ import {
isWebPage,
} from "webext-detect-page";

export function isBrowserSidebar(): boolean {
// return isExtensionContext() && location.pathname === "/sidebar.html";
const isExtension = isExtensionContext();
const pathname = location.pathname;
const result = isExtension && pathname === "/sidebar.html";
if (!result) {
console.log("*** isBrowserSidebar returned false", {
isExtension,
pathname,
});
}

return result;
/**
* Whether the current context is the top frame of the browser sidebar.
*
* Note: Returns false for nested frames in the MV3 sidebar, since generally their path won't match.
*/
export function isBrowserSidebarTopFrame(): boolean {
return isExtensionContext() && location.pathname === "/sidebar.html";
}

export function isPageEditor(): boolean {
/**
* Whether the current context is the top frame of the page editor.
*/
export function isPageEditorTopFrame(): boolean {
return location.pathname === "/pageEditor.html";
}

Expand Down Expand Up @@ -67,9 +64,9 @@ const contextMap = {
web: isWebPage,
extension: isExtensionContext,
background: isBackground,
pageEditor: isPageEditor,
pageEditor: isPageEditorTopFrame,
contentScript: isContentScript,
sidebar: isBrowserSidebar,
sidebar: isBrowserSidebarTopFrame,
} as const;

/**
Expand Down
4 changes: 2 additions & 2 deletions src/utils/sidePanelUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import { getErrorMessage } from "@/errors/errorHelpers";
import { isMV3 } from "@/mv3/api";
import { forbidContext, isBrowserSidebar } from "@/utils/expectContext";
import { forbidContext, isBrowserSidebarTopFrame } from "@/utils/expectContext";
import { type PageTarget, messenger, getThisFrame } from "webext-messenger";
import { isContentScript } from "webext-detect-page";
import { showSidebar } from "@/contentScript/messenger/strict/api";
Expand All @@ -32,7 +32,7 @@ export function isUserGestureRequiredError(error: unknown): boolean {
}

export async function openSidePanel(tabId: number): Promise<void> {
if (isBrowserSidebar()) {
if (isBrowserSidebarTopFrame()) {
console.warn(
'The sidePanel called "openSidePanel". This should not happen.',
);
Expand Down

0 comments on commit d771d0f

Please sign in to comment.