diff --git a/apps/desktop/src/App.vue b/apps/desktop/src/App.vue index 902162ba5c..ba0401009c 100644 --- a/apps/desktop/src/App.vue +++ b/apps/desktop/src/App.vue @@ -2621,16 +2621,18 @@ async function handleKeydown(e: KeyboardEvent) { void openSaveSqlDialog(); return; } + // CodeMirror ignores keydown events after an IME composition changes the + // document, so this app-level fallback must share the editor shortcut guard. if (activeTab.value?.mode === "query" && isExecuteSqlInNewResultTabShortcut(e, shortcuts) && e.target instanceof Element && e.target.closest("[data-query-editor-root]")) { e.preventDefault(); e.stopPropagation(); - requestActiveEditorExecuteInNewResultTab(); + if (!contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecuteInNewResultTab(); return; } if (activeTab.value?.mode === "query" && isExecuteSqlShortcut(e, shortcuts) && e.target instanceof Element && e.target.closest("[data-query-editor-root]")) { e.preventDefault(); e.stopPropagation(); - requestActiveEditorExecute(); + if (!contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecute(); return; } if (activeTab.value?.mode === "query" && isSendSelectionToAiShortcut(e, shortcuts) && e.target instanceof Element && e.target.closest("[data-query-editor-root]")) { diff --git a/apps/desktop/src/components/editor/QueryEditor.vue b/apps/desktop/src/components/editor/QueryEditor.vue index 15eb7937e7..546e7c04f7 100644 --- a/apps/desktop/src/components/editor/QueryEditor.vue +++ b/apps/desktop/src/components/editor/QueryEditor.vue @@ -133,6 +133,7 @@ import { extendQueryEditorSelection, runQueryEditorAltExtendSelection } from "@/ import { createQueryEditorStringMouseSelection } from "@/lib/editor/queryEditorStringMouseSelection"; import { createQueryEditorCompletionShortcutBindings } from "@/lib/editor/queryEditorCompletionShortcut"; import { acceptSelectedCompletionWithRetry, acceptSelectedOrFirstCompletion } from "@/lib/editor/queryEditorCompletionAcceptance"; +import { createQueryEditorExecutionShortcutBindings, createQueryEditorPostCompositionKeyGuard } from "@/lib/editor/queryEditorExecutionShortcut"; import type { StatementExecutionMarker } from "@/lib/tabs/tabPresentation"; import { isSchemaAware, isSingleDatabase, supportsDatabaseNameCompletion, supportsDatabaseSchemaQualifier, supportsQueryEditorBlockComments, supportsSqlInListPaste } from "@/lib/database/databaseFeatureSupport"; import { metadataSchemaForConnection, sqlSnippetDatabaseTypeForConnection } from "@/lib/database/jdbcDialect"; @@ -608,6 +609,8 @@ let editorIsActive = true; let tableReferenceDropListenerRegistered = false; let imeCompositionActive = false; let pendingImeModelEmit = false; +const postCompositionKeyGuard = createQueryEditorPostCompositionKeyGuard(); +let postCompositionKeyGuardCleanup: (() => void) | null = null; function runStatementGutterExtension(): import("@codemirror/state").Extension { const showRunButtons = !props.hideExecutionControls && settingsStore.editorSettings.showStatementRunButtons; @@ -1924,8 +1927,14 @@ function runKeymapExtension(codeMirrorKeymap: (typeof import("@codemirror/view") const binding = (shortcut: string, run: (view: EditorViewType) => boolean) => (shortcut ? [{ key: shortcutToCodeMirrorKey(shortcut), preventDefault: true, run }] : []); // Keep the shortcut on the shared execution-mode path (selection priority + configured cursor/all target), // but bypass the picker so the keyboard shortcut always executes directly instead of popping a dialog. - const executeBindings = props.hideExecutionControls ? [] : binding(shortcuts.executeSql, () => requestExecute({ bypassPicker: true })); - const executeInNewResultTabBindings = props.hideExecutionControls ? [] : binding(shortcuts.executeSqlInNewResultTab, requestExecuteInNewResultTab); + const executeBindings = props.hideExecutionControls + ? [] + : createQueryEditorExecutionShortcutBindings( + shortcuts.executeSql, + () => requestExecute({ bypassPicker: true }), + (currentView) => shouldBlockExecutionShortcut(undefined, currentView), + ); + const executeInNewResultTabBindings = props.hideExecutionControls ? [] : createQueryEditorExecutionShortcutBindings(shortcuts.executeSqlInNewResultTab, requestExecuteInNewResultTab, (currentView) => shouldBlockExecutionShortcut(undefined, currentView)); return [ Prec?.high( codeMirrorKeymap.of([ @@ -5641,6 +5650,7 @@ onMounted(async () => { }); view.value = new EditorView({ state, parent: editorElement }); + postCompositionKeyGuardCleanup = postCompositionKeyGuard.attach(view.value.contentDOM); registerEditorScrollbarPointerGuard(view.value); view.value.scrollDOM.addEventListener("scroll", scheduleEditorViewportEmit, { passive: true, @@ -5965,6 +5975,8 @@ onBeforeUnmount(() => { window.removeEventListener("keyup", clearTableNavigationHoverOnModifierRelease); window.removeEventListener("blur", clearTableNavigationHover); contextMenuPointerCleanup?.(); + postCompositionKeyGuardCleanup?.(); + postCompositionKeyGuardCleanup = null; zoomCommitScheduler.dispose(); view.value?.destroy(); }); @@ -6121,12 +6133,17 @@ function acceptGutterExecutionViewport(requestId: number) { return executionViewportOwnership.acceptRequest(requestId); } +function shouldBlockExecutionShortcut(event?: KeyboardEvent, currentView: EditorViewType | null = view.value): boolean { + return (currentView ? isEditorComposing(currentView) : false) || (event ? postCompositionKeyGuard.blocks(event) : false); +} + defineExpose({ openSearch, openReplace, scrollCursorIntoView, beginExecutionViewportTracking, acceptGutterExecutionViewport, + shouldBlockExecutionShortcut, requestExecute, requestExecuteInNewResultTab, pasteClipboardAsSqlInCondition, diff --git a/apps/desktop/src/components/layout/ContentArea.vue b/apps/desktop/src/components/layout/ContentArea.vue index 81246ef4e6..2628f88cbc 100644 --- a/apps/desktop/src/components/layout/ContentArea.vue +++ b/apps/desktop/src/components/layout/ContentArea.vue @@ -1120,6 +1120,10 @@ function requestQueryEditorExecuteInNewResultTab() { return queryEditorRef.value?.requestExecuteInNewResultTab(); } +function shouldBlockQueryEditorExecutionShortcut(event: KeyboardEvent) { + return queryEditorRef.value?.shouldBlockExecutionShortcut?.(event) ?? false; +} + function acceptQueryEditorExecutionViewport(requestId: number) { return queryEditorRef.value?.acceptGutterExecutionViewport(requestId) ?? false; } @@ -1164,6 +1168,7 @@ defineExpose({ handleModRTarget, requestQueryEditorExecute, requestQueryEditorExecuteInNewResultTab, + shouldBlockQueryEditorExecutionShortcut, acceptQueryEditorExecutionViewport, pasteClipboardAsSqlInCondition, applyTableStructureChanges, diff --git a/apps/desktop/src/lib/__tests__/editor/queryEditorExecution.spec.ts b/apps/desktop/src/lib/__tests__/editor/queryEditorExecution.spec.ts index f57df3bb31..8600a63386 100644 --- a/apps/desktop/src/lib/__tests__/editor/queryEditorExecution.spec.ts +++ b/apps/desktop/src/lib/__tests__/editor/queryEditorExecution.spec.ts @@ -1,6 +1,6 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -import { createQueryEditorExecutionViewportOwnership, isQueryEditorPositionVisible } from "@/lib/editor/queryEditorExecutionViewport"; +import { createQueryEditorExecutionViewportOwnership, isQueryEditorPositionVisible } from "../../editor/queryEditorExecutionViewport"; const queryEditorSource = readFileSync(new URL("../../../components/editor/QueryEditor.vue", import.meta.url), "utf8"); const contentAreaSource = readFileSync(new URL("../../../components/layout/ContentArea.vue", import.meta.url), "utf8"); @@ -10,12 +10,31 @@ const queryStoreSource = readFileSync(new URL("../../../stores/queryStore.ts", i describe("QueryEditor execution routing", () => { it("routes the execution shortcut through the shared execution-mode contract while bypassing the picker", () => { - expect(queryEditorSource).toContain("binding(shortcuts.executeSql, () => requestExecute({ bypassPicker: true }))"); + expect(queryEditorSource).toContain("createQueryEditorExecutionShortcutBindings(shortcuts.executeSql"); expect(queryEditorSource).not.toContain("forceCurrent"); }); + it("guards both CodeMirror execution bindings and the app-level fallback during IME composition", () => { + expect(queryEditorSource).toContain("createQueryEditorExecutionShortcutBindings(shortcuts.executeSql"); + expect(queryEditorSource).toContain("createQueryEditorExecutionShortcutBindings(shortcuts.executeSqlInNewResultTab"); + expect(queryEditorSource).toContain("isEditorComposing"); + expect(queryEditorSource).toContain("function shouldBlockExecutionShortcut(event?: KeyboardEvent"); + expect(queryEditorSource).toContain("postCompositionKeyGuard.blocks(event)"); + expect(contentAreaSource).toContain("function shouldBlockQueryEditorExecutionShortcut(event: KeyboardEvent)"); + expect(contentAreaSource).toContain("queryEditorRef.value?.shouldBlockExecutionShortcut?.(event)"); + expect(appSource).toContain("if (!contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecuteInNewResultTab();"); + expect(appSource).toContain("if (!contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecute();"); + }); + + it("keeps toolbar, context-menu, and gutter execution outside the shortcut guard", () => { + expect(queryEditorSource).toContain("function executeFromContextMenu()"); + expect(queryEditorSource).toContain("requestExecute();\n focusEditor();"); + expect(queryEditorSource).toContain("function executeSqlStatementFromGutter"); + expect(queryEditorSource).toContain("emitExecutionRequest({ ...sqlExecutionSnapshotForRange(currentView, statementRange), editorViewportRequestId })"); + }); + it("routes the new-result-tab shortcut through the same target selection contract", () => { - expect(queryEditorSource).toContain("binding(shortcuts.executeSqlInNewResultTab, requestExecuteInNewResultTab)"); + expect(queryEditorSource).toContain("createQueryEditorExecutionShortcutBindings(shortcuts.executeSqlInNewResultTab"); expect(queryEditorSource).toContain('emit("executeInNewResultTab", source)'); expect(queryEditorSource).toContain("requestExecute({ bypassPicker: true, openInNewResultTab: true })"); expect(contentAreaSource).toContain('const showResultRunTabs = computed(() => resultRuns.value.length > 0 && resultRunDisplayMode.value === "tabs")'); diff --git a/apps/desktop/src/lib/__tests__/editor/queryEditorExecutionShortcut.spec.ts b/apps/desktop/src/lib/__tests__/editor/queryEditorExecutionShortcut.spec.ts new file mode 100644 index 0000000000..271e33e48e --- /dev/null +++ b/apps/desktop/src/lib/__tests__/editor/queryEditorExecutionShortcut.spec.ts @@ -0,0 +1,90 @@ +// @vitest-environment happy-dom + +import { EditorState } from "@codemirror/state"; +import { EditorView, keymap, runScopeHandlers } from "@codemirror/view"; +import { describe, expect, it, vi } from "vitest"; +import { createQueryEditorExecutionShortcutBindings } from "../../editor/queryEditorExecutionShortcut"; + +function createView(bindings: ReturnType[]): EditorView { + return new EditorView({ + parent: document.createElement("div"), + state: EditorState.create({ + extensions: [keymap.of(bindings.flat())], + }), + }); +} + +function executeEvent(key: string, options: KeyboardEventInit = {}): KeyboardEvent { + return new KeyboardEvent("keydown", { + key, + code: key === "Enter" ? "Enter" : "Backslash", + ctrlKey: true, + bubbles: true, + cancelable: true, + ...options, + }); +} + +const isComposing = (view: EditorView): boolean => view.compositionStarted || view.composing; + +describe("QueryEditor execution shortcuts", () => { + it("executes the normal SQL shortcut when no IME composition is active", () => { + const execute = vi.fn(() => true); + const view = createView([createQueryEditorExecutionShortcutBindings("Mod+Enter", execute, isComposing)]); + + expect(view.composing).toBe(false); + expect(runScopeHandlers(view, executeEvent("Enter"), "editor")).toBe(true); + expect(execute).toHaveBeenCalledOnce(); + expect(execute).toHaveBeenCalledWith(view); + view.destroy(); + }); + + it("consumes the SQL shortcut without executing during IME composition", () => { + const execute = vi.fn(() => true); + const view = createView([createQueryEditorExecutionShortcutBindings("Mod+Enter", execute, isComposing)]); + + view.contentDOM.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + const startedEvent = executeEvent("Enter"); + view.contentDOM.dispatchEvent(startedEvent); + expect(startedEvent.defaultPrevented).toBe(true); + (view as EditorView & { inputState: { composing: number } }).inputState.composing = 1; + + expect(view.composing).toBe(true); + expect(runScopeHandlers(view, executeEvent("Enter"), "editor")).toBe(true); + expect(execute).not.toHaveBeenCalled(); + view.destroy(); + }); + + it("suppresses both execution destinations while composing", () => { + const execute = vi.fn(() => true); + const executeInNewResultTab = vi.fn(() => true); + const view = createView([createQueryEditorExecutionShortcutBindings("Mod+Enter", execute, isComposing), createQueryEditorExecutionShortcutBindings("Mod+\\", executeInNewResultTab, isComposing)]); + + view.contentDOM.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + (view as EditorView & { inputState: { composing: number } }).inputState.composing = 1; + + expect(runScopeHandlers(view, executeEvent("Enter"), "editor")).toBe(true); + expect(runScopeHandlers(view, executeEvent("\\"), "editor")).toBe(true); + expect(execute).not.toHaveBeenCalled(); + expect(executeInNewResultTab).not.toHaveBeenCalled(); + view.destroy(); + }); + + it("restores both execution shortcuts after composition ends", () => { + const execute = vi.fn(() => true); + const executeInNewResultTab = vi.fn(() => true); + const view = createView([createQueryEditorExecutionShortcutBindings("Mod+Enter", execute, isComposing), createQueryEditorExecutionShortcutBindings("Mod+\\", executeInNewResultTab, isComposing)]); + + view.contentDOM.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + (view as EditorView & { inputState: { composing: number } }).inputState.composing = 1; + view.contentDOM.dispatchEvent(new CompositionEvent("compositionend", { bubbles: true })); + + expect(view.composing).toBe(false); + expect(view.compositionStarted).toBe(false); + expect(runScopeHandlers(view, executeEvent("Enter"), "editor")).toBe(true); + expect(runScopeHandlers(view, executeEvent("\\"), "editor")).toBe(true); + expect(execute).toHaveBeenCalledOnce(); + expect(executeInNewResultTab).toHaveBeenCalledOnce(); + view.destroy(); + }); +}); diff --git a/apps/desktop/src/lib/__tests__/editor/queryEditorExecutionShortcutWebKit.spec.ts b/apps/desktop/src/lib/__tests__/editor/queryEditorExecutionShortcutWebKit.spec.ts new file mode 100644 index 0000000000..f732035bbf --- /dev/null +++ b/apps/desktop/src/lib/__tests__/editor/queryEditorExecutionShortcutWebKit.spec.ts @@ -0,0 +1,134 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it, vi } from "vitest"; + +const safariNavigator = { + maxTouchPoints: 0, + platform: "MacIntel", + userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 Version/18.6 Safari/605.1.15", + vendor: "Apple Computer, Inc.", +}; + +async function createSafariShortcutHarness() { + vi.stubGlobal("navigator", safariNavigator); + vi.resetModules(); + + const [{ EditorState }, { EditorView, keymap }, { createQueryEditorExecutionShortcutBindings, createQueryEditorPostCompositionKeyGuard }] = await Promise.all([import("@codemirror/state"), import("@codemirror/view"), import("../../editor/queryEditorExecutionShortcut")]); + const execute = vi.fn(() => true); + const fallbackExecute = vi.fn(); + const root = document.createElement("div"); + root.dataset.queryEditorRoot = ""; + document.body.append(root); + const view = new EditorView({ + parent: root, + state: EditorState.create({ + extensions: [keymap.of(createQueryEditorExecutionShortcutBindings("Mod+Enter", execute, (currentView) => currentView.compositionStarted || currentView.composing))], + }), + }); + const postCompositionKeyGuard = createQueryEditorPostCompositionKeyGuard(); + const detachGuard = postCompositionKeyGuard.attach(view.contentDOM); + let defaultPreventedBeforeFallback = false; + root.addEventListener("keydown", (event) => { + defaultPreventedBeforeFallback = event.defaultPrevented; + }); + const handleAppFallback = (event: KeyboardEvent) => { + if (event.defaultPrevented) return; + if (!event.metaKey || event.key !== "Enter" || !(event.target instanceof Element) || !event.target.closest("[data-query-editor-root]")) return; + event.preventDefault(); + event.stopPropagation(); + if (!(view.compositionStarted || view.composing || postCompositionKeyGuard.blocks(event))) fallbackExecute(); + }; + window.addEventListener("keydown", handleAppFallback); + + return { + defaultPreventedBeforeFallback: () => defaultPreventedBeforeFallback, + destroy() { + window.removeEventListener("keydown", handleAppFallback); + detachGuard(); + view.destroy(); + root.remove(); + }, + execute, + fallbackExecute, + view, + }; +} + +function dispatchComposition(target: HTMLElement, type: "compositionstart" | "compositionend") { + target.dispatchEvent(new CompositionEvent(type, { bubbles: true })); +} + +function dispatchExecuteShortcut(target: HTMLElement) { + const event = new KeyboardEvent("keydown", { + key: "Enter", + code: "Enter", + metaKey: true, + bubbles: true, + cancelable: true, + }); + target.dispatchEvent(event); + return event; +} + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.resetModules(); + document.body.replaceChildren(); +}); + +describe("QueryEditor Safari post-composition execution guard", () => { + it("does not block immediate post-composition keys outside desktop Safari", async () => { + const { createQueryEditorPostCompositionKeyGuard } = await import("../../editor/queryEditorExecutionShortcut"); + const guard = createQueryEditorPostCompositionKeyGuard({ + navigator: { + maxTouchPoints: 0, + userAgent: "Mozilla/5.0 Chrome/140.0.0.0 Safari/537.36", + vendor: "Google Inc.", + }, + }); + const target = document.createElement("div"); + const detachGuard = guard.attach(target); + + dispatchComposition(target, "compositionstart"); + dispatchComposition(target, "compositionend"); + const event = new KeyboardEvent("keydown", { key: "Enter", ctrlKey: true, bubbles: true, cancelable: true }); + target.dispatchEvent(event); + + expect(guard.blocks(event)).toBe(false); + detachGuard(); + }); + + it("blocks the bubbled App fallback for Safari pending composition key", async () => { + const harness = await createSafariShortcutHarness(); + + dispatchComposition(harness.view.contentDOM, "compositionstart"); + dispatchComposition(harness.view.contentDOM, "compositionend"); + const pendingKey = dispatchExecuteShortcut(harness.view.contentDOM); + + expect(harness.defaultPreventedBeforeFallback()).toBe(false); + expect(pendingKey.defaultPrevented).toBe(true); + expect(harness.execute).not.toHaveBeenCalled(); + expect(harness.fallbackExecute).not.toHaveBeenCalled(); + + dispatchExecuteShortcut(harness.view.contentDOM); + expect(harness.execute).toHaveBeenCalledOnce(); + expect(harness.fallbackExecute).not.toHaveBeenCalled(); + harness.destroy(); + }); + + it("allows a real execution shortcut after the pending-key window expires", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-08-25T12:00:00Z")); + const harness = await createSafariShortcutHarness(); + + dispatchComposition(harness.view.contentDOM, "compositionstart"); + dispatchComposition(harness.view.contentDOM, "compositionend"); + vi.advanceTimersByTime(101); + dispatchExecuteShortcut(harness.view.contentDOM); + + expect(harness.execute).toHaveBeenCalledOnce(); + expect(harness.fallbackExecute).not.toHaveBeenCalled(); + harness.destroy(); + }); +}); diff --git a/apps/desktop/src/lib/editor/queryEditorExecutionShortcut.ts b/apps/desktop/src/lib/editor/queryEditorExecutionShortcut.ts new file mode 100644 index 0000000000..792885d71b --- /dev/null +++ b/apps/desktop/src/lib/editor/queryEditorExecutionShortcut.ts @@ -0,0 +1,74 @@ +import type { Command, EditorView, KeyBinding } from "@codemirror/view"; +import { shortcutToCodeMirrorKey } from "@/lib/editor/shortcutRegistry"; + +const SAFARI_POST_COMPOSITION_KEY_WINDOW_MS = 100; + +type BrowserNavigator = Pick; + +function isDesktopSafari(currentNavigator: BrowserNavigator | undefined): boolean { + if (!currentNavigator) return false; + return /Apple Computer/.test(currentNavigator.vendor) && !/Mobile\/\w+/.test(currentNavigator.userAgent) && currentNavigator.maxTouchPoints <= 2; +} + +export function createQueryEditorPostCompositionKeyGuard(options: { navigator?: BrowserNavigator; now?: () => number } = {}) { + const currentNavigator = options.navigator ?? (typeof navigator === "undefined" ? undefined : navigator); + const now = options.now ?? Date.now; + const desktopSafari = isDesktopSafari(currentNavigator); + const blockedEvents = new WeakSet(); + let compositionEndedAt = 0; + let compositionPendingKey = false; + + function handleCompositionStart() { + compositionPendingKey = false; + } + + function handleCompositionEnd() { + compositionPendingKey = desktopSafari; + compositionEndedAt = desktopSafari ? now() : 0; + } + + function handleKeydown(event: KeyboardEvent) { + if (!compositionPendingKey) return; + compositionPendingKey = false; + if (now() - compositionEndedAt < SAFARI_POST_COMPOSITION_KEY_WINDOW_MS) blockedEvents.add(event); + } + + return { + attach(element: HTMLElement) { + element.addEventListener("compositionstart", handleCompositionStart, true); + element.addEventListener("compositionend", handleCompositionEnd, true); + element.addEventListener("keydown", handleKeydown, true); + return () => { + element.removeEventListener("compositionstart", handleCompositionStart, true); + element.removeEventListener("compositionend", handleCompositionEnd, true); + element.removeEventListener("keydown", handleKeydown, true); + compositionPendingKey = false; + }; + }, + blocks(event: KeyboardEvent) { + return blockedEvents.has(event); + }, + }; +} + +/** + * Create a query-editor execution binding that consumes the shortcut while an + * IME composition is active, without invoking the execution callback. + * + * CodeMirror may ignore keydown events after a composition has changed the + * document, so the app-level shortcut fallback also checks the editor state. + * This guard covers the keymap path when CodeMirror still dispatches it. + */ +export function createQueryEditorExecutionShortcutBindings(shortcut: string, run: Command, isComposing: (view: EditorView) => boolean): KeyBinding[] { + if (!shortcut) return []; + return [ + { + key: shortcutToCodeMirrorKey(shortcut), + preventDefault: true, + run(view) { + if (isComposing(view)) return true; + return run(view); + }, + }, + ]; +}