From 773af2cf6597fdd7ceed6dd5a154e1f7df78d7f1 Mon Sep 17 00:00:00 2001 From: IanMStevenson Date: Sat, 19 Sep 2026 02:46:38 +0100 Subject: [PATCH] feat(web): setting to open external links in a new tab (#1884) * feat(web): setting to open external links in a new tab Left-clicking an external http(s) link renders it with no explicit target, defaulting to _self. In an installed/standalone PWA this gets intercepted by the browser and shown as a captive in-app overlay instead of leaving the app; in a normal tab it navigates away from HAPI entirely. Right-click already opens in a new tab, but that's not a fair ask for every link. Add an opt-in Settings > Display > Links toggle that forces target="_blank" rel="noopener noreferrer" on genuine external http(s) links only (relative/SPA hrefs and other schemes like mailto are left untouched). Defaults to off, preserving today's behaviour, since the overlay may be intentional/liked by some users. Vibe Coding disclosure: implemented with Claude Sonnet 5 (Claude Code). * fix(web): simplify link-toggle setting description per review The description mentioned the target attribute (an implementation detail, not user-facing) and asserted the in-app overlay is what every installed PWA shows, which was only actually confirmed on Chromium/ Edge. Describe the user-visible behaviour only, without promising a specific rendering across all installed-app environments. * fix(web): stop promising specific behaviour when installed as an app Per follow-up review: describing the setting as "always" opening links in a new tab "even when installed as an app" still guarantees a specific outcome across every browser's PWA handling, which isn't something this change can verify or promise. State only what the setting does (open in a separate tab), not how every installed-app environment will render that. --------- Co-authored-by: Ian Stevenson --- .../assistant-ui/markdown-a.test.tsx | 46 +++++++++ .../assistant-ui/markdown-text.test.ts | 33 +++++++ .../components/assistant-ui/markdown-text.tsx | 36 ++++++- .../useOpenExternalLinksInNewTab.test.ts | 77 +++++++++++++++ web/src/hooks/useOpenExternalLinksInNewTab.ts | 99 +++++++++++++++++++ web/src/lib/locales/en.ts | 3 + web/src/lib/locales/zh-CN.ts | 3 + web/src/routes/settings/display.tsx | 11 +++ 8 files changed, 306 insertions(+), 2 deletions(-) create mode 100644 web/src/hooks/useOpenExternalLinksInNewTab.test.ts create mode 100644 web/src/hooks/useOpenExternalLinksInNewTab.ts diff --git a/web/src/components/assistant-ui/markdown-a.test.tsx b/web/src/components/assistant-ui/markdown-a.test.tsx index d5c16131..0fcf9b58 100644 --- a/web/src/components/assistant-ui/markdown-a.test.tsx +++ b/web/src/components/assistant-ui/markdown-a.test.tsx @@ -66,6 +66,7 @@ function chatContext(overrides: Partial = {}): HappyChatC } const STORAGE_KEY = 'hapi-allowed-schemes' +const OPEN_NEW_TAB_STORAGE_KEY = 'hapi-open-external-links-in-new-tab' beforeEach(() => { localStorage.clear() @@ -422,6 +423,51 @@ describe('markdown component — fail-closed path-like hrefs (#1452)', () => }) }) +// ── "open external links in a new tab" setting (Settings > Display > Links) ── + +describe('markdown component — open external links in new tab setting', () => { + it('defaults to no explicit target on an external http(s) link', () => { + renderA({ href: 'https://example.com', children: 'link' }) + expect(document.querySelector('a')!.getAttribute('target')).toBeNull() + }) + + it('forces target="_blank" + rel="noopener noreferrer" when the setting is enabled', () => { + localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true') + renderA({ href: 'https://example.com', children: 'link' }) + const link = document.querySelector('a')! + expect(link.getAttribute('target')).toBe('_blank') + expect(link.getAttribute('rel')).toBe('noopener noreferrer') + }) + + it('does not force target on a relative/SPA href even when the setting is enabled', () => { + localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true') + renderA({ href: '/settings', children: 'settings' }, chatContext()) + expect(document.querySelector('a')!.getAttribute('target')).toBeNull() + }) + + it('does not force target on a non-http(s) IANA scheme (mailto) even when enabled', () => { + localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true') + renderA({ href: 'mailto:a@b.com', children: 'mail' }) + expect(document.querySelector('a')!.getAttribute('target')).toBeNull() + }) + + it('updates an already-rendered link when the setting changes via storage event (settings page in another tab, or Settings > Display in this one after context refactor)', () => { + renderA({ href: 'https://example.com', children: 'link' }) + expect(document.querySelector('a')!.getAttribute('target')).toBeNull() + + act(() => { + localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true') + window.dispatchEvent(new StorageEvent('storage', { + key: OPEN_NEW_TAB_STORAGE_KEY, + newValue: 'true', + storageArea: localStorage, + })) + }) + + expect(document.querySelector('a')!.getAttribute('target')).toBe('_blank') + }) +}) + // ── intra-tab cross-provider sync (schemeListeners emitter) ────────────────── // // P7e.1 added a module-level `schemeListeners: Set` so that diff --git a/web/src/components/assistant-ui/markdown-text.test.ts b/web/src/components/assistant-ui/markdown-text.test.ts index 574a4a33..dab45789 100644 --- a/web/src/components/assistant-ui/markdown-text.test.ts +++ b/web/src/components/assistant-ui/markdown-text.test.ts @@ -12,6 +12,7 @@ import { MARKDOWN_PLUGINS_STANDALONE_WITH_BREAKS, MARKDOWN_PLUGINS_WITH_BREAKS, MARKDOWN_REHYPE_PLUGINS, + isExternalHttpHref, } from '@/components/assistant-ui/markdown-text' describe('MARKDOWN_PLUGINS integration', () => { @@ -97,3 +98,35 @@ describe('MARKDOWN_PLUGINS — currency prose vs KaTeX', () => { expect(html).toContain('class="katex"') }) }) + +describe('isExternalHttpHref', () => { + it('is true for http/https links', () => { + expect(isExternalHttpHref('https://example.com')).toBe(true) + expect(isExternalHttpHref('http://example.com/path?q=1#frag')).toBe(true) + expect(isExternalHttpHref('HTTPS://Example.com')).toBe(true) + }) + + it('is false for relative/SPA hrefs', () => { + expect(isExternalHttpHref('/settings')).toBe(false) + expect(isExternalHttpHref('./foo')).toBe(false) + expect(isExternalHttpHref('#section')).toBe(false) + expect(isExternalHttpHref('?q=1')).toBe(false) + expect(isExternalHttpHref('/path:colon')).toBe(false) + }) + + it('is false for non-http(s) schemes', () => { + expect(isExternalHttpHref('mailto:a@b.com')).toBe(false) + expect(isExternalHttpHref('vscode://file/foo')).toBe(false) + expect(isExternalHttpHref('javascript:alert(1)')).toBe(false) + }) + + it('is false for Windows drive paths (single-letter scheme, not http/https)', () => { + expect(isExternalHttpHref('C:\\Users\\ian\\file.txt')).toBe(false) + }) + + it('is false for empty/nullish input', () => { + expect(isExternalHttpHref('')).toBe(false) + expect(isExternalHttpHref(null)).toBe(false) + expect(isExternalHttpHref(undefined)).toBe(false) + }) +}) diff --git a/web/src/components/assistant-ui/markdown-text.tsx b/web/src/components/assistant-ui/markdown-text.tsx index 68ea713c..26982a6a 100644 --- a/web/src/components/assistant-ui/markdown-text.tsx +++ b/web/src/components/assistant-ui/markdown-text.tsx @@ -32,6 +32,7 @@ import { classifyNoSchemeHref } from '@/lib/markdown-href-policy' import { remarkSessionPathLinks } from '@/lib/remark-session-path-links' import { buildSessionReferencePath, parseSessionPathHref } from '@/lib/sessionReference' import { UriConfirmDialog } from '@/components/UriConfirmDialog' +import { DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB, useOpenExternalLinksInNewTab } from '@/hooks/useOpenExternalLinksInNewTab' import type { MarkdownTextPrimitiveProps } from '@assistant-ui/react-markdown' @@ -217,6 +218,23 @@ function hasScheme(href: string): boolean { return boundaryIdx < 0 || colonIdx < boundaryIdx } +/** + * True when href is a genuine external http(s) link — as opposed to a + * relative/SPA href (no scheme, e.g. `/settings`) or another scheme + * (`mailto:`, `vscode:`, a Windows drive path like `C:\Users\...`, etc). + * + * Used by to decide whether the "open external links in a new tab" + * setting (Settings > Display > Links) applies — forcing target="_blank" on + * a relative href would open the whole app in a second tab instead of + * navigating in-app, so this must stay scoped to real http(s) URLs. + */ +export function isExternalHttpHref(href: string | null | undefined): boolean { + if (!href || !hasScheme(href)) return false + const colonIdx = href.indexOf(':') + const scheme = href.slice(0, colonIdx).toLowerCase() + return scheme === 'http' || scheme === 'https' +} + // ── URL sanitize transform (deny-only) ────────────────────────────────────── // Passed as urlTransform to MarkdownTextPrimitive. Only deny-listed schemes // are stripped; every other scheme (IANA + custom) passes through so the @@ -349,6 +367,8 @@ type UriConfirmContextValue = { openUri: (url: string, scheme: string) => void /** Shared isAllowed so all tags in this tree re-render on the same state update. */ isAllowed: (scheme: string) => boolean + /** User preference (Settings > Display > Links): force target="_blank" on external http(s) links. */ + openExternalLinksInNewTab: boolean } const UriConfirmContext = createContext(null) @@ -366,6 +386,7 @@ const UriConfirmContext = createContext(null) export function UriConfirmProvider({ children }: { children: ReactNode }) { const [dialog, setDialog] = useState(null) const { allow, isAllowed } = useAllowedSchemes() + const { openExternalLinksInNewTab } = useOpenExternalLinksInNewTab() const openUri = useCallback((url: string, scheme: string) => { setDialog({ url, scheme }) @@ -387,7 +408,10 @@ export function UriConfirmProvider({ children }: { children: ReactNode }) { } } - const contextValue = useMemo(() => ({ openUri, isAllowed }), [openUri, isAllowed]) + const contextValue = useMemo( + () => ({ openUri, isAllowed, openExternalLinksInNewTab }), + [openUri, isAllowed, openExternalLinksInNewTab] + ) return ( @@ -693,6 +717,13 @@ function A(props: ComponentPropsWithoutRef<'a'>) { const scheme = colonIdx > 0 && !isRelative ? href!.slice(0, colonIdx).toLowerCase() : '' const isCustomAllowed = classification === 'custom' && isAllowed(scheme) + // Settings > Display > Links toggle: force genuine external http(s) links + // to open in a new browser tab instead of navigating same-tab. + const openExternalLinksInNewTab = ctx?.openExternalLinksInNewTab ?? DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB + const forceNewTab = openExternalLinksInNewTab && isExternalHttpHref(href) + const target = forceNewTab ? '_blank' : props.target + const effectiveRel = forceNewTab ? (rel ?? 'noopener noreferrer') : rel + const domHref = classification === 'iana' || isCustomAllowed ? href @@ -732,7 +763,8 @@ function A(props: ComponentPropsWithoutRef<'a'>) { diff --git a/web/src/hooks/useOpenExternalLinksInNewTab.test.ts b/web/src/hooks/useOpenExternalLinksInNewTab.test.ts new file mode 100644 index 00000000..fe05d1fe --- /dev/null +++ b/web/src/hooks/useOpenExternalLinksInNewTab.test.ts @@ -0,0 +1,77 @@ +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it } from 'vitest' +import { + DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB, + getInitialOpenExternalLinksInNewTab, + useOpenExternalLinksInNewTab, +} from './useOpenExternalLinksInNewTab' + +const STORAGE_KEY = 'hapi-open-external-links-in-new-tab' + +describe('useOpenExternalLinksInNewTab helpers', () => { + beforeEach(() => { + window.localStorage.clear() + }) + + it('defaults to disabled (today’s existing link behaviour)', () => { + expect(getInitialOpenExternalLinksInNewTab()).toBe(DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB) + expect(getInitialOpenExternalLinksInNewTab()).toBe(false) + }) + + it('reads the stored value and falls back for other values', () => { + window.localStorage.setItem(STORAGE_KEY, 'true') + expect(getInitialOpenExternalLinksInNewTab()).toBe(true) + + window.localStorage.setItem(STORAGE_KEY, 'invalid') + expect(getInitialOpenExternalLinksInNewTab()).toBe(false) + }) +}) + +describe('useOpenExternalLinksInNewTab', () => { + beforeEach(() => { + window.localStorage.clear() + }) + + it('persists changes and synchronizes hook instances in the same tab', () => { + const first = renderHook(() => useOpenExternalLinksInNewTab()) + const second = renderHook(() => useOpenExternalLinksInNewTab()) + + act(() => first.result.current.setOpenExternalLinksInNewTab(true)) + + expect(first.result.current.openExternalLinksInNewTab).toBe(true) + expect(second.result.current.openExternalLinksInNewTab).toBe(true) + expect(window.localStorage.getItem(STORAGE_KEY)).toBe('true') + + act(() => second.result.current.setOpenExternalLinksInNewTab(false)) + + expect(first.result.current.openExternalLinksInNewTab).toBe(false) + expect(second.result.current.openExternalLinksInNewTab).toBe(false) + expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull() + }) + + it('synchronizes changes from another browsing context', () => { + const { result } = renderHook(() => useOpenExternalLinksInNewTab()) + + act(() => { + window.dispatchEvent(new StorageEvent('storage', { + key: STORAGE_KEY, + newValue: 'true', + })) + }) + + expect(result.current.openExternalLinksInNewTab).toBe(true) + }) + + it('ignores storage events for unrelated keys', () => { + const { result } = renderHook(() => useOpenExternalLinksInNewTab()) + + act(() => { + window.dispatchEvent(new StorageEvent('storage', { + key: 'hapi-other-setting', + newValue: 'false', + })) + }) + + expect(result.current.openExternalLinksInNewTab).toBe(false) + }) +}) diff --git a/web/src/hooks/useOpenExternalLinksInNewTab.ts b/web/src/hooks/useOpenExternalLinksInNewTab.ts new file mode 100644 index 00000000..3af18613 --- /dev/null +++ b/web/src/hooks/useOpenExternalLinksInNewTab.ts @@ -0,0 +1,99 @@ +import { useCallback, useEffect, useState } from 'react' + +const OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY = 'hapi-open-external-links-in-new-tab' +export const DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB = false + +// Same-tab sync: `storage` events only fire in OTHER tabs/windows, but several +// instances can coexist in one window (e.g. one per +// chat message), each with its own useState. Without this, toggling the +// setting in Settings would only affect providers mounted after the change. +const sameTabListeners = new Set<(value: boolean) => void>() + +function isBrowser(): boolean { + return typeof window !== 'undefined' && typeof document !== 'undefined' +} + +function safeGetItem(key: string): string | null { + if (!isBrowser()) { + return null + } + try { + return localStorage.getItem(key) + } catch { + return null + } +} + +function safeSetItem(key: string, value: string): void { + if (!isBrowser()) { + return + } + try { + localStorage.setItem(key, value) + } catch { + // Ignore storage errors + } +} + +function safeRemoveItem(key: string): void { + if (!isBrowser()) { + return + } + try { + localStorage.removeItem(key) + } catch { + // Ignore storage errors + } +} + +function parseOpenExternalLinksInNewTab(raw: string | null): boolean { + if (raw === 'true') { + return true + } + return DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB +} + +export function getInitialOpenExternalLinksInNewTab(): boolean { + return parseOpenExternalLinksInNewTab(safeGetItem(OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY)) +} + +export function useOpenExternalLinksInNewTab(): { + openExternalLinksInNewTab: boolean + setOpenExternalLinksInNewTab: (value: boolean) => void +} { + const [openExternalLinksInNewTab, setOpenExternalLinksInNewTabState] = useState(getInitialOpenExternalLinksInNewTab) + + useEffect(() => { + if (!isBrowser()) { + return + } + + const onStorage = (event: StorageEvent) => { + if (event.key !== OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY) { + return + } + setOpenExternalLinksInNewTabState(parseOpenExternalLinksInNewTab(event.newValue)) + } + + sameTabListeners.add(setOpenExternalLinksInNewTabState) + window.addEventListener('storage', onStorage) + return () => { + sameTabListeners.delete(setOpenExternalLinksInNewTabState) + window.removeEventListener('storage', onStorage) + } + }, []) + + const setOpenExternalLinksInNewTab = useCallback((value: boolean) => { + for (const listener of sameTabListeners) { + listener(value) + } + + if (value === DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB) { + safeRemoveItem(OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY) + } else { + safeSetItem(OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY, String(value)) + } + }, []) + + return { openExternalLinksInNewTab, setOpenExternalLinksInNewTab } +} diff --git a/web/src/lib/locales/en.ts b/web/src/lib/locales/en.ts index 2d5e8215..647b8005 100644 --- a/web/src/lib/locales/en.ts +++ b/web/src/lib/locales/en.ts @@ -895,6 +895,9 @@ export default { 'settings.display.sessionHeader.createdAt': 'Created time', 'settings.display.sessionHeader.updatedAt': 'Updated time', 'settings.display.sessionHeader.worktree': 'Worktree branch', + 'settings.display.links': 'Links', + 'settings.display.openExternalLinksInNewTab': 'Open external links in a new tab', + 'settings.display.openExternalLinksInNewTab.desc': 'Off (default) keeps today’s behaviour. On opens external links in a separate browser tab instead.', 'session.header.createdAt': 'Created', 'session.header.updatedAt': 'Updated', 'settings.chat.title': 'Chat', diff --git a/web/src/lib/locales/zh-CN.ts b/web/src/lib/locales/zh-CN.ts index 6dca4945..6886e3ae 100644 --- a/web/src/lib/locales/zh-CN.ts +++ b/web/src/lib/locales/zh-CN.ts @@ -893,6 +893,9 @@ export default { 'settings.display.sessionHeader.createdAt': '创建时间', 'settings.display.sessionHeader.updatedAt': '更新时间', 'settings.display.sessionHeader.worktree': '工作树分支', + 'settings.display.links': '链接', + 'settings.display.openExternalLinksInNewTab': '在新标签页中打开外部链接', + 'settings.display.openExternalLinksInNewTab.desc': '关闭(默认)保留当前行为。开启后外部链接会在单独的浏览器标签页中打开。', 'session.header.createdAt': '创建', 'session.header.updatedAt': '更新', 'settings.chat.title': '聊天', diff --git a/web/src/routes/settings/display.tsx b/web/src/routes/settings/display.tsx index 10865f1c..92f23f50 100644 --- a/web/src/routes/settings/display.tsx +++ b/web/src/routes/settings/display.tsx @@ -11,6 +11,7 @@ import { MAX_SESSION_PREVIEW_LIMIT, MIN_SESSION_PREVIEW_LIMIT, normalizeSessionP import { useThemeColors, type ThemeColorKeyId } from '@/hooks/useThemeColors' import { useSessionHeaderMetadata, type SessionHeaderMetadataKey } from '@/hooks/useSessionHeaderMetadata' import { useAppBadgePreference } from '@/hooks/useAppBadgePreference' +import { useOpenExternalLinksInNewTab } from '@/hooks/useOpenExternalLinksInNewTab' import { SettingsChoiceGroup, SettingsFieldLabel, SettingsPageContent, SettingsRow, SettingsSection, SettingsSwitch } from '@/components/settings/SettingsPrimitives' function MinusIcon() { @@ -140,6 +141,7 @@ export default function SettingsDisplayPage() { const { showActiveSessionsOnly, setShowActiveSessionsOnly } = useShowActiveSessionsOnly() const { pinInProgressSessions, setPinInProgressSessions } = usePinInProgressSessions() const { appBadgeEnabled, setAppBadgeEnabled } = useAppBadgePreference() + const { openExternalLinksInNewTab, setOpenExternalLinksInNewTab } = useOpenExternalLinksInNewTab() const { preferences: sessionHeaderMetadata, setPreference: setSessionHeaderMetadata } = useSessionHeaderMetadata() const sessionHeaderOptions: ReadonlyArray<{ key: SessionHeaderMetadataKey; labelKey: string }> = [ { key: 'showLabels', labelKey: 'settings.display.sessionHeader.showLabels' }, @@ -197,6 +199,15 @@ export default function SettingsDisplayPage() { /> ))} + + + + ) }