From 68a6a4d9d3d16b437595ec9aee3519b11f2165b3 Mon Sep 17 00:00:00 2001 From: robinvrd Date: Fri, 30 Oct 2026 16:14:54 +0100 Subject: [PATCH 1/7] feat: add host-controlled external links for data apps Add ExternalLink and an opt-in navigation route with HTTPS validation, origin policy, and host-owned confirmation. Preserve the bundle sandbox and wire the local preview host. Validation: build, static checks, reference check, starter check, and 1,044 tests pass. Packed consumer installation is blocked by npm returning 404 for es-toolkit@1.53.1. --- REFERENCE.md | 6 ++ dev/fixtures/bundle-host.tsx | 6 ++ docs/app-authoring.md | 18 ++++ docs/embed.md | 31 ++++++ packages/data-app-bun/src/preview/host.tsx | 6 ++ packages/data-app-embed/src/external-link.ts | 88 +++++++++++++++++ packages/data-app-embed/src/index.ts | 2 + .../data-app-react/src/authoring/index.ts | 2 + packages/data-app-react/src/index.ts | 2 + .../data-app-react/src/ui/ExternalLink.tsx | 98 +++++++++++++++++++ packages/data-app/src/core/contract.ts | 7 ++ packages/data-app/src/core/external-link.ts | 37 +++++++ tests/public-api/apps-external-links.test.ts | 74 ++++++++++++++ tests/public-api/external-links.test.ts | 54 ++++++++++ tests/public-api/fixtures/bundle-app.tsx | 9 +- tests/public-api/fixtures/bundle-host.tsx | 7 ++ 16 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 packages/data-app-embed/src/external-link.ts create mode 100644 packages/data-app-react/src/ui/ExternalLink.tsx create mode 100644 packages/data-app/src/core/external-link.ts create mode 100644 tests/public-api/apps-external-links.test.ts create mode 100644 tests/public-api/external-links.test.ts diff --git a/REFERENCE.md b/REFERENCE.md index 65e36a30..1bb0e762 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -708,6 +708,12 @@ ExplorationActionControl.indicator?: 'chevron' | 'ellipsis' — Chart labels use ExplorationActionControl.ref?: Ref — Shares this menu trigger with an item's context and keyboard actions. ExplorationActionControl.className?: string ExplorationActionControlProps (type, @altertable/data-app-react/ui): = props listed as ExplorationActionControl.* +ExternalLink (component, @altertable/data-app-react): (props: ExternalLinkProps) — Open HTTPS links through the host when embedded, or a native new tab when standalone. +ExternalLink.href: string +ExternalLink.children: ReactNode +ExternalLink.className?: string +ExternalLink.title?: string +ExternalLinkProps (type, @altertable/data-app-react): = props listed as ExternalLink.* FilterActions (component, @altertable/data-app-react/ui): (props: FilterActionsProps) FilterActions.onClear?: (() => void) FilterActions.onApply: () => void [variant 1 of 2] diff --git a/dev/fixtures/bundle-host.tsx b/dev/fixtures/bundle-host.tsx index 32122ec3..5a7b4e96 100644 --- a/dev/fixtures/bundle-host.tsx +++ b/dev/fixtures/bundle-host.tsx @@ -2,6 +2,7 @@ import { type DataAppLogger, type DataAppStatus, createNavigationHandler, + createExternalLinkHandler, createSqlQueryHandler, } from '@altertable/data-app-embed'; import { DataAppBridge } from '@altertable/data-app-embed/react'; @@ -23,6 +24,7 @@ import { annotationSendRoute, annotationUpdateRoute, createMessageRouter, + openExternalLinkRoute, defineMessageRoute, sqlQueryRoute, DataSourceError, @@ -219,6 +221,8 @@ function Host() { ...(hostOptions.has('lazy') ? { loading: 'lazy' as const } : {}), }; const forward = createHttpTransport(); + const [openExternalLink] = useState(() => createExternalLinkHandler()); + const fileExportRoute = defineMessageRoute({ input(value: unknown): { filename: string; blob: Blob } { if (!value || typeof value !== 'object') @@ -260,6 +264,7 @@ function Host() { 'annotation:send': annotationSendRoute, 'annotation:update': annotationUpdateRoute, 'data:sql': sqlQueryRoute, + 'navigation:openExternal': openExternalLinkRoute, 'export:csv': fileExportRoute, 'export:zip': fileExportRoute, }, @@ -275,6 +280,7 @@ function Host() { if (annotationFailure) throw new Error('Fixture failure'); return annotationsHost.handlers['annotation:draft'](draft); }, + 'navigation:openExternal': openExternalLink, 'export:csv': downloadExport, 'export:zip': downloadExport, 'data:sql': createSqlQueryHandler(async () => ({ diff --git a/docs/app-authoring.md b/docs/app-authoring.md index f2801a21..877c5de2 100644 --- a/docs/app-authoring.md +++ b/docs/app-authoring.md @@ -52,6 +52,24 @@ Keep credentials, authorization, and enforced query limits in the host/backend. The starter supplies transport and mounting; change them only for an explicit integration task. +## Link to external pages + +Use `` for HTTPS destinations in widget content: + +```tsx +import { ExternalLink } from '@altertable/data-app-react'; + +function ReportLink() { + return ( + + View full report + + ); +} +``` + +Embedded hosts must enable the [external link route](embed.md#external-links). + ## Verify the app Run the authoring workflow's validation, then inspect the app: diff --git a/docs/embed.md b/docs/embed.md index ca76d773..11ba7635 100644 --- a/docs/embed.md +++ b/docs/embed.md @@ -86,6 +86,37 @@ Host Back/Forward events publish opaque host state through the bridge. The app's optional navigation adapter interprets search/hash and synchronizes controls; opaque apps keep that state in memory rather than modifying their document URL. +## External links + +Register the external link route to let apps request HTTPS navigation without +relaxing the iframe sandbox: + +```ts +import { + createMessageRouter, + openExternalLinkRoute, +} from '@altertable/data-app/contract'; +import { createExternalLinkHandler } from '@altertable/data-app-embed'; + +const router = createMessageRouter( + { 'navigation:openExternal': openExternalLinkRoute }, + { + 'navigation:openExternal': createExternalLinkHandler({ + allowedOrigins: ['https://example.com'], + }), + } +); +// Pass router.dispatch as the bridge's onMessage handler. +``` + +The handler shows the destination in a host-owned dialog. The user opens a new tab +or cancels; the link uses `noopener noreferrer`. Omit `allowedOrigins` to accept +any HTTPS destination after confirmation, or pass an empty list to deny all. +The initial origin check does not constrain redirects at the destination. +URLs can carry app data, so choose permitted origins according to the host's data +policy. Custom handlers must enforce their own policy and user interaction. +Unregistered hosts reject the request; the component displays an error. + ## Status and diagnostics `onStatusChange` receives `connecting`, `connected`, `ready`, `failed`, or diff --git a/packages/data-app-bun/src/preview/host.tsx b/packages/data-app-bun/src/preview/host.tsx index 3fa9cd43..0852c79d 100644 --- a/packages/data-app-bun/src/preview/host.tsx +++ b/packages/data-app-bun/src/preview/host.tsx @@ -1,6 +1,7 @@ import { createDataAppRepairSession, createNavigationHandler, + createExternalLinkHandler, createSqlQueryHandler, type DataAppStatus, } from '@altertable/data-app-embed'; @@ -11,6 +12,7 @@ import { import { DataSourceError, createMessageRouter, + openExternalLinkRoute, defineMessageRoute, navigationUpdateRoute, sqlQueryRoute, @@ -19,6 +21,8 @@ import { downloadBlob } from '@altertable/data-app/internal'; import { StrictMode, useEffect, useState } from 'react'; import { createRoot } from 'react-dom/client'; +const openExternalLink = createExternalLinkHandler(); + const fileExportRoute = defineMessageRoute({ input(value: unknown): { filename: string; blob: Blob } { const file = value as { filename?: unknown; blob?: unknown } | null; @@ -41,6 +45,7 @@ const router = createMessageRouter( { 'data:sql': sqlQueryRoute, 'navigation:update': navigationUpdateRoute, + 'navigation:openExternal': openExternalLinkRoute, 'export:csv': fileExportRoute, 'export:zip': fileExportRoute, }, @@ -63,6 +68,7 @@ const router = createMessageRouter( }, })), 'navigation:update': createNavigationHandler(), + 'navigation:openExternal': openExternalLink, 'export:csv': download, 'export:zip': download, } diff --git a/packages/data-app-embed/src/external-link.ts b/packages/data-app-embed/src/external-link.ts new file mode 100644 index 00000000..eb6fa63b --- /dev/null +++ b/packages/data-app-embed/src/external-link.ts @@ -0,0 +1,88 @@ +import { + openExternalLinkRoute, + MessageRoutingError, + type OpenExternalLink, + type ExternalLinkResult, + type MessageContext, +} from '@altertable/data-app/contract'; + +/** Show a host-owned confirmation before opening a new tab. No iframe permissions are added. */ +export function createExternalLinkHandler({ + window: host = window, + allowedOrigins, +}: { + window?: Window; + /** Exact HTTPS origins. Omit to allow any HTTPS destination after confirmation. */ + allowedOrigins?: readonly string[]; +} = {}) { + const origins = + allowedOrigins && + new Set( + allowedOrigins.map(origin => { + const url = new URL(origin); + if (url.protocol !== 'https:' || url.origin !== origin) + throw new Error('Expected an exact HTTPS origin.'); + return origin; + }) + ); + let pending = false; + return async function openExternalLink( + input: OpenExternalLink, + { signal }: MessageContext + ): Promise { + signal.throwIfAborted(); + const { url } = openExternalLinkRoute.input(input); + if (origins && !origins.has(new URL(url).origin)) + throw new MessageRoutingError( + 'forbidden', + 'This external destination is not allowed.' + ); + if (pending) + throw new MessageRoutingError( + 'external_link_busy', + 'An external link is already awaiting confirmation.' + ); + pending = true; + const document = host.document; + const previousFocus = document.activeElement; + const dialog = document.createElement('dialog'); + dialog.setAttribute('aria-label', 'Open external link'); + const description = document.createElement('p'); + description.textContent = 'This app wants to open an external website:'; + const destination = document.createElement('p'); + destination.textContent = url; + destination.style.overflowWrap = 'anywhere'; + const link = document.createElement('a'); + link.href = url; + link.target = '_blank'; + link.rel = 'noopener noreferrer'; + link.textContent = `Open ${new URL(url).host}`; + const cancel = document.createElement('button'); + cancel.textContent = 'Cancel'; + cancel.autofocus = true; + dialog.append(description, destination, link, ' ', cancel); + document.body.append(dialog); + let cleanup: (() => void) | undefined; + try { + return await new Promise((resolve, reject) => { + function abort() { + reject(signal.reason); + } + signal.addEventListener('abort', abort, { once: true }); + dialog.addEventListener('close', () => resolve('cancelled')); + cancel.addEventListener('click', () => dialog.close()); + link.addEventListener('click', () => resolve('opened')); + // Remove the abort listener when the dialog settles, including showModal failures. + cleanup = () => signal.removeEventListener('abort', abort); + dialog.showModal(); + }); + } finally { + cleanup?.(); + dialog.close(); + dialog.remove(); + pending = false; + if (previousFocus && 'focus' in previousFocus) + (previousFocus as HTMLElement).focus(); + } + }; +} diff --git a/packages/data-app-embed/src/index.ts b/packages/data-app-embed/src/index.ts index 6043bc28..19917088 100644 --- a/packages/data-app-embed/src/index.ts +++ b/packages/data-app-embed/src/index.ts @@ -23,3 +23,5 @@ export { type DataAppRepairOptions, type DataAppRepairStatus, } from './repair.ts'; + +export { createExternalLinkHandler } from './external-link.ts'; diff --git a/packages/data-app-react/src/authoring/index.ts b/packages/data-app-react/src/authoring/index.ts index 840e3759..a56880e1 100644 --- a/packages/data-app-react/src/authoring/index.ts +++ b/packages/data-app-react/src/authoring/index.ts @@ -106,3 +106,5 @@ export type { SectionHeadingProps } from '../ui/SectionHeading.tsx'; export { DateDisplay } from '../ui/DateDisplay.tsx'; export type { DateDisplayProps } from '../ui/DateDisplay.tsx'; export { useDateFormatting } from '../date-formatting.ts'; + +export { ExternalLink, type ExternalLinkProps } from '../ui/ExternalLink.tsx'; diff --git a/packages/data-app-react/src/index.ts b/packages/data-app-react/src/index.ts index ccc52092..dadfc794 100644 --- a/packages/data-app-react/src/index.ts +++ b/packages/data-app-react/src/index.ts @@ -108,3 +108,5 @@ export { injectDataAppAnnotationStyles, } from './host/index.ts'; export type { AnnotationBarHandle, AnnotationBarProps } from './host/index.ts'; + +export { ExternalLink, type ExternalLinkProps } from './authoring/index.ts'; diff --git a/packages/data-app-react/src/ui/ExternalLink.tsx b/packages/data-app-react/src/ui/ExternalLink.tsx new file mode 100644 index 00000000..92feee66 --- /dev/null +++ b/packages/data-app-react/src/ui/ExternalLink.tsx @@ -0,0 +1,98 @@ +import { + createMessageClient, + getDataAppTransport, +} from '@altertable/data-app/client'; +import { + openExternalLinkRoute, + parseExternalUrl, +} from '@altertable/data-app/contract'; +import { + useEffect, + useRef, + useState, + type ReactNode, + type MouseEvent, +} from 'react'; + +export type ExternalLinkProps = { + href: string; + children: ReactNode; + className?: string; + title?: string; +}; + +/** Open HTTPS links through the host when embedded, or a native new tab when standalone. */ +export function ExternalLink({ + href, + children, + className, + title, +}: ExternalLinkProps) { + const [error, setError] = useState(false); + const [pending, setPending] = useState(false); + const request = useRef(undefined); + useEffect(() => () => request.current?.abort(), []); + let url: string | undefined; + try { + url = parseExternalUrl(href); + } catch { + /* Invalid destinations are not clickable. */ + } + function open(event: MouseEvent) { + const transport = getDataAppTransport(); + if (!url || pending) { + event.preventDefault(); + return; + } + if (!transport) return; + event.preventDefault(); + if (request.current) return; + const controller = new AbortController(); + request.current = controller; + setPending(true); + setError(false); + const client = createMessageClient( + { 'navigation:openExternal': openExternalLinkRoute }, + transport.request + ); + void client + .request( + 'navigation:openExternal', + { url }, + { signal: controller.signal } + ) + .catch(() => { + if (!controller.signal.aborted) setError(true); + }) + .finally(() => { + request.current = undefined; + if (!controller.signal.aborted) setPending(false); + }); + } + return ( + <> + { + if (event.button === 1) open(event); + }} + > + {children} + + {(!url || error) && ( + + {!url + ? 'Invalid external link.' + : 'Could not open this link. The host may not support external links.'} + + )} + + ); +} diff --git a/packages/data-app/src/core/contract.ts b/packages/data-app/src/core/contract.ts index eceba9c7..9601d217 100644 --- a/packages/data-app/src/core/contract.ts +++ b/packages/data-app/src/core/contract.ts @@ -126,3 +126,10 @@ export type { DataAppAnnotationDraft, DataAppAnnotationPresentation, } from './annotations.ts'; + +export { + parseExternalUrl, + openExternalLinkRoute, + type OpenExternalLink, + type ExternalLinkResult, +} from './external-link.ts'; diff --git a/packages/data-app/src/core/external-link.ts b/packages/data-app/src/core/external-link.ts new file mode 100644 index 00000000..ac4ef349 --- /dev/null +++ b/packages/data-app/src/core/external-link.ts @@ -0,0 +1,37 @@ +import { defineMessageRoute } from './messages.ts'; + +/** Absolute HTTPS destinations only; URLs must not contain embedded credentials. */ +export function parseExternalUrl(value: unknown): string { + if ( + typeof value !== 'string' || + value.length > 8192 || + /[\s\\\p{Cc}\p{Cf}]/u.test(value) + ) + throw new Error('Expected an absolute HTTPS URL.'); + const url = new URL(value); + if ( + !/^https:\/\//i.test(value) || + url.protocol !== 'https:' || + url.username || + url.password + ) + throw new Error('Expected an HTTPS URL without credentials.'); + return url.href; +} + +export type OpenExternalLink = { url: string }; +export type ExternalLinkResult = 'opened' | 'cancelled'; + +/** Opt-in host route. Validation does not replace the host's destination policy. */ +export const openExternalLinkRoute = /* @__PURE__ */ defineMessageRoute({ + input(value: unknown): OpenExternalLink { + if (!value || typeof value !== 'object') + throw new Error('Invalid external link.'); + return { url: parseExternalUrl((value as OpenExternalLink).url) }; + }, + output(value: unknown): ExternalLinkResult { + if (value !== 'opened' && value !== 'cancelled') + throw new Error('Invalid external link response.'); + return value; + }, +}); diff --git a/tests/public-api/apps-external-links.test.ts b/tests/public-api/apps-external-links.test.ts new file mode 100644 index 00000000..c6c0d9cd --- /dev/null +++ b/tests/public-api/apps-external-links.test.ts @@ -0,0 +1,74 @@ +import { expect } from 'vitest'; + +import { test } from '@/tests/public-api/browser'; + +test('sandboxed widget opens an approved external link only after host confirmation', async ({ + page, +}) => { + await page.context().route('https://example.com/**', route => + route.fulfill({ + contentType: 'text/html', + body: '

