Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DX - Refactor some of the sidebar frame function names to clarify behavior, and fix sidebar-sub-frame console error #8475

Merged
merged 7 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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";
29 changes: 24 additions & 5 deletions src/contrib/automationanywhere/aaFrameProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import { expectContext } from "@/utils/expectContext";
import { getConnectedTarget } from "@/sidebar/connectedTarget";
import { getCopilotHostData } from "@/contentScript/messenger/strict/api";
import {
SET_COPILOT_DATA_MESSAGE_TYPE,
type ProcessDataMap,
SET_COPILOT_DATA_MESSAGE_TYPE,
} from "@/contrib/automationanywhere/aaTypes";
import { type TopLevelFrame } from "webext-messenger";

/**
* `window.postMessage` data payload the Co-Pilot frame sends to the host application.
Expand Down Expand Up @@ -121,11 +122,29 @@ 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();
let connectedTarget: TopLevelFrame | null = null;
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.
connectedTarget = await getConnectedTarget();
} catch (error) {
console.warn(
"Error getting connected target, aborting co-pilot messenger initialization",
error,
);
}

if (!connectedTarget) {
return;
}

// Fetch the current data from the content script when the frame loads
const data = await getCopilotHostData(frame);
// 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,
Expand Down
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
24 changes: 17 additions & 7 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 { type TopLevelFrame, getTopLevelFrame } from "webext-messenger";
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,9 +54,12 @@ 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 })
export const getConnectedTarget: () => Promise<TopLevelFrame> =
isMV3() && isBrowserSidebarTopFrame()
? async () => ({
tabId: getConnectedTabIdForMV3SidebarTopFrame(),
frameId: 0,
})
: getTopLevelFrame;

export async function getConnectedTargetUrl(): Promise<string | undefined> {
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
16 changes: 12 additions & 4 deletions src/utils/expectContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,19 @@ import {
isWebPage,
} from "webext-detect-page";

export function isBrowserSidebar(): boolean {
/**
* 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 @@ -56,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
Loading