From 9d0a15e9e696c377fe50ef819198915c6bab5de6 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Thu, 25 Apr 2024 10:37:37 -0400 Subject: [PATCH] #8228: run show sidebar in top-level frame (#8299) --- .../tests/runtime/sidebarController.spec.ts | 78 +++++++++++++++++++ .../messenger/strict/registration.ts | 6 +- src/contentScript/sidebarController.tsx | 68 +++++++++++----- src/utils/sidePanelUtils.ts | 12 ++- 4 files changed, 139 insertions(+), 25 deletions(-) create mode 100644 end-to-end-tests/tests/runtime/sidebarController.spec.ts diff --git a/end-to-end-tests/tests/runtime/sidebarController.spec.ts b/end-to-end-tests/tests/runtime/sidebarController.spec.ts new file mode 100644 index 0000000000..445e140627 --- /dev/null +++ b/end-to-end-tests/tests/runtime/sidebarController.spec.ts @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { test, expect } from "../../fixtures/extensionBase"; +import { ActivateModPage } from "../../pageObjects/extensionConsole/modsPage"; +// @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only +import { type Page, test as base } from "@playwright/test"; +import { getSidebarPage, isSidebarOpen } from "../../utils"; +import { MV } from "../../env"; + +test.describe("sidebar controller", () => { + test("can open sidebar immediately from iframe without focus dialog", async ({ + page, + extensionId, + }) => { + const modId = "@pixies/test/frame-sidebar-actions"; + + const modActivationPage = new ActivateModPage(page, extensionId, modId); + await modActivationPage.goto(); + await modActivationPage.clickActivateAndWaitForModsPageRedirect(); + + await page.goto("/frames-builder.html"); + + const frame = page.frameLocator("iframe"); + await frame.getByRole("link", { name: "Show Sidebar Immediately" }).click(); + + // Don't use getSidebarPage because it automatically clicks the MV3 focus dialog. + await expect(() => { + expect(isSidebarOpen(page, extensionId)).toBe(false); + }).toPass({ timeout: 5000 }); + }); + + test("shows focus dialog in top-level frame", async ({ + page, + extensionId, + }) => { + test.skip(MV === "2", "This test is only relevant for MV3"); + + const modId = "@pixies/test/frame-sidebar-actions"; + + const modActivationPage = new ActivateModPage(page, extensionId, modId); + await modActivationPage.goto(); + await modActivationPage.clickActivateAndWaitForModsPageRedirect(); + + await page.goto("/frames-builder.html"); + + const frame = page.frameLocator("iframe"); + + // Mod waits 7.5 seconds before running Show Sidebar brick to ensure the user gesture dialog is shown + await frame.getByRole("link", { name: "Show Sidebar after Wait" }).click(); + // eslint-disable-next-line playwright/no-wait-for-timeout -- match wait in the mod + await page.waitForTimeout(8000); + + // Focus dialog should be visible in the top-level frame + await expect(page.getByRole("button", { name: "OK" })).toBeVisible(); + + // The focus dialog should not be shown in the iframe. Check after checking the top-level frame + // because it's a positive check for the dialog being shown. + await expect(frame.getByRole("button", { name: "OK" })).not.toBeVisible(); + + // Will error if page/frame not available + await getSidebarPage(page, extensionId); + }); +}); diff --git a/src/contentScript/messenger/strict/registration.ts b/src/contentScript/messenger/strict/registration.ts index f43af5265c..a79b979423 100644 --- a/src/contentScript/messenger/strict/registration.ts +++ b/src/contentScript/messenger/strict/registration.ts @@ -19,7 +19,7 @@ import { hideMv2SidebarInTopFrame, - showSidebar, + showSidebarInTopFrame, sidebarWasLoaded, updateSidebar, removeExtensions as removeSidebars, @@ -54,7 +54,7 @@ declare global { FORM_CANCEL: typeof cancelForm; UPDATE_SIDEBAR: typeof updateSidebar; SIDEBAR_WAS_LOADED: typeof sidebarWasLoaded; - SHOW_SIDEBAR: typeof showSidebar; + SHOW_SIDEBAR: typeof showSidebarInTopFrame; HIDE_SIDEBAR: typeof hideMv2SidebarInTopFrame; REMOVE_SIDEBARS: typeof removeSidebars; HANDLE_MENU_ACTION: typeof handleMenuAction; @@ -84,7 +84,7 @@ export default function registerMessenger(): void { FORM_CANCEL: cancelForm, UPDATE_SIDEBAR: updateSidebar, SIDEBAR_WAS_LOADED: sidebarWasLoaded, - SHOW_SIDEBAR: showSidebar, + SHOW_SIDEBAR: showSidebarInTopFrame, HIDE_SIDEBAR: hideMv2SidebarInTopFrame, REMOVE_SIDEBARS: removeSidebars, HANDLE_MENU_ACTION: handleMenuAction, diff --git a/src/contentScript/sidebarController.tsx b/src/contentScript/sidebarController.tsx index 76fd19aa87..eae83f83ef 100644 --- a/src/contentScript/sidebarController.tsx +++ b/src/contentScript/sidebarController.tsx @@ -19,10 +19,12 @@ import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; import { expectContext } from "@/utils/expectContext"; import sidebarInThisTab from "@/sidebar/messenger/api"; +import * as contentScriptApi from "@/contentScript/messenger/strict/api"; import { isEmpty, throttle } from "lodash"; import { signalFromEvent } from "abort-utils"; import { SimpleEventTarget } from "@/utils/SimpleEventTarget"; import * as sidebarMv2 from "@/contentScript/sidebarDomControllerLite"; +import { getSidebarElement } from "@/contentScript/sidebarDomControllerLite"; import { type Except } from "type-fest"; import { type RunArgs, RunReason } from "@/types/runtimeTypes"; import { type UUID } from "@/types/stringTypes"; @@ -30,8 +32,8 @@ import { type RegistryId } from "@/types/registryTypes"; import { type ModComponentRef } from "@/types/modComponentTypes"; import type { ActivatePanelOptions, - ModActivationPanelEntry, FormPanelEntry, + ModActivationPanelEntry, PanelEntry, PanelPayload, TemporaryPanelEntry, @@ -41,18 +43,29 @@ import { getFormPanelSidebarEntries } from "@/platform/forms/formController"; import { memoizeUntilSettled } from "@/utils/promiseUtils"; import { getTimedSequence } from "@/types/helpers"; import { isMV3 } from "@/mv3/api"; -import { getErrorMessage } from "@/errors/errorHelpers"; import { focusCaptureDialog } from "@/contentScript/focusCaptureDialog"; import { isLoadedInIframe } from "@/utils/iframeUtils"; import { showMySidePanel } from "@/background/messenger/strict/api"; -import { getSidebarElement } from "@/contentScript/sidebarDomControllerLite"; import focusController from "@/utils/focusController"; import selectionController from "@/utils/selectionController"; -import { messenger } from "webext-messenger"; -import { getSidebarTargetForCurrentTab } from "@/utils/sidePanelUtils"; +import { getTopLevelFrame, messenger } from "webext-messenger"; +import { + getSidebarTargetForCurrentTab, + isUserGestureRequiredError, +} from "@/utils/sidePanelUtils"; const HIDE_SIDEBAR_EVENT_NAME = "pixiebrix:hideSidebar"; +/** + * Event listeners triggered when the sidebar shows and is ready to receive messages. + */ +export const sidebarShowEvents = new SimpleEventTarget(); + +// eslint-disable-next-line local-rules/persistBackgroundData -- Unused there +const panels: PanelEntry[] = []; + +let modActivationPanelEntry: ModActivationPanelEntry | null = null; + /* * Only one check at a time * Cannot throttle because subsequent checks need to be able to be made immediately @@ -88,30 +101,37 @@ const pingSidebar = memoizeUntilSettled( }, 1000) as () => Promise, ); -/** - * Event listeners triggered when the sidebar shows and is ready to receive messages. - */ -export const sidebarShowEvents = new SimpleEventTarget(); - export function sidebarWasLoaded(): void { sidebarShowEvents.emit({ reason: RunReason.MANUAL }); } -// eslint-disable-next-line local-rules/persistBackgroundData -- Unused there -const panels: PanelEntry[] = []; - -let modActivationPanelEntry: ModActivationPanelEntry | null = null; - /** - * Attach the sidebar to the page if it's not already attached. Then re-renders all panels. + * Content script handler for showing the sidebar in the top-level frame. Regular callers should call + * showSidebar instead, which handles calls from iframes. + * + * - Resolves when the sidebar is initialized (responds to a ping) + * - Shows focusCaptureDialog if a user gesture is required + * + * @see showSidebar + * @see pingSidebar + * @throws Error if the sidebar ping fails or does not respond in time */ -export async function showSidebar(): Promise { +// Don't memoizeUntilSettled this method. focusCaptureDialog is memoized which prevents this method from showing +// the focus dialog from multiple times. By allowing multiple concurrent calls to showSidebarInTopFrame, +// a subsequent call might succeed, which will then automatically close the focusCaptureDialog (via it's abort signal) +export async function showSidebarInTopFrame() { reportEvent(Events.SIDEBAR_SHOW); + + if (isLoadedInIframe()) { + console.warn("showSidebarInTopFrame should not be called in an iframe"); + } + + // Defensively handle accidental calls from iframes if (isMV3() || isLoadedInIframe()) { try { await showMySidePanel(); } catch (error) { - if (!getErrorMessage(error).includes("user gesture")) { + if (!isUserGestureRequiredError(error)) { throw error; } @@ -137,6 +157,18 @@ export async function showSidebar(): Promise { } } +/** + * Attach the sidebar to the page if it's not already attached. Safe to call from any frame. Resolves when the + * sidebar is initialized. + * @see showSidebarInTopFrame + */ +export async function showSidebar(): Promise { + // Could consider explicitly calling showSidebarInTopFrame directly if we're already in the top frame. + // But the messenger will already handle that case automatically. + const topLevelFrame = await getTopLevelFrame(); + await contentScriptApi.showSidebar(topLevelFrame); +} + /** * Force-show the panel for the given extension id * @param extensionId the extension UUID diff --git a/src/utils/sidePanelUtils.ts b/src/utils/sidePanelUtils.ts index 63b35dfb63..d16d7024f6 100644 --- a/src/utils/sidePanelUtils.ts +++ b/src/utils/sidePanelUtils.ts @@ -24,6 +24,13 @@ import { type PageTarget, messenger, getThisFrame } from "webext-messenger"; import { isContentScript } from "webext-detect-page"; import { showSidebar } from "@/contentScript/messenger/strict/api"; +/** + * Returns true if an error showing sidebar is due to a missing user gesture. + */ +export function isUserGestureRequiredError(error: unknown): boolean { + return getErrorMessage(error).includes("user gesture"); +} + export async function openSidePanel(tabId: number): Promise { if (isBrowserSidebar()) { console.warn( @@ -60,10 +67,7 @@ async function openSidePanelMv3(tabId: number): Promise { // it's still part of a user gesture. // If it's not, it will throw an error *even if the side panel is already open*. // The following code silences that error iff the side panel is already open. - if ( - getErrorMessage(error).includes("user gesture") && - (await isSidePanelOpen(tabId)) - ) { + if (isUserGestureRequiredError(error) && (await isSidePanelOpen(tabId))) { // The `openSidePanel` call was not required in the first place, the error can be silenced // TODO: After switching to MV3, verify whether we drop that `openSidePanel` call return;