From c427cd2492bd9108a064e95fe8a3fd3c534d0561 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sat, 25 Apr 2026 10:07:43 +0200 Subject: [PATCH] fix: guard native file drops --- src/hooks/useImageDrop.test.ts | 63 ++++++++++++++++++-------- src/hooks/useImageDrop.ts | 80 ++++++++++++++++++++++++--------- src/hooks/useMenuEvents.test.ts | 16 +++++++ src/hooks/useMenuEvents.ts | 14 ++++-- src/main.test.ts | 69 ++++++++++++++++++++++++++++ src/main.tsx | 24 ++++++++++ 6 files changed, 223 insertions(+), 43 deletions(-) diff --git a/src/hooks/useImageDrop.test.ts b/src/hooks/useImageDrop.test.ts index ce1f26af..f9fa20b1 100644 --- a/src/hooks/useImageDrop.test.ts +++ b/src/hooks/useImageDrop.test.ts @@ -16,13 +16,16 @@ vi.mock('../mock-tauri', () => ({ type DragDropEvent = { payload: { type: string; paths: string[]; position: { x: number; y: number } } } type DragDropCallback = (event: DragDropEvent) => void -let capturedDragDropHandler: DragDropCallback | null = null +const capturedDragDropHandlers: Record = {} +let nativeDropUnlisten: (eventName: string) => void = (eventName) => { + delete capturedDragDropHandlers[eventName] +} vi.mock('@tauri-apps/api/webview', () => ({ getCurrentWebview: () => ({ - onDragDropEvent: vi.fn((cb: DragDropCallback) => { - capturedDragDropHandler = cb - return Promise.resolve(() => { capturedDragDropHandler = null }) + listen: vi.fn((eventName: string, cb: DragDropCallback) => { + capturedDragDropHandlers[eventName] = cb + return Promise.resolve(() => nativeDropUnlisten(eventName)) }), }), })) @@ -185,14 +188,17 @@ describe('useImageDrop — Tauri native drag-drop', () => { beforeEach(() => { tauriMode = true - capturedDragDropHandler = null + nativeDropUnlisten = (eventName) => { + delete capturedDragDropHandlers[eventName] + } + for (const eventName of Object.keys(capturedDragDropHandlers)) delete capturedDragDropHandlers[eventName] container = document.createElement('div') document.body.appendChild(container) }) afterEach(() => { tauriMode = false - capturedDragDropHandler = null + for (const eventName of Object.keys(capturedDragDropHandlers)) delete capturedDragDropHandlers[eventName] container.remove() }) @@ -202,23 +208,33 @@ describe('useImageDrop — Tauri native drag-drop', () => { return renderHook(() => useImageDrop({ containerRef: ref, ...opts })) } - it('does not set isDragOver on Tauri over event (internal drags are indistinguishable)', async () => { + function emitNativeDropEvent(eventName: string, payload: DragDropEvent['payload']) { + const handler = capturedDragDropHandlers[eventName] + if (!handler) throw new Error(`No native drop handler registered for ${eventName}`) + handler({ payload }) + } + + async function waitForNativeDropListeners() { + await waitFor(() => { + expect(capturedDragDropHandlers['tauri://drag-drop']).toBeDefined() + expect(capturedDragDropHandlers['tauri://drag-leave']).toBeDefined() + }) + } + + it('registers only native events that carry actionable drop state', async () => { const { result } = renderImageDropTauri() - await waitFor(() => expect(capturedDragDropHandler).not.toBeNull()) - - act(() => { - capturedDragDropHandler!({ payload: { type: 'over', paths: [], position: { x: 100, y: 100 } } }) - }) + await waitForNativeDropListeners() expect(result.current.isDragOver).toBe(false) + expect(capturedDragDropHandlers['tauri://drag-over']).toBeUndefined() }) it('resets isDragOver on Tauri drop event', async () => { const onImageUrl = vi.fn() const { result } = renderImageDropTauri({ onImageUrl, vaultPath: '/vault' }) - await waitFor(() => expect(capturedDragDropHandler).not.toBeNull()) + await waitForNativeDropListeners() // Set isDragOver via HTML5 dragover (simulates real OS file drag) const file = new File(['data'], 'photo.png', { type: 'image/png' }) @@ -226,18 +242,16 @@ describe('useImageDrop — Tauri native drag-drop', () => { expect(result.current.isDragOver).toBe(true) act(() => { - capturedDragDropHandler!({ - payload: { type: 'drop', paths: ['/tmp/photo.png'], position: { x: 100, y: 100 } }, - }) + emitNativeDropEvent('tauri://drag-drop', { type: 'drop', paths: ['/tmp/photo.png'], position: { x: 100, y: 100 } }) }) expect(result.current.isDragOver).toBe(false) }) - it('resets isDragOver on Tauri cancel event', async () => { + it('resets isDragOver on Tauri leave event', async () => { const { result } = renderImageDropTauri() - await waitFor(() => expect(capturedDragDropHandler).not.toBeNull()) + await waitForNativeDropListeners() // Set isDragOver via HTML5 dragover first const file = new File(['data'], 'photo.png', { type: 'image/png' }) @@ -245,9 +259,20 @@ describe('useImageDrop — Tauri native drag-drop', () => { expect(result.current.isDragOver).toBe(true) act(() => { - capturedDragDropHandler!({ payload: { type: 'cancel', paths: [], position: { x: 0, y: 0 } } }) + emitNativeDropEvent('tauri://drag-leave', { type: 'leave', paths: [], position: { x: 0, y: 0 } }) }) expect(result.current.isDragOver).toBe(false) }) + + it('swallows duplicate native unlisten failures from dev-mode remounts', async () => { + nativeDropUnlisten = () => { + throw new TypeError("undefined is not an object (evaluating 'listeners[eventId].handlerId')") + } + const { unmount } = renderImageDropTauri() + + await waitForNativeDropListeners() + + expect(() => unmount()).not.toThrow() + }) }) diff --git a/src/hooks/useImageDrop.ts b/src/hooks/useImageDrop.ts index c90f13ad..4e74c58a 100644 --- a/src/hooks/useImageDrop.ts +++ b/src/hooks/useImageDrop.ts @@ -1,9 +1,16 @@ import { useEffect, useRef, useState, type RefObject } from 'react' import { invoke, convertFileSrc } from '@tauri-apps/api/core' +import type { Event as TauriEvent, UnlistenFn } from '@tauri-apps/api/event' +import type { DragDropEvent as TauriDragDropPayload } from '@tauri-apps/api/webview' import { isTauri } from '../mock-tauri' const IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif', 'image/webp'] const IMAGE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'gif', 'webp', 'svg', 'bmp', 'tiff'] +const TAURI_DRAG_DROP_EVENT = 'tauri://drag-drop' +const TAURI_DRAG_LEAVE_EVENT = 'tauri://drag-leave' + +type ImageUrlHandler = (url: string) => void +type TauriDropEvent = TauriEvent function hasImageFiles(dt: DataTransfer): boolean { for (let i = 0; i < dt.items.length; i++) { @@ -46,6 +53,44 @@ async function copyImageToVault(sourcePath: string, vaultPath: string): Promise< return convertFileSrc(savedPath) } +function insertDroppedImages( + imagePaths: string[], + vaultPath: string | undefined, + onImageUrl: ImageUrlHandler | undefined, +): void { + if (imagePaths.length === 0) return + if (!vaultPath || !onImageUrl) return + + for (const sourcePath of imagePaths) { + void copyImageToVault(sourcePath, vaultPath).then(onImageUrl) + } +} + +function cleanupNativeDropListeners(unlisteners: UnlistenFn[]): void { + for (const unlisten of unlisteners) { + void Promise.resolve() + .then(unlisten) + .catch(() => {}) + } +} + +async function registerNativeDropListeners( + handler: (event: TauriDropEvent) => void, +): Promise { + const { getCurrentWebview } = await import('@tauri-apps/api/webview') + const webview = getCurrentWebview() + const unlisteners: UnlistenFn[] = [] + + try { + unlisteners.push(await webview.listen(TAURI_DRAG_DROP_EVENT, handler)) + unlisteners.push(await webview.listen(TAURI_DRAG_LEAVE_EVENT, handler)) + return unlisteners + } catch (error) { + cleanupNativeDropListeners(unlisteners) + throw error + } +} + interface UseImageDropOptions { containerRef: RefObject /** Called with an asset URL for each image dropped via Tauri native drag-drop. */ @@ -99,33 +144,25 @@ export function useImageDrop({ containerRef, onImageUrl, vaultPath }: UseImageDr useEffect(() => { if (!isTauri()) return - let unlisten: (() => void) | null = null + let unlisteners: UnlistenFn[] = [] let mounted = true void (async () => { try { - const { getCurrentWebview } = await import('@tauri-apps/api/webview') - if (!mounted) return - unlisten = await getCurrentWebview().onDragDropEvent((event) => { - const payload = event.payload - if (payload.type === 'over') { - // Tauri 'over' events don't include paths and can't distinguish - // OS file drags from internal drags (tabs, blocks). Let the HTML5 - // dragover handler drive isDragOver — it checks hasImageFiles(). - } else if (payload.type === 'drop') { - setIsDragOver(false) - const imagePaths = payload.paths.filter(isImagePath) - const vault = vaultPathRef.current - const callback = onImageUrlRef.current - if (imagePaths.length > 0 && vault && callback) { - for (const sourcePath of imagePaths) { - void copyImageToVault(sourcePath, vault).then(callback) - } - } - } else { + const nextUnlisteners = await registerNativeDropListeners((event) => { + if (event.payload.type === 'drop') { setIsDragOver(false) + insertDroppedImages( + event.payload.paths.filter(isImagePath), + vaultPathRef.current, + onImageUrlRef.current, + ) + return } + setIsDragOver(false) }) + if (mounted) unlisteners = nextUnlisteners + else cleanupNativeDropListeners(nextUnlisteners) } catch { // Tauri webview API not available (e.g. older Tauri version) } @@ -133,7 +170,8 @@ export function useImageDrop({ containerRef, onImageUrl, vaultPath }: UseImageDr return () => { mounted = false - unlisten?.() + cleanupNativeDropListeners(unlisteners) + unlisteners = [] } }, []) diff --git a/src/hooks/useMenuEvents.test.ts b/src/hooks/useMenuEvents.test.ts index 35c1fcec..0c638621 100644 --- a/src/hooks/useMenuEvents.test.ts +++ b/src/hooks/useMenuEvents.test.ts @@ -90,6 +90,22 @@ describe('useMenuEvents', () => { expect(teardown).toHaveBeenCalledTimes(1) }) + + it('swallows stale native menu unlisten failures from dev-mode remounts', async () => { + isTauriMock.mockReturnValue(true) + const teardown = vi.fn(() => { + throw new TypeError("undefined is not an object (evaluating 'listeners[eventId].handlerId')") + }) + + listenMock.mockResolvedValueOnce(teardown) + + const { unmount } = renderHook(() => useMenuEvents(makeHandlers())) + await vi.dynamicImportSettled() + + expect(() => unmount()).not.toThrow() + await vi.dynamicImportSettled() + expect(teardown).toHaveBeenCalledTimes(1) + }) }) describe('dispatchMenuEvent', () => { diff --git a/src/hooks/useMenuEvents.ts b/src/hooks/useMenuEvents.ts index 42e33a4d..66fb4dbb 100644 --- a/src/hooks/useMenuEvents.ts +++ b/src/hooks/useMenuEvents.ts @@ -14,6 +14,8 @@ import { const NOTE_LIST_SEARCH_MENU_ID = 'edit-toggle-note-list-search' +type NativeUnlisten = () => void | Promise + export interface MenuEventHandlers extends AppCommandHandlers { activeTabPath: string | null modifiedCount?: number @@ -57,12 +59,18 @@ function syncNativeMenuState(state: MenuStatePayload): void { .catch((err) => console.warn('[menu] Failed to sync native menu state:', err)) } +function cleanupNativeMenuListener(unlisten: NativeUnlisten): void { + void Promise.resolve() + .then(unlisten) + .catch(() => {}) +} + function useNativeMenuEventListener(handlersRef: { current: MenuEventHandlers }) { useEffect(() => { if (!isTauri()) return let disposed = false - let unlisten: (() => void) | null = null + let unlisten: NativeUnlisten | null = null import('@tauri-apps/api/event') .then(async ({ listen }) => { @@ -71,7 +79,7 @@ function useNativeMenuEventListener(handlersRef: { current: MenuEventHandlers }) }) if (disposed) { - teardown() + cleanupNativeMenuListener(teardown) return } @@ -83,7 +91,7 @@ function useNativeMenuEventListener(handlersRef: { current: MenuEventHandlers }) return () => { disposed = true - unlisten?.() + if (unlisten) cleanupNativeMenuListener(unlisten) } }, [handlersRef]) } diff --git a/src/main.test.ts b/src/main.test.ts index 669b698c..c72a1005 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -49,6 +49,31 @@ async function importEntrypoint() { await import('./main') } +function createDragEventWithDataTransfer( + type: 'dragover' | 'drop', + dataTransfer: Partial, +): DragEvent { + const event = new Event(type, { bubbles: true, cancelable: true }) as DragEvent + Object.defineProperty(event, 'dataTransfer', { + value: dataTransfer, + }) + return event +} + +function createFileDataTransfer(): Partial { + return { + files: { length: 1 } as FileList, + items: { length: 0 } as DataTransferItemList, + types: ['Files'], + } +} + +function dispatchFileDragEvent(target: EventTarget, type: 'dragover' | 'drop'): DragEvent { + const event = createDragEventWithDataTransfer(type, createFileDataTransfer()) + target.dispatchEvent(event) + return event +} + function rootOptions(): ReactRootOptions { const options = mocks.createRoot.mock.calls[0]?.[1] if (!options) throw new Error('createRoot was not called with root options') @@ -89,4 +114,48 @@ describe('main entrypoint', () => { expect(mocks.sentryHandler).toHaveBeenCalledWith(error, { componentStack: '' }) }) + + it('prevents browser navigation for file drags and still lets app drop handlers run', async () => { + await importEntrypoint() + + const appDropHandler = vi.fn() + document.body.addEventListener('drop', appDropHandler, { once: true }) + + const dragOverEvent = dispatchFileDragEvent(document.body, 'dragover') + const dropEvent = dispatchFileDragEvent(document.body, 'drop') + + expect(dragOverEvent.defaultPrevented).toBe(true) + expect(dropEvent.defaultPrevented).toBe(true) + expect(appDropHandler).toHaveBeenCalledWith(dropEvent) + }) + + it('leaves editor file drags to the editor drop handler', async () => { + await importEntrypoint() + + const editor = document.createElement('div') + editor.className = 'editor__blocknote-container' + const editorChild = document.createElement('div') + editor.appendChild(editorChild) + document.body.appendChild(editor) + + const dragOverEvent = dispatchFileDragEvent(editorChild, 'dragover') + const dropEvent = dispatchFileDragEvent(editorChild, 'drop') + + expect(dragOverEvent.defaultPrevented).toBe(false) + expect(dropEvent.defaultPrevented).toBe(false) + }) + + it('does not prevent app-internal drags without file payloads', async () => { + await importEntrypoint() + + const dragOverEvent = createDragEventWithDataTransfer('dragover', { + files: { length: 0 } as FileList, + items: { length: 0 } as DataTransferItemList, + types: ['text/plain'], + }) + + document.body.dispatchEvent(dragOverEvent) + + expect(dragOverEvent.defaultPrevented).toBe(false) + }) }) diff --git a/src/main.tsx b/src/main.tsx index 7dec705d..62e1429e 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -18,6 +18,30 @@ import { } from './hooks/appCommandCatalog' import { shouldUseLinuxWindowChrome } from './utils/platform' +const EDITOR_DROP_SELECTOR = '.editor__blocknote-container' + +function dataTransferHasFiles(dataTransfer: DataTransfer | null): boolean { + if (!dataTransfer) return false + if (dataTransfer.files.length > 0) return true + if (Array.from(dataTransfer.types).includes('Files')) return true + + return Array.from(dataTransfer.items).some((item) => item.kind === 'file') +} + +function isEditorDropTarget(target: EventTarget | null): boolean { + return target instanceof Element && target.closest(EDITOR_DROP_SELECTOR) !== null +} + +function preventFileDropNavigation(event: DragEvent): void { + if (isEditorDropTarget(event.target)) return + if (!dataTransferHasFiles(event.dataTransfer)) return + + event.preventDefault() +} + +document.addEventListener('dragover', preventFileDropNavigation, true) +document.addEventListener('drop', preventFileDropNavigation, true) + // Disable native WebKit context menu in Tauri (WKWebView intercepts right-click // at native level before React's synthetic events can call preventDefault). // Capture phase fires first → prevents native menu; React bubble phase still fires