DX - Refactor some of the sidebar frame function names to clarify beh…

…avior, and fix sidebar-sub-frame console error (#8475)

* debug changes

* make the code a bit more clear, and try-catch around the error related to sub-frames

* only wrap the getConnectedTarget call in try catch

* fix types

* fix var

---------

Co-authored-by: Ben Loe <[email protected]>
Co-authored-by: Todd Schiller <[email protected]>

src/__mocks__/@/utils/expectContext.ts

@@ -18,4 +18,7 @@
 export function expectContext() {}
 export function forbidContext() {}
 
-export { isPageEditor, isBrowserSidebar } from "../../../utils/expectContext";
+export {
+  isPageEditorTopFrame,
+  isBrowserSidebarTopFrame,
+} from "../../../utils/expectContext";

src/contrib/automationanywhere/aaFrameProtocol.ts

@@ -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.
@@ -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,

src/contrib/google/sheets/core/useCurrentOrigin.ts

@@ -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";
 
 /**
@@ -42,7 +42,7 @@ function useCurrentOrigin(): Nullishable<string> {
       return browser.runtime.getURL("");
     }
 
-    if (isPageEditor()) {
+    if (isPageEditorTopFrame()) {
       return "devtools://devtools";
     }
 

src/errors/contextInvalidated.ts

@@ -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";
 
@@ -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;
   }

src/sidebar/ConnectedSidebar.tsx

@@ -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";
@@ -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 &&

src/sidebar/connectedTarget.tsx

@@ -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,
@@ -38,7 +45,7 @@ async function getConnectedTabIdMv2() {
 }
 
 export const getConnectedTabId = once(
-  isMV3() ? getConnectedTabIdMv3 : getConnectedTabIdMv2,
+  isMV3() ? getConnectedTabIdForMV3SidebarTopFrame : getConnectedTabIdMv2,
 );
 
 /**
@@ -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> {

src/tinyPages/RestrictedUrlPopupApp.tsx

@@ -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";
 
@@ -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();
   }
 }

src/utils/expectContext.ts

@@ -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";
 }
 
@@ -56,9 +64,9 @@ const contextMap = {
   web: isWebPage,
   extension: isExtensionContext,
   background: isBackground,
-  pageEditor: isPageEditor,
+  pageEditor: isPageEditorTopFrame,
   contentScript: isContentScript,
-  sidebar: isBrowserSidebar,
+  sidebar: isBrowserSidebarTopFrame,
 } as const;
 
 /**

src/utils/sidePanelUtils.ts

@@ -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";
@@ -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.',
     );