Skip to content

Commit

Permalink
#8219: don't let frames control sidebar panel via setPanels (#8225)
Browse files Browse the repository at this point in the history
* #8219: don't let frames control sidebar

* #8219: improve comment

* Improve comment
  • Loading branch information
twschiller authored Apr 11, 2024
1 parent 6e2f02e commit b59631d
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/contentScript/contentScriptCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -101,8 +102,13 @@ export async function init(): Promise<void> {
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();
Expand Down
15 changes: 15 additions & 0 deletions src/contentScript/sidebarController.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ export async function updateSidebar(
export async function renderPanelsIfVisible(): Promise<void> {
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 {
Expand Down Expand Up @@ -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)));
Expand Down
File renamed without changes.
5 changes: 4 additions & 1 deletion src/utils/iframeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b59631d

Please sign in to comment.