From c77d2af537ca759ee995db4341bf556aaea5651b Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sun, 24 May 2026 12:29:33 +0200 Subject: [PATCH] fix: ignore canceled external URL opens --- src/components/PulseView.test.tsx | 15 +++++++++++ src/components/PulseView.tsx | 12 ++++----- src/utils/url.test.ts | 19 ++++++++++++- src/utils/url.ts | 44 ++++++++++++++++++++++++------- 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/components/PulseView.test.tsx b/src/components/PulseView.test.tsx index a8af8cd2..ac6daf42 100644 --- a/src/components/PulseView.test.tsx +++ b/src/components/PulseView.test.tsx @@ -3,6 +3,7 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/react' import { PulseView } from './PulseView' import type { PulseCommit } from '../types' +const openExternalUrl = vi.fn() const mockCommits: PulseCommit[] = [ { hash: 'abc123def456', @@ -45,6 +46,9 @@ vi.mock('../mock-tauri', () => ({ vi.mock('../hooks/useDragRegion', () => ({ useDragRegion: () => ({ onMouseDown: dragRegionMouseDown }), })) +vi.mock('../utils/url', () => ({ + openExternalUrl: (...args: unknown[]) => openExternalUrl(...args), +})) describe('PulseView', () => { beforeEach(() => { @@ -109,6 +113,17 @@ describe('PulseView', () => { expect(nonLink.tagName).toBe('SPAN') }) + it('opens commit links through the shared external URL opener', async () => { + mockInvokeFn.mockResolvedValue(mockCommits) + + render() + + const link = await screen.findByText('abc123d') + fireEvent.click(link) + + expect(openExternalUrl).toHaveBeenCalledWith('https://github.com/owner/repo/commit/abc123def456') + }) + it('renders file list with correct titles when expanded', async () => { mockInvokeFn.mockResolvedValue(mockCommits) diff --git a/src/components/PulseView.tsx b/src/components/PulseView.tsx index b40daafc..88c4be22 100644 --- a/src/components/PulseView.tsx +++ b/src/components/PulseView.tsx @@ -5,6 +5,7 @@ import { isTauri, mockInvoke } from '../mock-tauri' import { useDragRegion } from '../hooks/useDragRegion' import type { PulseCommit, PulseFile } from '../types' import { relativeDate } from '../utils/noteListHelpers' +import { openExternalUrl } from '../utils/url' import { getLocaleDateLocale, translate, type AppLocale } from '../lib/i18n' import { GitRepositorySelect } from './GitRepositorySelect' import type { GitRepositoryOption } from '../utils/gitRepositories' @@ -147,6 +148,7 @@ function CommitCard({ const [expanded, setExpanded] = useState(false) const Chevron = expanded ? CaretDown : CaretRight const toggleExpanded = useCallback(() => setExpanded((value) => !value), []) + const commitUrl = commit.githubUrl return (
@@ -174,19 +176,15 @@ function CommitCard({
{relativeDate(commit.date)} - {commit.githubUrl ? ( + {commitUrl ? ( { e.preventDefault() e.stopPropagation() - if (isTauri()) { - import('@tauri-apps/plugin-opener').then((mod) => mod.openUrl(commit.githubUrl!)) - } else { - window.open(commit.githubUrl!, '_blank') - } + void openExternalUrl(commitUrl) }} title={translate(locale, 'pulse.openOnGitHub')} > diff --git a/src/utils/url.test.ts b/src/utils/url.test.ts index 44f34a2d..2774c563 100644 --- a/src/utils/url.test.ts +++ b/src/utils/url.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { invoke } from '@tauri-apps/api/core' -import { revealItemInDir } from '@tauri-apps/plugin-opener' +import { openUrl, revealItemInDir } from '@tauri-apps/plugin-opener' import { copyLocalPath, isUrlValue, @@ -56,6 +56,23 @@ describe('openExternalUrl', () => { expect(open).not.toHaveBeenCalled() }) + + it('treats Windows user-canceled opener dialogs as a benign no-op', async () => { + vi.stubGlobal('isTauri', true) + vi.mocked(openUrl).mockRejectedValueOnce(new Error('The operation was canceled by the user. (os error 1223)')) + + await expect(openExternalUrl('https://example.com')).resolves.toBeUndefined() + + expect(openUrl).toHaveBeenCalledWith('https://example.com') + }) + + it('keeps real native opener failures visible to callers', async () => { + vi.stubGlobal('isTauri', true) + const failure = new Error('failed to open default browser') + vi.mocked(openUrl).mockRejectedValueOnce(failure) + + await expect(openExternalUrl('https://example.com')).rejects.toThrow(failure) + }) }) describe('local file actions', () => { diff --git a/src/utils/url.ts b/src/utils/url.ts index 8c667276..1cfdf899 100644 --- a/src/utils/url.ts +++ b/src/utils/url.ts @@ -1,6 +1,9 @@ import { isTauri } from '../mock-tauri' -function parseHttpUrl(candidate: string): URL | null { +type ExternalUrlCandidate = string +type AbsoluteFilePath = string + +function parseHttpUrl(candidate: ExternalUrlCandidate): URL | null { try { const parsedUrl = new URL(candidate) return parsedUrl.protocol === 'http:' || parsedUrl.protocol === 'https:' ? parsedUrl : null @@ -14,12 +17,28 @@ function hasBareDomainHost(parsedUrl: URL): boolean { return dotIndex > 0 && dotIndex <= parsedUrl.hostname.length - 3 } -function startsWithHttpProtocol(url: string): boolean { +function startsWithHttpProtocol(url: ExternalUrlCandidate): boolean { const lowerUrl = url.toLowerCase() return lowerUrl.startsWith('http://') || lowerUrl.startsWith('https://') } -export function normalizeExternalUrl(value: string): string | null { +function getErrorMessage(error: unknown): string { + if (error instanceof Error) return error.message + if (typeof error === 'string') return error + if (!error || typeof error !== 'object') return '' + + const message = Reflect.get(error, 'message') + return typeof message === 'string' ? message : '' +} + +function isExternalOpenCanceledByUser(error: unknown): boolean { + const message = getErrorMessage(error).toLowerCase() + return message.includes('os error 1223') || + message.includes('operation was canceled by the user') || + message.includes('operation was cancelled by the user') +} + +export function normalizeExternalUrl(value: ExternalUrlCandidate): string | null { const trimmed = value.trim() if (!trimmed) return null @@ -36,11 +55,11 @@ export function normalizeExternalUrl(value: string): string | null { return bareDomainCandidate } -export function isUrlValue(value: string): boolean { +export function isUrlValue(value: ExternalUrlCandidate): boolean { return normalizeExternalUrl(value) !== null } -export function normalizeUrl(url: string): string { +export function normalizeUrl(url: ExternalUrlCandidate): string { const normalized = normalizeExternalUrl(url) if (normalized) return normalized if (startsWithHttpProtocol(url)) return url @@ -48,20 +67,25 @@ export function normalizeUrl(url: string): string { } /** Open a URL in the system browser. Uses Tauri opener plugin in native mode, window.open in browser. */ -export async function openExternalUrl(url: string): Promise { +export async function openExternalUrl(url: ExternalUrlCandidate): Promise { const normalized = normalizeExternalUrl(url) if (!normalized) return if (isTauri()) { const { openUrl } = await import('@tauri-apps/plugin-opener') - await openUrl(normalized) + try { + await openUrl(normalized) + } catch (error) { + if (isExternalOpenCanceledByUser(error)) return + throw error + } } else { window.open(normalized, '_blank') } } /** Open a local file path with the system default app (e.g. TextEdit for .json). */ -export async function openLocalFile(absolutePath: string, vaultPath?: string): Promise { +export async function openLocalFile(absolutePath: AbsoluteFilePath, vaultPath?: AbsoluteFilePath): Promise { if (isTauri()) { const { invoke } = await import('@tauri-apps/api/core') const args: { path: string; vaultPath?: string } = { path: absolutePath } @@ -71,7 +95,7 @@ export async function openLocalFile(absolutePath: string, vaultPath?: string): P } /** Reveal a local file or folder in the system file manager. */ -export async function revealLocalPath(absolutePath: string): Promise { +export async function revealLocalPath(absolutePath: AbsoluteFilePath): Promise { if (isTauri()) { const { revealItemInDir } = await import('@tauri-apps/plugin-opener') await revealItemInDir(absolutePath) @@ -79,7 +103,7 @@ export async function revealLocalPath(absolutePath: string): Promise { } /** Copy a local file or folder path to the system clipboard. */ -export async function copyLocalPath(absolutePath: string): Promise { +export async function copyLocalPath(absolutePath: AbsoluteFilePath): Promise { if (!navigator.clipboard?.writeText) { throw new Error('Clipboard API is unavailable') }