External report

', + }) + ); + await page.goto('/bundle-host'); + const app = page.frameLocator('iframe'); + await app.getByRole('link', { name: 'View full report' }).click(); + const dialog = page.getByRole('dialog', { name: 'Open external link' }); + await expect.poll(() => dialog.isVisible()).toBe(true); + expect(await dialog.textContent()).toContain( + 'https://example.com/report?source=widget' + ); + expect(page.context().pages()).toHaveLength(1); + expect(await page.locator('iframe').getAttribute('sandbox')).toBe( + 'allow-scripts' + ); + await dialog.getByRole('button', { name: 'Cancel' }).press('Escape'); + await expect.poll(() => dialog.count()).toBe(0); + await app.getByRole('link', { name: 'View full report' }).press('Enter'); + const opened = page.context().waitForEvent('page'); + await dialog.getByRole('link', { name: 'Open example.com' }).click(); + const external = await opened; + await external.waitForLoadState(); + expect(external.url()).toBe('https://example.com/report?source=widget'); + expect(await external.evaluate(() => window.opener)).toBeNull(); + expect(await external.evaluate(() => document.referrer)).toBe(''); + await expect.poll(() => dialog.count()).toBe(0); +}); + +test('denied and unsafe widget links do not navigate', async ({ page }) => { + await page.goto('/bundle-host'); + const app = page.frameLocator('iframe'); + await app.getByRole('link', { name: 'Denied report' }).click(); + await expect + .poll(() => + app.getByRole('alert').filter({ hasText: 'Could not open' }).isVisible() + ) + .toBe(true); + expect(await page.getByRole('dialog').count()).toBe(0); + expect( + await app.getByText('Unsafe report', { exact: true }).getAttribute('href') + ).toBeNull(); + expect(page.context().pages()).toHaveLength(1); +}); + +test('iframe reload dismisses a pending host confirmation and permits another request', async ({ + page, +}) => { + await page.setViewportSize({ width: 375, height: 812 }); + await page.goto('/bundle-host'); + await page.getByRole('button', { name: 'Change theme' }).click(); + const app = page.frameLocator('iframe'); + const link = app.getByRole('link', { name: 'View full report' }); + await link.click({ button: 'middle' }); + const dialog = page.getByRole('dialog', { name: 'Open external link' }); + await expect.poll(() => dialog.isVisible()).toBe(true); + await page.locator('iframe').evaluate(frame => { + frame.setAttribute('src', frame.getAttribute('src')!); + }); + await expect.poll(() => dialog.count()).toBe(0); + await link.click(); + await dialog.getByRole('button', { name: 'Cancel' }).click(); + await expect.poll(() => dialog.count()).toBe(0); + expect(page.context().pages()).toHaveLength(1); +}); diff --git a/tests/public-api/external-links.test.ts b/tests/public-api/external-links.test.ts new file mode 100644 index 00000000..9d9b9c91 --- /dev/null +++ b/tests/public-api/external-links.test.ts @@ -0,0 +1,54 @@ +import { createExternalLinkHandler } from '@altertable/data-app-embed'; +import { + createMessageRouter, + openExternalLinkRoute, +} from '@altertable/data-app/contract'; +import { describe, expect, test } from 'vitest'; + +describe('external link policy', () => { + test.each([ + 'javascript:alert(1)', + 'data:text/html,hi', + 'http://example.com', + '//example.com', + '/report', + 'https:example.com', + 'https://user:secret@example.com', + 'https://example.com\\@evil.test', + 'https://example.com/\nreport', + '', + 'https://example.com/' + 'a'.repeat(8192), + ])('rejects unsafe destination %s', url => { + expect(() => openExternalLinkRoute.input({ url })).toThrow(); + }); + test('normalizes HTTPS destinations and rejects invalid responses', () => { + expect( + openExternalLinkRoute.input({ url: 'https://EXAMPLE.com:443/report' }) + ).toEqual({ url: 'https://example.com/report' }); + expect(() => + openExternalLinkRoute.output('success', { url: '' }) + ).toThrow(); + }); + test('host rejects destinations outside its exact origin policy before touching the DOM', async () => { + const router = createMessageRouter( + { 'navigation:openExternal': openExternalLinkRoute }, + { + 'navigation:openExternal': createExternalLinkHandler({ + window: {} as Window, + allowedOrigins: ['https://example.com'], + }), + } + ); + for (const url of [ + 'https://example.com.evil.test/', + 'https://example.com:444/', + 'https://evil.test/', + ]) + await expect( + router.dispatch( + { route: 'navigation:openExternal', payload: { url } }, + { signal: new AbortController().signal } + ) + ).rejects.toMatchObject({ code: 'forbidden' }); + }); +}); diff --git a/tests/public-api/fixtures/bundle-app.tsx b/tests/public-api/fixtures/bundle-app.tsx index 3fbf2fe2..fd07ac21 100644 --- a/tests/public-api/fixtures/bundle-app.tsx +++ b/tests/public-api/fixtures/bundle-app.tsx @@ -1,5 +1,5 @@ import { defineDataApp } from '@altertable/data-app'; -import { injectDataAppStyles } from '@altertable/data-app-react'; +import { ExternalLink, injectDataAppStyles } from '@altertable/data-app-react'; import { mountDataApp, searchVariable } from '@altertable/data-app-react'; import { DataApp, useAppVariables } from '@altertable/data-app-react/ui'; import { @@ -69,6 +69,13 @@ function App() { toolbarActions={} footerActions={} > + + View full report + + + Denied report + + Unsafe report } footerActions={} > - - View full report - + + + + View full report + + Denied report From 960c0f5470243142eca2d3bcb076f481a31cad40 Mon Sep 17 00:00:00 2001 From: robinvrd Date: Mon, 2 Nov 2026 10:50:24 +0100 Subject: [PATCH 5/7] feat: support host navigation helpers for external links --- docs/embed.md | 3 + packages/data-app-embed/src/external-link.ts | 22 ++++++- tests/public-api/apps-external-links.test.ts | 61 ++++++++++++++++++++ tests/public-api/fixtures/bundle-host.tsx | 25 +++++++- 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/docs/embed.md b/docs/embed.md index d16bcf9e..23642cb4 100644 --- a/docs/embed.md +++ b/docs/embed.md @@ -109,6 +109,9 @@ const router = createMessageRouter( // Pass router.dispatch as the bridge's onMessage handler. ``` +To use an existing host navigation helper, pass it as `navigate`, for example +`createExternalLinkHandler({ navigate: navigateNewTab })`. + Choose permitted origins according to the host's data policy: URLs can carry app data, and the initial origin check does not constrain redirects at the destination. diff --git a/packages/data-app-embed/src/external-link.ts b/packages/data-app-embed/src/external-link.ts index eb6fa63b..27143200 100644 --- a/packages/data-app-embed/src/external-link.ts +++ b/packages/data-app-embed/src/external-link.ts @@ -10,10 +10,16 @@ import { export function createExternalLinkHandler({ window: host = window, allowedOrigins, + navigate, }: { window?: Window; /** Exact HTTPS origins. Omit to allow any HTTPS destination after confirmation. */ allowedOrigins?: readonly string[]; + /** Called synchronously on the host confirmation click instead of native navigation. + * The event carries confirmation-click modifiers. The callback owns popup opening + * and opener/referrer protection; do not await before opening a window. + */ + navigate?: (url: string, event: MouseEvent) => void; } = {}) { const origins = allowedOrigins && @@ -71,7 +77,21 @@ export function createExternalLinkHandler({ signal.addEventListener('abort', abort, { once: true }); dialog.addEventListener('close', () => resolve('cancelled')); cancel.addEventListener('click', () => dialog.close()); - link.addEventListener('click', () => resolve('opened')); + function open(event: MouseEvent) { + if (event.type === 'auxclick' && event.button !== 1) return; + if (navigate) { + event.preventDefault(); + try { + navigate(url, event); + } catch (error) { + reject(error); + return; + } + } + resolve('opened'); + } + link.addEventListener('click', open); + link.addEventListener('auxclick', open); // Remove the abort listener when the dialog settles, including showModal failures. cleanup = () => signal.removeEventListener('abort', abort); dialog.showModal(); diff --git a/tests/public-api/apps-external-links.test.ts b/tests/public-api/apps-external-links.test.ts index 700f3b02..ff3701f2 100644 --- a/tests/public-api/apps-external-links.test.ts +++ b/tests/public-api/apps-external-links.test.ts @@ -96,3 +96,64 @@ test('an Activity-hidden link cancels confirmation and works again when restored await expect.poll(() => link.getAttribute('aria-disabled')).toBe('false'); expect(page.context().pages()).toHaveLength(1); }); + +test('host navigation helper runs on confirmation with user activation and modifiers', async ({ + page, +}) => { + await page + .context() + .route('https://example.com/**', route => + route.fulfill({ contentType: 'text/html', body: '

