fix(desktop): open page links without blanking the page

apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/PageViewer/PageViewer.tsx

@@ -2,6 +2,7 @@ import { useLingui } from "@lingui/react/macro";
 import { usePageComments } from "@superset/cloud-client";
 import { errorMessage } from "@superset/i18n/errors";
 import { pageCommentUser } from "@superset/shared/page-comments";
+import type { PageLinkClick } from "@superset/shared/page-comments-runtime";
 import {
 	AllCommentsButton,
 	CommentProvider,
@@ -15,6 +16,7 @@ import { TRPCClientError } from "@trpc/client";
 import { useEffect, useMemo, useRef } from "react";
 import { authClient } from "renderer/lib/auth-client";
 import { cloudTrpc } from "renderer/lib/cloud-trpc";
+import { electronTrpcClient } from "renderer/lib/trpc-client";
 import { PageViewerMessage } from "./components/PageViewerMessage";
 
 const scrollPositions = new Map<string, number>();
@@ -33,6 +35,7 @@ interface PageViewerProps {
 	onCommentsEnabledChange: (enabled: boolean) => void;
 	onResolved?: (page: ResolvedPage) => void;
 	onFramePointerDown?: () => void;
+	onLinkClick?: (click: PageLinkClick) => void;
 	version?: number | null;
 	onExitPreview?: () => void;
 }
@@ -45,6 +48,11 @@ export function PageViewer({
 	onCommentsEnabledChange,
 	onResolved,
 	onFramePointerDown,
+	onLinkClick = (click) => {
+		void electronTrpcClient.external.openUrl
+			.mutate(click.url)
+			.catch(console.error);
+	},
 	version,
 	onExitPreview,
 }: PageViewerProps) {
@@ -144,6 +152,7 @@ export function PageViewer({
 							initialScrollY={scrollPositions.get(scrollKey) ?? 0}
 							onScrollYChange={(y) => scrollPositions.set(scrollKey, y)}
 							onFramePointerDown={onFramePointerDown}
+							onLinkClick={onLinkClick}
 						/>
 					</div>
 					<AllCommentsButton />

apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/PagePane/PagePane.tsx

@@ -1,10 +1,14 @@
+import type { RendererContext } from "@superset/panes";
 import { useCallback, useRef } from "react";
+import { useTerminalUrlPolicy } from "renderer/lib/clickPolicy";
 import { PageViewer } from "renderer/routes/_authenticated/_dashboard/components/PageViewer";
-import type { PagePaneData } from "../../../../types";
+import type { PagePaneData, PaneViewerData } from "../../../../types";
 import { usePagePaneUi } from "../../hooks/usePagePaneUi";
+import { runUrlLinkAction } from "../../utils/runTerminalLinkAction";
 
 interface PagePaneProps {
 	data: PagePaneData;
+	store: RendererContext<PaneViewerData>["store"];
 	paneId: string;
 	onDataChange: (data: PagePaneData) => void;
 	/**
@@ -17,10 +21,12 @@ interface PagePaneProps {
 
 export function PagePane({
 	data,
+	store,
 	paneId,
 	onDataChange,
 	onFocus,
 }: PagePaneProps) {
+	const urlPolicy = useTerminalUrlPolicy();
 	const {
 		commentsEnabled,
 		setCommentsEnabled,
@@ -53,6 +59,16 @@ export function PagePane({
 			onCommentsEnabledChange={setCommentsEnabled}
 			onResolved={handleResolved}
 			onFramePointerDown={onFocus}
+			onLinkClick={(click) => {
+				const action = /^(mailto:|tel:)/i.test(click.url)
+					? "external"
+					: (urlPolicy.getAction(click) ??
+						(!click.metaKey && !click.ctrlKey && !click.shiftKey
+							? "pane"
+							: null));
+				if (action)
+					runUrlLinkAction({ store, isPagesEnabled: true }, click.url, action);
+			}}
 			onExitPreview={() => setPreviewVersion(null)}
 		/>
 	);

apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx

@@ -841,6 +841,7 @@ export function usePaneRegistry({
 							),
 							renderPane: (ctx: RendererContext<PaneViewerData>) => (
 								<PagePane
+									store={ctx.store}
 									data={ctx.pane.data as PagePaneData}
 									paneId={ctx.pane.id}
 									onDataChange={(data) =>

packages/shared/src/page-comments-runtime.test.ts

@@ -3,10 +3,11 @@ import { createContext, runInContext } from "node:vm";
 import { PAGE_COMMENTS_RUNTIME_SOURCE } from "./page-comments-runtime";
 
 interface Posted {
+	[key: string]: unknown;
 	type: string;
 }
 
-function setup() {
+function setup({ baseUrl = "https://page.example/view" } = {}) {
 	const listeners = new Map<string, (event: unknown) => void>();
 	const posted: Posted[] = [];
 	let frames: (() => void)[] = [];
@@ -29,11 +30,14 @@ function setup() {
 	};
 
 	const context = createContext({
+		URL,
+		location: { href: "https://page.example/view" },
 		scrollY: 0,
 		scrollX: 0,
 		innerWidth: 400,
 		innerHeight: 800,
 		document: {
+			baseURI: baseUrl,
 			documentElement: { style: {}, ...element },
 			body: { ...element },
 			addEventListener: (type: string, fn: (event: unknown) => void) =>
@@ -85,6 +89,63 @@ function setup() {
 
 	return {
 		posted,
+		configureLinks: (enabled = true, trusted = true) =>
+			listeners.get("message")?.({
+				source: trusted ? context.parent : {},
+				data: {
+					channel: "superset-comments/host",
+					type: "set-link-handling",
+					enabled,
+				},
+			}),
+		commentMode: () =>
+			listeners.get("message")?.({
+				data: {
+					channel: "superset-comments/host",
+					type: "set-mode",
+					enabled: true,
+					locked: true,
+				},
+			}),
+		clickLink: (
+			href: string,
+			options: {
+				metaKey?: boolean;
+				ctrlKey?: boolean;
+				shiftKey?: boolean;
+				button?: number;
+				download?: boolean;
+				defaultPrevented?: boolean;
+				svg?: boolean;
+				noAnchor?: boolean;
+			} = {},
+		) => {
+			let prevented = false;
+			const event = {
+				button: 0,
+				...options,
+				target: {
+					closest: () =>
+						options.noAnchor
+							? null
+							: {
+									href: options.svg
+										? { baseVal: href }
+										: new URL(href, baseUrl).href,
+									getAttribute: () => href,
+									hasAttribute: () => options.download ?? false,
+								},
+				},
+				preventDefault: () => {
+					prevented = true;
+				},
+				stopPropagation: () => {},
+			};
+			listeners.get(
+				options.button === 1 ? "document:auxclick" : "document:click",
+			)?.(event);
+			return prevented;
+		},
 		advance,
 		track: (count: number) => {
 			listeners.get("message")?.({
@@ -160,3 +221,143 @@ describe("page comments runtime, scroll cost", () => {
 		expect(rects.length).toBe(FRAMES);
 	});
 });
+
+describe("page links", () => {
+	test("prevents frame navigation and forwards the resolved URL and modifiers", () => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(
+			page.clickLink("https://example.com/job", {
+				metaKey: true,
+				shiftKey: true,
+			}),
+		).toBe(true);
+		expect(page.posted).toEqual([
+			{
+				channel: "superset-comments/frame",
+				type: "link-click",
+				url: "https://example.com/job",
+				metaKey: true,
+				ctrlKey: false,
+				shiftKey: true,
+			},
+		]);
+	});
+	test("requires the parent to opt in", () => {
+		const page = setup();
+		expect(page.clickLink("https://example.com")).toBe(false);
+		page.configureLinks(true, false);
+		expect(page.clickLink("https://example.com")).toBe(false);
+		page.configureLinks();
+		expect(page.clickLink("https://example.com")).toBe(true);
+		page.configureLinks(false);
+		expect(page.clickLink("https://example.com")).toBe(false);
+	});
+	test("keeps local anchors, downloads, and unsupported schemes in the page", () => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		for (const href of [
+			"#section",
+			"javascript:void(0)",
+			"data:text/html,test",
+		]) {
+			expect(page.clickLink(href)).toBe(false);
+		}
+		expect(page.clickLink("https://example.com/file", { download: true })).toBe(
+			false,
+		);
+		expect(page.posted).toEqual([]);
+	});
+	test("forwards relative URLs, middle clicks, and external protocols", () => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(page.clickLink("/other", { button: 1, ctrlKey: true })).toBe(true);
+		expect(page.posted[0]).toMatchObject({
+			url: "https://page.example/other",
+			ctrlKey: true,
+		});
+		expect(page.clickLink("mailto:hello@example.com")).toBe(true);
+		expect(page.clickLink("tel:123")).toBe(true);
+	});
+	test("comment selection never opens links", () => {
+		const page = setup();
+		page.configureLinks();
+		page.commentMode();
+		page.clear();
+		expect(page.clickLink("https://example.com")).toBe(true);
+		expect(page.clickLink("https://example.com", { button: 1 })).toBe(true);
+		expect(page.posted).toEqual([]);
+	});
+});
+
+describe("page link edge cases", () => {
+	test.each([
+		"#",
+		"#section",
+		" # ",
+		"/view#",
+		"https://page.example/view#",
+		"https://page.example/view#section",
+	])("keeps same-document anchor %s in the frame", (href) => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(page.clickLink(href)).toBe(false);
+		expect(page.posted).toEqual([]);
+	});
+	test.each(["", "  "])("keeps empty href %j in the frame", (href) => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(page.clickLink(href)).toBe(false);
+		expect(page.posted).toEqual([]);
+	});
+	test("resolves links against the document base, including SVG anchors", () => {
+		const page = setup({ baseUrl: "https://other.example/docs/" });
+		page.configureLinks();
+		page.clear();
+		expect(page.clickLink("guide", { svg: true })).toBe(true);
+		expect(page.posted[0]).toMatchObject({
+			url: "https://other.example/docs/guide",
+		});
+		expect(page.clickLink("#section")).toBe(true);
+		expect(page.posted[1]).toMatchObject({
+			url: "https://other.example/docs/#section",
+		});
+	});
+	test.each([
+		"/other#section",
+		"/view?query=1#section",
+		"//other.example/view#",
+		"https://page.example/view%23section",
+	])("forwards distinct document %s", (href) => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(page.clickLink(href)).toBe(true);
+		expect(page.posted).toHaveLength(1);
+	});
+	test("ignores malformed URLs", () => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(page.clickLink("http://[", { svg: true })).toBe(false);
+		expect(page.posted).toEqual([]);
+	});
+	test("ignores cancelled events, right clicks, and non-link targets", () => {
+		const page = setup();
+		page.configureLinks();
+		page.clear();
+		expect(
+			page.clickLink("https://example.com", { defaultPrevented: true }),
+		).toBe(false);
+		expect(page.clickLink("https://example.com", { button: 2 })).toBe(false);
+		expect(page.clickLink("https://example.com", { noAnchor: true })).toBe(
+			false,
+		);
+		expect(page.posted).toEqual([]);
+	});
+});