From b59631d30ca5096ebcf1bd20b4f3cb1e21b6d0a3 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Thu, 11 Apr 2024 13:08:14 -0400 Subject: [PATCH] #8219: don't let frames control sidebar panel via setPanels (#8225) * #8219: don't let frames control sidebar * #8219: improve comment * Improve comment --- src/contentScript/contentScriptCore.ts | 10 ++++++++-- src/contentScript/sidebarController.tsx | 15 +++++++++++++++ src/{ => store}/sidebar/sidebarSlice.test.ts | 0 src/utils/iframeUtils.ts | 5 ++++- 4 files changed, 27 insertions(+), 3 deletions(-) rename src/{ => store}/sidebar/sidebarSlice.test.ts (100%) diff --git a/src/contentScript/contentScriptCore.ts b/src/contentScript/contentScriptCore.ts index 58a0c85baa..b2eb8d22cd 100644 --- a/src/contentScript/contentScriptCore.ts +++ b/src/contentScript/contentScriptCore.ts @@ -55,6 +55,7 @@ import { markDocumentAsFocusableByUser } from "@/utils/focusTracker"; import contentScriptPlatform from "@/contentScript/contentScriptPlatform"; import axios from "axios"; import { initDeferredLoginController } from "@/contentScript/integrations/deferredLoginController"; +import { isLoadedInIframe } from "@/utils/iframeUtils"; setPlatform(contentScriptPlatform); @@ -101,8 +102,13 @@ export async function init(): Promise { initSidebarFocusEvents(); void initSidebarActivation(); - // Update `sidePanel` - void renderPanelsIfVisible(); + if (!isLoadedInIframe()) { + // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/8209 + // Reset panels in `sidePanel` on content script initialization (which happens on non-SPA reload/navigation). + // That's the safe behavior for now to avoid showing stale data/invalid forms. + // Re-examine this call when we implement: https://www.notion.so/pixiebrix/0efdedb6c1e44b088e65106202e08c28 + void renderPanelsIfVisible(); + } // Let the partner page know initPartnerIntegrations(); diff --git a/src/contentScript/sidebarController.tsx b/src/contentScript/sidebarController.tsx index fd654c611f..33f0a40e66 100644 --- a/src/contentScript/sidebarController.tsx +++ b/src/contentScript/sidebarController.tsx @@ -210,6 +210,15 @@ export async function updateSidebar( export async function renderPanelsIfVisible(): Promise { expectContext("contentScript"); + if (isLoadedInIframe()) { + // The top-level frame is responsible for managing the panels for the sidebar. + // Include this isLoadedInIframe check as a stop gap to prevent accidental calls from iframes. + console.warn( + "sidebarController:renderPanelsIfVisible should not be called from a frame", + ); + return; + } + if (await isSidePanelOpen()) { void sidebarInThisTab.renderPanels(getTimedSequence(), panels); } else { @@ -298,6 +307,12 @@ export function removeExtensions(extensionIds: UUID[]): void { console.debug("sidebarController:removeExtensions", { extensionIds }); + // Avoid unnecessary messaging. More importantly, renderPanelsIfVisible should not be called from iframes. Iframes + // might call removeExtensions as part of cleanup + if (extensionIds.length === 0) { + return; + } + // `panels` is const, so replace the contents const current = panels.splice(0); panels.push(...current.filter((x) => !extensionIds.includes(x.extensionId))); diff --git a/src/sidebar/sidebarSlice.test.ts b/src/store/sidebar/sidebarSlice.test.ts similarity index 100% rename from src/sidebar/sidebarSlice.test.ts rename to src/store/sidebar/sidebarSlice.test.ts diff --git a/src/utils/iframeUtils.ts b/src/utils/iframeUtils.ts index 609a059286..f0c6300e24 100644 --- a/src/utils/iframeUtils.ts +++ b/src/utils/iframeUtils.ts @@ -19,9 +19,12 @@ * Returns true if this function is called within an IFrame */ -export const isLoadedInIframe = () => { +export const isLoadedInIframe = (): boolean => { // Using a try/catch block because in some cases trying to // access window.top from an iframe can result in a SecurityError + + // If updating this method to use the messenger/frameId, remember that frameId === 0 only for the active top-level + // frame: see https://developer.chrome.com/blog/extension-instantnav try { return window.self !== window.top; } catch {