Report

' }) + ); + await page.goto('/bundle-host?custom-navigation'); + const app = page.frameLocator('iframe'); + const dialog = page.getByRole('dialog', { name: 'Open external link' }); + const navigation = page.getByLabel('External navigation'); + await app.getByRole('link', { name: 'Denied report' }).click(); + await expect + .poll(() => + app.getByRole('alert').filter({ hasText: 'Could not open' }).count() + ) + .toBe(1); + expect(await navigation.textContent()).toBe(''); + await app.getByRole('link', { name: 'View full report' }).click(); + await dialog.getByRole('button', { name: 'Cancel' }).click(); + expect(await navigation.textContent()).toBe(''); + await app.getByRole('link', { name: 'View full report' }).click(); + const opened = page.context().waitForEvent('page'); + await dialog.getByRole('link').click({ modifiers: ['Shift'] }); + const external = await opened; + await external.waitForLoadState(); + expect(JSON.parse((await navigation.textContent())!)).toEqual({ + url: 'https://example.com/report?source=widget', + shiftKey: true, + trusted: true, + activated: true, + }); + expect(await external.evaluate(() => window.opener)).toBeNull(); + expect(await external.evaluate(() => document.referrer)).toBe(''); + expect(page.context().pages()).toHaveLength(2); + await expect.poll(() => dialog.count()).toBe(0); +}); + +test('a failing host navigation helper closes confirmation and permits retry', async ({ + page, +}) => { + await page.goto('/bundle-host?custom-navigation&navigation-error'); + const app = page.frameLocator('iframe'); + const link = app.getByRole('link', { name: 'View full report' }); + const dialog = page.getByRole('dialog', { name: 'Open external link' }); + await link.click(); + await dialog.getByRole('link').click({ button: 'middle' }); + await expect.poll(() => dialog.count()).toBe(0); + await expect + .poll(() => + app.getByRole('alert').filter({ hasText: 'Could not open' }).count() + ) + .toBe(1); + await expect.poll(() => link.getAttribute('aria-disabled')).toBe('false'); + expect(page.context().pages()).toHaveLength(1); + await link.click(); + await dialog.getByRole('button', { name: 'Cancel' }).click(); + await expect.poll(() => dialog.count()).toBe(0); +}); diff --git a/tests/public-api/fixtures/bundle-host.tsx b/tests/public-api/fixtures/bundle-host.tsx index 0391dd27..4f45c3e3 100644 --- a/tests/public-api/fixtures/bundle-host.tsx +++ b/tests/public-api/fixtures/bundle-host.tsx @@ -32,9 +32,6 @@ const javascript = await fetch( ? '/__test/content-sizing' : '/__test/bundle' ).then(response => response.text()); -const openExternalLink = createExternalLinkHandler({ - allowedOrigins: ['https://example.com'], -}); const fileRoute = defineMessageRoute({ input(value: unknown) { const file = value as { filename: string; blob: Blob }; @@ -56,6 +53,27 @@ const offscreen = { } as const; function Host() { + const [externalNavigation, setExternalNavigation] = useState(''); + const [openExternalLink] = useState(() => + createExternalLinkHandler({ + allowedOrigins: ['https://example.com'], + navigate: params.has('custom-navigation') + ? (url, event) => { + setExternalNavigation( + JSON.stringify({ + url, + shiftKey: event.shiftKey, + trusted: event.isTrusted, + activated: navigator.userActivation.isActive, + }) + ); + if (params.has('navigation-error')) + throw new Error('Navigation failed'); + window.open(url, '_blank', 'noopener,noreferrer'); + } + : undefined, + }) + ); const [echoRequests, setEchoRequests] = useState(0); const [pendingRequests, setPendingRequests] = useState(0); const [cancelledRequests, setCancelledRequests] = useState(0); @@ -175,6 +193,7 @@ function Host() { {JSON.stringify(diagnostics)} + {externalNavigation} } footerActions={} > + + {navigationResult} + + + + diff --git a/tests/public-api/fixtures/bundle-host.tsx b/tests/public-api/fixtures/bundle-host.tsx index 4f45c3e3..83f10726 100644 --- a/tests/public-api/fixtures/bundle-host.tsx +++ b/tests/public-api/fixtures/bundle-host.tsx @@ -58,18 +58,23 @@ function Host() { createExternalLinkHandler({ allowedOrigins: ['https://example.com'], navigate: params.has('custom-navigation') - ? (url, event) => { + ? (url, target) => { setExternalNavigation( JSON.stringify({ url, - shiftKey: event.shiftKey, - trusted: event.isTrusted, + target, activated: navigator.userActivation.isActive, }) ); if (params.has('navigation-error')) throw new Error('Navigation failed'); - window.open(url, '_blank', 'noopener,noreferrer'); + window.open( + url, + '_blank', + target === 'window' + ? 'popup,noopener,noreferrer' + : 'noopener,noreferrer' + ); } : undefined, }) From 276f00c8f4e7220eefc5ee17e2b43ec99237b5fe Mon Sep 17 00:00:00 2001 From: robinvrd Date: Mon, 2 Nov 2026 11:37:20 +0100 Subject: [PATCH 7/7] refactor: align host navigation callback with event helpers --- docs/embed.md | 11 ++-- packages/data-app-embed/src/external-link.ts | 16 ++++-- tests/public-api/apps-external-links.test.ts | 21 +++++++- tests/public-api/fixtures/bundle-host.tsx | 56 +++++++++++++------- 4 files changed, 73 insertions(+), 31 deletions(-) diff --git a/docs/embed.md b/docs/embed.md index 3970f16d..939d968b 100644 --- a/docs/embed.md +++ b/docs/embed.md @@ -109,20 +109,17 @@ const router = createMessageRouter( // Pass router.dispatch as the bridge's onMessage handler. ``` -To use existing host navigation helpers, dispatch on the resolved target: +To use an existing modifier-based host navigation helper, register it directly: ```ts const openExternalLink = createExternalLinkHandler({ - navigate: (url, target) => { - if (target === 'window') navigateNewWindow(url); - else navigateNewTab(url); - }, + navigate: navigateNewWindowOrTab, }); // Register openExternalLink for navigation:openExternal in the host router. ``` -Here `navigateNewWindow` and `navigateNewTab` are the host's navigation helpers. -The target comes from the original frame request, including its modifier keys. +Here `navigateNewWindowOrTab` is the host's `(url, event)` navigation helper. +Use the frontend helper, not the app-side bridge client with the same name. Choose permitted origins according to the host's data policy: URLs can carry app data, and the initial origin check does not constrain redirects at the destination. diff --git a/packages/data-app-embed/src/external-link.ts b/packages/data-app-embed/src/external-link.ts index 7d22572e..aed4660a 100644 --- a/packages/data-app-embed/src/external-link.ts +++ b/packages/data-app-embed/src/external-link.ts @@ -1,7 +1,6 @@ import { openExternalLinkRoute, resolveExternalLinkTarget, - type ExternalLinkTarget, MessageRoutingError, type OpenExternalLink, type ExternalLinkResult, @@ -18,10 +17,11 @@ export function createExternalLinkHandler({ /** Exact HTTPS origins. Omit to allow any HTTPS destination after confirmation. */ allowedOrigins?: readonly string[]; /** Called synchronously on the host confirmation click instead of native navigation. - * The target is resolved from the app's original request, including its modifier keys. + * Receives an undispatched, synthetic KeyboardEvent with the original modifier keys. + * Explicit tab/window requests map to Ctrl/Shift so modifier-based helpers work directly. * The callback owns popup opening and opener/referrer protection; do not await before opening. */ - navigate?: (url: string, target: ExternalLinkTarget) => void; + navigate?: (url: string, event: KeyboardEvent) => void; } = {}) { const origins = allowedOrigins && @@ -48,6 +48,14 @@ export function createExternalLinkHandler({ ); const target = resolveExternalLinkTarget(request); if (!target) return 'ignored'; + const keys = + request.target === 'event' + ? request.keys + : { + shiftKey: target === 'window', + ctrlKey: target === 'tab', + metaKey: false, + }; if (pending) throw new MessageRoutingError( 'external_link_busy', @@ -86,7 +94,7 @@ export function createExternalLinkHandler({ if (event.type === 'auxclick' && event.button !== 1) return; event.preventDefault(); try { - if (navigate) navigate(url, target!); + if (navigate) navigate(url, new KeyboardEvent('navigation', keys)); else host.open( url, diff --git a/tests/public-api/apps-external-links.test.ts b/tests/public-api/apps-external-links.test.ts index 8dc7cb4c..d1a968c0 100644 --- a/tests/public-api/apps-external-links.test.ts +++ b/tests/public-api/apps-external-links.test.ts @@ -127,6 +127,8 @@ test('host navigation helper uses the app request and retains host user activati expect(JSON.parse((await navigation.textContent())!)).toEqual({ url: 'https://example.com/report?source=widget', target: 'tab', + keys: { shiftKey: false, metaKey: false, ctrlKey: true }, + keyboardEvent: true, activated: true, }); expect(await external.evaluate(() => window.opener)).toBeNull(); @@ -217,7 +219,24 @@ for (const { name, button, modifiers, target } of [ await external.waitForLoadState(); expect( JSON.parse((await page.getByLabel('External navigation').textContent())!) - ).toEqual({ url: 'https://example.com/report', target, activated: true }); + ).toEqual({ + url: 'https://example.com/report', + target, + activated: true, + keyboardEvent: true, + keys: + button === 'Request from event' + ? { + shiftKey: (modifiers as readonly string[]).includes('Shift'), + metaKey: (modifiers as readonly string[]).includes('Meta'), + ctrlKey: (modifiers as readonly string[]).includes('Control'), + } + : { + shiftKey: target === 'window', + metaKey: false, + ctrlKey: target === 'tab', + }, + }); await expect .poll(() => app.getByLabel('External navigation result').textContent()) .toBe('opened'); diff --git a/tests/public-api/fixtures/bundle-host.tsx b/tests/public-api/fixtures/bundle-host.tsx index 83f10726..c68b629d 100644 --- a/tests/public-api/fixtures/bundle-host.tsx +++ b/tests/public-api/fixtures/bundle-host.tsx @@ -15,7 +15,12 @@ import { sqlQueryRoute, DataSourceError, } from '@altertable/data-app/contract'; -import { useReducer, useState } from 'react'; +import { + useReducer, + useState, + type KeyboardEvent as ReactKeyboardEvent, + type MouseEvent as ReactMouseEvent, +} from 'react'; import { createRoot } from 'react-dom/client'; import { bridgeRoutes } from '@/tests/public-api/fixtures/bridge-routes'; @@ -54,28 +59,41 @@ const offscreen = { function Host() { const [externalNavigation, setExternalNavigation] = useState(''); + function navigateNewWindowOrTab( + url: string, + event: KeyboardEvent | ReactKeyboardEvent | ReactMouseEvent + ) { + const target = event.shiftKey + ? 'window' + : event.metaKey || event.ctrlKey + ? 'tab' + : undefined; + if (!target) return; + setExternalNavigation( + JSON.stringify({ + url, + target, + keys: { + shiftKey: event.shiftKey, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, + }, + keyboardEvent: event instanceof KeyboardEvent, + activated: navigator.userActivation.isActive, + }) + ); + if (params.has('navigation-error')) throw new Error('Navigation failed'); + window.open( + url, + '_blank', + target === 'window' ? 'popup,noopener,noreferrer' : 'noopener,noreferrer' + ); + } const [openExternalLink] = useState(() => createExternalLinkHandler({ allowedOrigins: ['https://example.com'], navigate: params.has('custom-navigation') - ? (url, target) => { - setExternalNavigation( - JSON.stringify({ - url, - target, - activated: navigator.userActivation.isActive, - }) - ); - if (params.has('navigation-error')) - throw new Error('Navigation failed'); - window.open( - url, - '_blank', - target === 'window' - ? 'popup,noopener,noreferrer' - : 'noopener,noreferrer' - ); - } + ? navigateNewWindowOrTab : undefined, }) );