diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/PageViewer/PageViewer.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/PageViewer/PageViewer.tsx index 6bf2d0cd49c..66e3175467a 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/PageViewer/PageViewer.tsx +++ b/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(); @@ -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} /> diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/PagePane/PagePane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/PagePane/PagePane.tsx index cf26c384864..abfa7862bdf 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/PagePane/PagePane.tsx +++ b/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["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)} /> ); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx index 856a7572eaa..0ff6bc0ee50 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx @@ -841,6 +841,7 @@ export function usePaneRegistry({ ), renderPane: (ctx: RendererContext) => ( diff --git a/packages/shared/src/page-comments-runtime.test.ts b/packages/shared/src/page-comments-runtime.test.ts index 0058dd697ee..59dbdf1383b 100644 --- a/packages/shared/src/page-comments-runtime.test.ts +++ b/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 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([]); + }); +}); diff --git a/packages/shared/src/page-comments-runtime.ts b/packages/shared/src/page-comments-runtime.ts index 9aa1b142023..f1376b9a421 100644 --- a/packages/shared/src/page-comments-runtime.ts +++ b/packages/shared/src/page-comments-runtime.ts @@ -29,8 +29,16 @@ export const FRAME_CHANNEL = "superset-comments/frame"; export const PENDING_ANCHOR_ID = "superset-pending-anchor"; +export interface PageLinkClick { + url: string; + metaKey: boolean; + ctrlKey: boolean; + shiftKey: boolean; +} + export type HostMessageBody = | { type: "ready" } + | { type: "set-link-handling"; enabled: boolean } | { type: "enable-pinch-zoom" } | { type: "set-mode"; enabled: boolean; locked: boolean } | { type: "track"; anchors: { id: string; anchor: CommentAnchor }[] } @@ -39,6 +47,7 @@ export type HostMessageBody = export type HostMessage = HostMessageBody & { channel: typeof HOST_CHANNEL }; export type FrameMessage = + | ({ channel: typeof FRAME_CHANNEL; type: "link-click" } & PageLinkClick) | { channel: typeof FRAME_CHANNEL; type: "ready" } | { channel: typeof FRAME_CHANNEL; @@ -72,6 +81,7 @@ export const PAGE_COMMENTS_RUNTIME_SOURCE = `(() => { const FRAME = ${JSON.stringify(FRAME_CHANNEL)}; let enabled = false; + let handleLinks = false; let locked = false; let lockedAtPointerDown = false; let tracked = []; @@ -227,12 +237,40 @@ export const PAGE_COMMENTS_RUNTIME_SOURCE = `(() => { true, ); + const forwardLink = (event) => { + if (!handleLinks || event.defaultPrevented || event.button > 1) return; + const anchor = event.target?.closest?.("a[href]"); + if (!anchor || anchor.hasAttribute("download")) return; + const href = anchor.getAttribute("href")?.trim(); + if (!href) return; + let url; + try { url = new URL(href, document.baseURI); } catch { return; } + if (!["http:", "https:", "mailto:", "tel:"].includes(url.protocol)) return; + const current = new URL(location.href); + if (url.href.includes("#") && url.origin === current.origin && url.pathname === current.pathname && url.search === current.search) return; + event.preventDefault(); + event.stopPropagation(); + post({ + type: "link-click", + url: url.href, + metaKey: Boolean(event.metaKey), + ctrlKey: Boolean(event.ctrlKey), + shiftKey: Boolean(event.shiftKey), + }); + }; + + document.addEventListener("auxclick", (event) => { + if (event.button !== 1) return; + if (enabled) { event.preventDefault(); return; } + forwardLink(event); + }, true); + document.addEventListener( "click", (event) => { const dismissing = lockedAtPointerDown; lockedAtPointerDown = false; - if (!enabled) return; + if (!enabled) { forwardLink(event); return; } event.preventDefault(); event.stopPropagation(); if (locked || dismissing) return; @@ -302,6 +340,7 @@ export const PAGE_COMMENTS_RUNTIME_SOURCE = `(() => { const data = event.data; if (!data || data.channel !== HOST) return; if (data.type === "enable-pinch-zoom" && event.source === parent) pinchZoom.enable(); + if (data.type === "set-link-handling" && event.source === parent) handleLinks = Boolean(data.enabled); if (data.type === "ready") post({ type: "ready" }); if (data.type === "set-mode") { enabled = Boolean(data.enabled); diff --git a/packages/ui/src/components/PageComments/components/PageCommentsView/PageCommentsView.tsx b/packages/ui/src/components/PageComments/components/PageCommentsView/PageCommentsView.tsx index f45f57ad718..557f173c1f7 100644 --- a/packages/ui/src/components/PageComments/components/PageCommentsView/PageCommentsView.tsx +++ b/packages/ui/src/components/PageComments/components/PageCommentsView/PageCommentsView.tsx @@ -8,6 +8,7 @@ import { type FrameRect, HOST_CHANNEL, type HostMessageBody, + type PageLinkClick, PENDING_ANCHOR_ID, } from "@superset/shared/page-comments-runtime"; import { @@ -38,6 +39,7 @@ interface PageCommentsViewProps { * host that focuses on click (a pane) hears about it here instead. */ onFramePointerDown?: () => void; + onLinkClick?: (click: PageLinkClick) => void; } export function PageCommentsView({ @@ -47,7 +49,10 @@ export function PageCommentsView({ pinchZoomEnabled = false, onScrollYChange, onFramePointerDown, + onLinkClick, }: PageCommentsViewProps) { + const onLinkClickRef = useRef(onLinkClick); + onLinkClickRef.current = onLinkClick; const scrollYRef = useRef(initialScrollY ?? 0); const onScrollYChangeRef = useRef(onScrollYChange); onScrollYChangeRef.current = onScrollYChange; @@ -196,7 +201,18 @@ export function PageCommentsView({ height: rect.height * v.scale, }; }; + if ( + data.type === "link-click" && + typeof data.url === "string" && + /^(https?:|mailto:|tel:)/i.test(data.url) + ) { + onLinkClickRef.current?.(data); + } if (data.type === "ready") { + send({ + type: "set-link-handling", + enabled: Boolean(onLinkClickRef.current), + }); if (pinchZoomEnabled) send({ type: "enable-pinch-zoom" }); setReadySrc(src); setFrameEpoch((epoch) => epoch + 1);