fix(editor): keep scroll after canceling dangerous sql

This commit is contained in:
jearhe
2026-09-01 12:16:59 +08:00
committed by GitHub
parent a751b5690b
commit 694611aade
9 changed files with 215 additions and 11 deletions
+1
View File
@@ -465,6 +465,7 @@ const {
onMissingDatabase: promptActiveDatabaseSelection,
requestDangerConfirmation: (request) => sqlExecutionDangerStore.requestConfirmation(request),
onExecutionStarted: (editorViewportRequestId) => contentAreaRef.value?.acceptQueryEditorExecutionViewport(editorViewportRequestId),
onExecutionCancelled: (editorViewportRequestId) => contentAreaRef.value?.cancelQueryEditorExecutionViewport(editorViewportRequestId),
});
function captureActiveEditorExecutionSnapshot() {
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { computed, ref } from "vue";
import { computed, ref, watch } from "vue";
import { useI18n } from "vue-i18n";
import { AlertTriangle, Check, Copy, Loader2, TextWrap } from "@lucide/vue";
import { Button } from "@/components/ui/button";
@@ -21,6 +21,10 @@ const suppressFuturePrompts = defineModel<boolean>("suppressFuturePrompts", { de
const wrap = ref(true);
const copied = ref(false);
// The CodeMirror editor (if any) that was focused when this dialog opened, so
// its internal selection can be restored without letting the browser move the caret.
let editorRootToRestoreFocus: HTMLElement | null = null;
const props = withDefaults(
defineProps<{
sql?: string;
@@ -73,6 +77,34 @@ const dialogOpen = computed({
},
});
watch(
() => open.value,
(isOpen) => {
if (!isOpen) return;
const active = document.activeElement;
editorRootToRestoreFocus = active instanceof HTMLElement ? active.closest(".cm-editor") : null;
},
{ immediate: true },
);
/**
* Restores focus through CodeMirror's own `EditorView.focus()` instead of the browser default.
*
* Reka UI's default close-auto-focus calls plain DOM `.focus()` on the element that was active
* before the dialog opened. In WebKit, refocusing a CodeMirror contenteditable this way can reset
* its caret to the start of the document, which makes the editor scroll to the top (#7692).
* `EditorView.focus()` restores CodeMirror's internal selection without creating that false change.
*/
function onDangerDialogCloseAutoFocus(event: Event) {
const target = editorRootToRestoreFocus;
editorRootToRestoreFocus = null;
if (!target || !target.isConnected) return;
event.preventDefault();
void import("@codemirror/view").then(({ EditorView }) => {
EditorView.findFromDOM(target)?.focus();
});
}
function onConfirm() {
// Guard here as well as on the button: a disabled button still fires on some
// synthetic/keyboard paths, and this one gates a destructive operation.
@@ -92,7 +124,7 @@ async function copyFullCode() {
<template>
<Dialog v-model:open="dialogOpen">
<DialogContent class="sm:max-w-[480px]">
<DialogContent class="sm:max-w-[480px]" @close-auto-focus="onDangerDialogCloseAutoFocus">
<DialogHeader>
<DialogTitle class="flex items-center gap-2 text-destructive">
<AlertTriangle class="h-5 w-5" />
@@ -6584,6 +6584,10 @@ function acceptGutterExecutionViewport(requestId: number) {
return executionViewportOwnership.acceptRequest(requestId);
}
function cancelGutterExecutionViewport(requestId: number) {
return executionViewportOwnership.cancelPendingRequest(requestId);
}
function shouldBlockExecutionShortcut(event?: KeyboardEvent, currentView: EditorViewType | null = view.value): boolean {
return (currentView ? isEditorComposing(currentView) : false) || (event ? postCompositionKeyGuard.blocks(event) : false);
}
@@ -6594,6 +6598,7 @@ defineExpose({
scrollCursorIntoView,
beginExecutionViewportTracking,
acceptGutterExecutionViewport,
cancelGutterExecutionViewport,
shouldBlockExecutionShortcut,
requestExecute,
requestExecuteInNewResultTab,
@@ -9,6 +9,17 @@ import { copyToClipboard } from "@/lib/common/clipboard";
const DANGER_PREVIEW_MAX_CHARACTERS = 8192;
const DANGER_PREVIEW_MAX_LINES = 200;
const highlight = vi.fn((sql: string) => `<span>${sql}</span>`);
const focusMocks = vi.hoisted(() => {
const focus = vi.fn();
return {
focus,
findFromDOM: vi.fn(() => ({ focus })),
};
});
vi.mock("@codemirror/view", () => ({
EditorView: { findFromDOM: focusMocks.findFromDOM },
}));
vi.mock("@/composables/useSqlHighlighter", () => ({
useSqlHighlighter: () => ({ highlight }),
@@ -50,6 +61,8 @@ afterEach(() => {
document.body.innerHTML = "";
highlight.mockClear();
vi.mocked(copyToClipboard).mockClear();
focusMocks.findFromDOM.mockClear();
focusMocks.focus.mockClear();
});
describe("DangerConfirmDialog SQL preview", () => {
@@ -107,6 +120,26 @@ describe("DangerConfirmDialog SQL preview", () => {
expect(copyToClipboard).toHaveBeenCalledWith(sql);
});
it("restores a previously focused CodeMirror editor through EditorView on close", async () => {
const editorRoot = document.createElement("div");
editorRoot.className = "cm-editor";
const editorContent = document.createElement("div");
editorContent.className = "cm-content";
editorContent.tabIndex = 0;
editorRoot.append(editorContent);
document.body.append(editorRoot);
editorContent.focus();
await mountDialog("DROP TABLE users;");
const cancelButton = Array.from(document.body.querySelectorAll("button")).find((button) => button.textContent?.trim() === "Cancel");
cancelButton?.click();
await vi.waitFor(() => {
expect(focusMocks.findFromDOM).toHaveBeenCalledWith(editorRoot);
});
expect(focusMocks.focus).toHaveBeenCalledOnce();
});
});
describe("DangerConfirmDialog running/cancel footer state", () => {
@@ -1139,6 +1139,10 @@ function acceptQueryEditorExecutionViewport(requestId: number) {
return queryEditorRef.value?.acceptGutterExecutionViewport(requestId) ?? false;
}
function cancelQueryEditorExecutionViewport(requestId: number) {
return queryEditorRef.value?.cancelGutterExecutionViewport(requestId) ?? false;
}
async function handleExportQuery(payload: { sql: string; format: "csv" | "xlsx" | "txt"; columnComments?: (string | null)[] }) {
const tab = props.activeTab;
if (!tab || tab.mode !== "query") return;
@@ -1183,6 +1187,7 @@ defineExpose({
requestQueryEditorExecuteInNewResultTab,
shouldBlockQueryEditorExecutionShortcut,
acceptQueryEditorExecutionViewport,
cancelQueryEditorExecutionViewport,
pasteClipboardAsSqlInCondition,
applyTableStructureChanges,
insertRedisCommand,
@@ -1,4 +1,4 @@
import { computed, ref } from "vue";
import { computed, nextTick, ref } from "vue";
import { createPinia, setActivePinia } from "pinia";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { isDangerousSql, requiresDatabaseSelection, supportsSqlTemplateParameters, useSqlExecution } from "../useSqlExecution";
@@ -7,6 +7,7 @@ import { useHistoryStore } from "@/stores/historyStore";
import { useQueryStore } from "@/stores/queryStore";
import { useSettingsStore } from "@/stores/settingsStore";
import { useProductionSafetyStore } from "@/stores/productionSafetyStore";
import { createQueryEditorExecutionViewportOwnership } from "@/lib/editor/queryEditorExecutionViewport";
import * as objectMetadataCache from "@/lib/metadata/objectMetadataCache";
import type { ConnectionConfig, QueryTab } from "@/types/database";
@@ -1114,6 +1115,97 @@ SELECT @value AS Message;`;
expect(addHistory).toHaveBeenCalledWith(expect.objectContaining({ success: true, error: undefined }));
});
it("cancels the editor viewport request when dangerous SQL is dismissed", async () => {
const sql = "DELETE FROM users WHERE id = 7";
const activeTab = ref<QueryTab | undefined>({ ...queryTab("app"), sql });
const activeConnection = ref<ConnectionConfig | undefined>(connection("mysql"));
const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result");
const queryStore = useQueryStore();
const executeCurrentSql = vi.spyOn(queryStore, "executeCurrentSql");
const onExecutionCancelled = vi.fn();
const execution = useSqlExecution({
activeTab: computed(() => activeTab.value),
activeConnection: computed(() => activeConnection.value),
executableSql: computed(() => sql),
activeOutputView,
onExecutionCancelled,
});
await execution.tryExecute({
fullSql: sql,
selectedSql: sql,
cursorPos: 0,
selectionFrom: 0,
selectionTo: sql.length,
editorViewportRequestId: 17,
});
expect(execution.showDangerDialog.value).toBe(true);
execution.showDangerDialog.value = false;
await nextTick();
expect(onExecutionCancelled).toHaveBeenCalledTimes(1);
expect(onExecutionCancelled).toHaveBeenCalledWith(17);
expect(executeCurrentSql).not.toHaveBeenCalled();
});
it("does not reuse a cancelled viewport request on the next dangerous execution", async () => {
const sql = "DELETE FROM users WHERE id = 7";
const activeTab = ref<QueryTab | undefined>({ ...queryTab("app"), sql });
const activeConnection = ref<ConnectionConfig | undefined>(connection("mysql"));
const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result");
const queryStore = useQueryStore();
const ownership = createQueryEditorExecutionViewportOwnership();
const executeCurrentSql = vi.spyOn(queryStore, "executeCurrentSql").mockImplementation(async (_sql, options) => {
options?.onExecutionStarted?.();
if (activeTab.value) activeTab.value.result = { columns: [], rows: [], affected_rows: 1, execution_time_ms: 1 };
});
vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined);
const onExecutionStarted = vi.fn((requestId: number) => ownership.acceptRequest(requestId));
const onExecutionCancelled = vi.fn((requestId: number) => ownership.cancelPendingRequest(requestId));
const execution = useSqlExecution({
activeTab: computed(() => activeTab.value),
activeConnection: computed(() => activeConnection.value),
executableSql: computed(() => sql),
activeOutputView,
onExecutionStarted,
onExecutionCancelled,
});
const snapshotFor = (editorViewportRequestId: number) => ({
fullSql: sql,
selectedSql: sql,
cursorPos: 0,
selectionFrom: 0,
selectionTo: sql.length,
editorViewportRequestId,
});
const firstRequestId = ownership.beginRequest();
await execution.tryExecute(snapshotFor(firstRequestId));
expect(execution.showDangerDialog.value).toBe(true);
execution.showDangerDialog.value = false;
await nextTick();
expect(onExecutionCancelled).toHaveBeenCalledTimes(1);
expect(onExecutionCancelled).toHaveBeenCalledWith(firstRequestId);
expect(ownership.acceptRequest(firstRequestId)).toBe(false);
const secondRequestId = ownership.beginRequest();
await execution.tryExecute(snapshotFor(secondRequestId));
expect(execution.showDangerDialog.value).toBe(true);
await execution.onDangerConfirm();
expect(onExecutionStarted).toHaveBeenCalledTimes(1);
expect(onExecutionStarted).toHaveBeenCalledWith(secondRequestId);
expect(ownership.consumeCompletionPreservation()).toBe(true);
expect(ownership.consumeCompletionPreservation()).toBe(false);
expect(executeCurrentSql).toHaveBeenCalledTimes(1);
});
it("keeps the full dangerous script and new-result-tab intent through confirmation", async () => {
const activeTab = ref<QueryTab | undefined>(queryTab("app"));
const activeConnection = ref<ConnectionConfig | undefined>(connection("mysql"));
@@ -117,6 +117,7 @@ export function useSqlExecution(deps: {
onMissingDatabase?: () => void;
requestDangerConfirmation?: (request: SqlExecutionDangerRequest) => Promise<boolean>;
onExecutionStarted?: (editorViewportRequestId: number) => void;
onExecutionCancelled?: (editorViewportRequestId: number) => void;
}) {
const { t } = useI18n();
const queryStore = useQueryStore();
@@ -144,6 +145,10 @@ export function useSqlExecution(deps: {
const pendingDangerEditorViewportRequestId = ref<number | undefined>();
let pendingSqlParameterContinuation: ((sql: string, sourceOffset?: number) => Promise<void> | void) | undefined;
function cancelEditorViewportRequest(editorViewportRequestId?: number) {
if (editorViewportRequestId !== undefined) deps.onExecutionCancelled?.(editorViewportRequestId);
}
async function resolvedExecutableSql(source?: SqlExecutionOverride): Promise<{ sql: string; sourceOffset?: number; editorViewportRequestId?: number }> {
const atSetEnabled = resolveSqlVariableSyntaxToggles(settingsStore.editorSettings.sqlVariableSyntaxOverrides, deps.activeConnection.value?.db_type, settingsStore.editorSettings.sqlVariableSubstitutionEnabled).atSet;
const databaseType = effectiveDatabaseTypeForConnection(deps.activeConnection.value) ?? deps.activeConnection.value?.db_type;
@@ -164,9 +169,13 @@ export function useSqlExecution(deps: {
const tab = deps.activeTab.value;
const { sql, sourceOffset, editorViewportRequestId } = await resolvedExecutableSql(sqlOverride);
const executionOptions = { ...options, editorViewportRequestId };
if (!tab || !sql.trim()) return;
if (!tab || !sql.trim()) {
cancelEditorViewportRequest(editorViewportRequestId);
return;
}
if (requiresDatabaseSelection(tab, deps.activeConnection.value, sql)) {
deps.onMissingDatabase?.();
cancelEditorViewportRequest(editorViewportRequestId);
return;
}
if (supportsSqlTemplateParameters(deps.activeConnection.value, sql) && prepareSqlParameterDialog(sql, sourceOffset, executionOptions)) return;
@@ -178,7 +187,10 @@ export function useSqlExecution(deps: {
}
async function continueExecute(sql: string, sourceOffset?: number, options: SqlExecutionOptions = {}) {
if (!(await ensureReadOnlyWriteAccess({ connection: deps.activeConnection.value, sql, source: t("production.sourceSqlEditor") }))) return;
if (!(await ensureReadOnlyWriteAccess({ connection: deps.activeConnection.value, sql, source: t("production.sourceSqlEditor") }))) {
cancelEditorViewportRequest(options.editorViewportRequestId);
return;
}
// Redis: block dangerous commands when toggle is on (scan entire batch for highest safety level)
if (deps.activeConnection.value?.db_type === "redis" && deps.blockDangerousRedisCommands?.value !== false) {
const commands = sql
@@ -198,6 +210,7 @@ export function useSqlExecution(deps: {
}
if (highestSafety === "blocked") {
toast(t("redis.blockedCommand", { command: "Redis" }), 5000);
cancelEditorViewportRequest(options.editorViewportRequestId);
return;
}
if (highestSafety === "confirm") {
@@ -223,6 +236,7 @@ export function useSqlExecution(deps: {
source: t("production.sourceSqlEditor"),
});
if (confirmed) await doExecute(sql, sourceOffset, options);
else cancelEditorViewportRequest(options.editorViewportRequestId);
return;
}
if (isDangerousSql(sql, deps.activeConnection.value?.db_type) && settingsStore.editorSettings.confirmDangerousSqlExecution) {
@@ -284,13 +298,19 @@ export function useSqlExecution(deps: {
}
async function doExecute(sql?: string, sourceOffset?: number, options: SqlExecutionOptions = {}) {
if (sql === undefined) ({ sql, sourceOffset } = await resolvedExecutableSql());
if (sql === undefined) {
({ sql, sourceOffset } = await resolvedExecutableSql());
}
const tab = deps.activeTab.value;
if (!tab || !sql.trim()) return;
if (!tab || !sql.trim()) {
cancelEditorViewportRequest(options.editorViewportRequestId);
return;
}
const executionConnection = connectionStore.getConfig(tab.connectionId) ?? deps.activeConnection.value;
const executionDatabaseType = executionConnection?.db_type;
if (requiresDatabaseSelection(tab, executionConnection, sql)) {
deps.onMissingDatabase?.();
cancelEditorViewportRequest(options.editorViewportRequestId);
return;
}
const statementCount = splitSqlStatementRanges(sql, executionDatabaseType).length;
@@ -304,7 +324,10 @@ export function useSqlExecution(deps: {
...(options.openInNewResultTab ? { openInNewResultTab: true } : {}),
...(options.editorViewportRequestId !== undefined ? { onExecutionStarted: () => deps.onExecutionStarted?.(options.editorViewportRequestId!) } : {}),
});
if (producedResult === false) return;
if (producedResult === false) {
cancelEditorViewportRequest(options.editorViewportRequestId);
return;
}
const executionTabStillActive = deps.activeTab.value?.id === tab.id;
const sqlServerMessageResultIndex = executionDatabaseType === "sqlserver" ? tab.results?.findIndex((result) => result.server_message === true) : undefined;
if (sqlServerMessageResultIndex !== undefined && sqlServerMessageResultIndex >= 0) {
@@ -554,7 +577,10 @@ export function useSqlExecution(deps: {
settingsStore.updateEditorSettings({ confirmDangerousSqlExecution: false });
}
suppressDangerConfirm.value = false;
if (!(await ensureReadOnlyWriteAccess({ connection: deps.activeConnection.value, sql, source: t("production.sourceSqlEditor") }))) return;
if (!(await ensureReadOnlyWriteAccess({ connection: deps.activeConnection.value, sql, source: t("production.sourceSqlEditor") }))) {
cancelEditorViewportRequest(editorViewportRequestId);
return;
}
await doExecute(sql, sourceOffset, { openInNewResultTab, editorViewportRequestId });
}
@@ -578,6 +604,7 @@ export function useSqlExecution(deps: {
watch(showSqlParameterDialog, (open) => {
if (open) return;
const editorViewportRequestId = pendingSqlParameterEditorViewportRequestId.value;
sqlParameterSourceSql.value = "";
sqlParameterNames.value = [];
sqlParameterDatabaseType.value = undefined;
@@ -586,16 +613,19 @@ export function useSqlExecution(deps: {
pendingOpenInNewResultTab.value = false;
pendingSqlParameterEditorViewportRequestId.value = undefined;
pendingSqlParameterContinuation = undefined;
cancelEditorViewportRequest(editorViewportRequestId);
});
watch(showDangerDialog, (open) => {
if (open) return;
const editorViewportRequestId = pendingDangerEditorViewportRequestId.value;
pendingDangerSql.value = "";
pendingDangerSourceOffset.value = undefined;
pendingDangerKind.value = "sql";
pendingOpenInNewResultTab.value = false;
pendingDangerEditorViewportRequestId.value = undefined;
suppressDangerConfirm.value = false;
cancelEditorViewportRequest(editorViewportRequestId);
});
return {
@@ -92,8 +92,11 @@ describe("QueryEditor execution routing", () => {
it("claims gutter viewport ownership only after the matching execution starts", () => {
expect(appSource).toContain("acceptQueryEditorExecutionViewport(editorViewportRequestId)");
expect(appSource).toContain("onExecutionCancelled: (editorViewportRequestId) => contentAreaRef.value?.cancelQueryEditorExecutionViewport(editorViewportRequestId)");
expect(contentAreaSource).toContain("acceptGutterExecutionViewport(requestId)");
expect(contentAreaSource).toContain("cancelGutterExecutionViewport(requestId)");
expect(sqlExecutionSource).toContain("onExecutionStarted: () => deps.onExecutionStarted?.(options.editorViewportRequestId!)");
expect(sqlExecutionSource).toContain("onExecutionCancelled?: (editorViewportRequestId: number) => void;");
expect(queryStoreSource.indexOf("tab.isExecuting = true")).toBeLessThan(queryStoreSource.indexOf("options?.onExecutionStarted?.()"));
});
@@ -160,7 +163,7 @@ describe("QueryEditor execution viewport ownership", () => {
const ownership = createQueryEditorExecutionViewportOwnership();
const cancelledRequestId = ownership.beginRequest();
ownership.cancelPendingRequest();
expect(ownership.cancelPendingRequest(cancelledRequestId)).toBe(true);
expect(ownership.acceptRequest(cancelledRequestId)).toBe(false);
expect(ownership.consumeCompletionPreservation()).toBe(false);
@@ -21,8 +21,11 @@ export function createQueryEditorExecutionViewportOwnership() {
pendingRequestId = nextRequestId;
return nextRequestId;
},
cancelPendingRequest() {
cancelPendingRequest(requestId?: number): boolean {
if (requestId !== undefined && pendingRequestId !== requestId) return false;
if (pendingRequestId === undefined) return false;
pendingRequestId = undefined;
return true;
},
acceptRequest(requestId: number): boolean {
if (pendingRequestId !== requestId) return false;