diff --git a/src/lib/openFile.js b/src/lib/openFile.js index 24dc8979d..e4b6dee1e 100644 --- a/src/lib/openFile.js +++ b/src/lib/openFile.js @@ -13,6 +13,8 @@ import fileTypeHandler from "./fileTypeHandler"; import recents from "./recents"; import appSettings from "./settings"; +let loadingFileCount = 0; + /** * @typedef {object} FileOptions * @property {string} text @@ -24,6 +26,7 @@ import appSettings from "./settings"; * @property {string} uri * @property {string} paneId * @property {boolean} persistInSession + * @property {AbortSignal} signal Discard an obsolete open before activating its file. */ /** @@ -33,6 +36,9 @@ import appSettings from "./settings"; */ export default async function openFile(file, options = {}) { + const { signal } = options; + if (signal?.aborted) return; + let releaseTitleLoader; try { let uri = typeof file === "string" ? file : file.uri; if (!uri) return; @@ -112,13 +118,15 @@ export default async function openFile(file, options = {}) { return; } - loader.showTitleLoader(); + releaseTitleLoader = acquireTitleLoader(signal); const settings = appSettings.value; const fs = fsOperation(uri); const fileInfo = await fs.stat(); + if (signal?.aborted) return; const name = fileInfo.name || file.filename || uri; const readOnly = fileInfo.canWrite === false; const createEditor = (isUnsaved, text, detectedEncoding) => { + if (signal?.aborted) return; new EditorFile(name, { uri, text, @@ -152,10 +160,12 @@ export default async function openFile(file, options = {}) { encoding, mode, createEditor, + signal, }, }); return; } catch (error) { + if (signal?.aborted) return; console.error(`File handler '${customHandler.id}' failed:`, error); // Continue with default handling if custom handler fails } @@ -173,6 +183,10 @@ export default async function openFile(file, options = {}) { if (videoRegex.test(name)) { const objectUrl = await fileToDataUrl(uri); + if (signal?.aborted) { + URL.revokeObjectURL(objectUrl); + return; + } const videoContainer = (
{ + // An aborted filesystem call may settle much later. Release immediately + // on abort, and make its eventual finally block a no-op. + if (released) return; + released = true; + signal?.removeEventListener("abort", release); + if (--loadingFileCount === 0) loader.removeTitleLoader(); + }; + signal?.addEventListener("abort", release, { once: true }); + return release; +} + /** * Converts file to data url * @param {string} file file url diff --git a/src/sidebarApps/searchInFiles/index.js b/src/sidebarApps/searchInFiles/index.js index 00f245058..9efae14b0 100644 --- a/src/sidebarApps/searchInFiles/index.js +++ b/src/sidebarApps/searchInFiles/index.js @@ -1,6 +1,5 @@ import "./styles.scss"; import fsOperation from "fileSystem"; -import { EditorView } from "@codemirror/view"; import autosize from "autosize"; import { getDocText } from "cm/editorUtils"; import Checkbox from "components/checkbox"; @@ -15,6 +14,7 @@ import { addedFolder } from "lib/openFolder"; import settings from "lib/settings"; import helpers from "utils/helpers"; import { createSearchResultView } from "./cmResultView"; +import navigateToResult from "./navigateToResult"; // Local highlight sources const words = []; @@ -1121,27 +1121,13 @@ async function onCursorChange(line) { const result = results[line]; if (!result) return; const { file, position } = result; - if (!position) { - // header line clicked; CM view folding not implemented yet - return; - } + const url = filesSearched[file]?.url; + if (!position || !url) return; rememberResultScroll(); Sidebar.hide(); - const { url } = filesSearched[file]; - await openFile(url, { render: true }); - const { editor } = editorManager; try { - // Compute offsets from row/column (rows from worker are 0-based) - const doc = editor.state.doc; - const startLine = doc.line(position.start.row + 1); - const endLine = doc.line(position.end.row + 1); - const from = Math.min(startLine.from + position.start.column, startLine.to); - const to = Math.min(endLine.from + position.end.column, endLine.to); - editor.dispatch({ - selection: { anchor: from, head: to }, - effects: EditorView.scrollIntoView(from, { y: "center" }), - }); + await navigateToResult(url, position); } catch (error) { console.warn(`Failed to focus search result at line ${line}.`, error); } diff --git a/src/sidebarApps/searchInFiles/navigateToResult.js b/src/sidebarApps/searchInFiles/navigateToResult.js new file mode 100644 index 000000000..f4f53fe0a --- /dev/null +++ b/src/sidebarApps/searchInFiles/navigateToResult.js @@ -0,0 +1,59 @@ +import openFile from "lib/openFile"; + +let currentNavigation; + +/** Open a search match and reveal its zero-based row/column range. */ +export default async function navigateToResult(url, position) { + currentNavigation?.abort(); + const navigation = new AbortController(); + currentNavigation = navigation; + const { signal } = navigation; + + try { + // Start immediately even if an obsolete filesystem operation is stalled. + // openFile checks the signal before creating/activating a late result. + await openFile(url, { render: true, signal }); + const file = editorManager.getFile(url, "uri"); + if ( + signal.aborted || + file?.type !== "editor" || + editorManager.activeFile !== file + ) { + return false; + } + + // load() reuses a restored tab's in-flight load. Do not cancel that shared + // load; only discard this request's reveal if a newer result is selected. + await file.load(); + if ( + signal.aborted || + !file.loaded || + file.loading || + editorManager.activeFile !== file || + editorManager.getFile(url, "uri") !== file + ) { + return false; + } + + const doc = editorManager.editor.state.doc; + const from = positionToOffset(doc, position.start); + const to = positionToOffset(doc, position.end); + // Cancel delayed tab scroll restoration and scrollbar locks before reveal. + return editorManager.revealRange(from, to, { + y: "center", + userEvent: "select.search", + }); + } catch (error) { + if (signal.aborted) return false; + throw error; + } finally { + if (currentNavigation === navigation) currentNavigation = undefined; + } +} + +function positionToOffset(doc, { row, column }) { + // Search results can outlive edits to the file. Clamp both coordinates so + // an older result still navigates to the nearest available position. + const line = doc.line(Math.max(1, Math.min(row + 1, doc.lines))); + return line.from + Math.max(0, Math.min(column, line.length)); +} diff --git a/tests/unit/openFileCancellation.test.js b/tests/unit/openFileCancellation.test.js new file mode 100644 index 000000000..4f8cd26a3 --- /dev/null +++ b/tests/unit/openFileCancellation.test.js @@ -0,0 +1,223 @@ +import fs from "node:fs"; +import ts from "typescript"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// openFile contains app-specific JSX. Compile the actual module for this +// isolated test, supplying its Cordova/UI dependencies without booting the app. +const source = fs.readFileSync( + new URL("../../src/lib/openFile.js", import.meta.url), + "utf8", +); +const { outputText } = ts.transpileModule(source, { + fileName: "openFile.jsx", + compilerOptions: { + module: ts.ModuleKind.CommonJS, + target: ts.ScriptTarget.ES2020, + jsx: ts.JsxEmit.React, + }, +}); + +function deferred() { + let resolve; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +describe("openFile cancellation", () => { + let openFile; + let manager; + let stat; + let readFile; + let decode; + let detectEncoding; + let handler; + let createEditor; + let recents; + let controller; + let titleLoader; + let loaderVisible; + + beforeEach(() => { + controller = new AbortController(); + loaderVisible = false; + titleLoader = { + showTitleLoader: vi.fn(() => { + loaderVisible = true; + }), + removeTitleLoader: vi.fn(() => { + loaderVisible = false; + }), + }; + manager = { getFile: vi.fn(), activeFile: null }; + stat = vi.fn().mockResolvedValue({ name: "target.txt", length: 10 }); + readFile = vi.fn().mockResolvedValue("bytes"); + decode = vi.fn().mockResolvedValue("target text"); + detectEncoding = vi.fn().mockResolvedValue("UTF-8"); + handler = { getFileHandler: vi.fn() }; + recents = { addFile: vi.fn() }; + createEditor = vi.fn(function (name, options) { + manager.activeFile = { name, ...options }; + }); + const dependencies = { + fileSystem: { default: () => ({ stat, readFile }) }, + "@codemirror/state": {}, + "components/audioPlayer": {}, + "dialogs/alert": {}, + "dialogs/confirm": {}, + "dialogs/loader": { + default: titleLoader, + }, + "palettes/changeEncoding": {}, + "utils/encodings": { decode, detectEncoding }, + "utils/helpers": { + default: { getStatMtime: () => 0, isBinary: () => false }, + }, + "./editorFile": { default: createEditor }, + "./fileSessionPersistence": { promoteSessionPersistence: vi.fn() }, + "./fileTypeHandler": { default: handler }, + "./recents": { default: recents }, + "./settings": { + default: { value: { maxFileSize: 10, defaultFileEncoding: "auto" } }, + }, + }; + const exports = {}; + new Function("require", "exports", "editorManager", outputText)( + (name) => { + if (!(name in dependencies)) throw new Error(`Unexpected import: ${name}`); + return dependencies[name]; + }, + exports, + manager, + ); + openFile = exports.default; + }); + + it("preserves normal file opening without a signal", async () => { + await openFile("target", { render: true }); + expect(createEditor).toHaveBeenCalledOnce(); + expect(manager.activeFile.text).toBe("target text"); + expect(recents.addFile).toHaveBeenCalledWith("target"); + }); + + it("does not activate an existing file with an already-aborted signal", async () => { + const file = { makeActive: vi.fn() }; + manager.getFile.mockReturnValue(file); + controller.abort(); + await openFile("target", { render: true, signal: controller.signal }); + expect(file.makeActive).not.toHaveBeenCalled(); + }); + + it.each(["stat", "read", "detect", "decode"])( + "does not steal focus when cancelled during %s", + async (stage) => { + const waiting = deferred(); + const operation = { stat, read: readFile, detect: detectEncoding, decode }[ + stage + ]; + operation.mockReturnValueOnce(waiting.promise); + const opening = openFile("obsolete", { + render: true, + signal: controller.signal, + }); + await vi.waitFor(() => expect(operation).toHaveBeenCalledOnce()); + controller.abort(); + + // A newer open completes before the obsolete filesystem work does. + await openFile("latest", { render: true }); + const latest = manager.activeFile; + waiting.resolve( + stage === "stat" ? { name: "obsolete.txt", length: 10 } : "text", + ); + await opening; + expect(manager.activeFile).toBe(latest); + expect(createEditor).toHaveBeenCalledOnce(); + expect(recents.addFile).toHaveBeenCalledExactlyOnceWith("latest"); + }, + ); + + it("guards a custom handler's delayed createEditor callback", async () => { + const waiting = deferred(); + const handleFile = vi.fn(async ({ options }) => { + await waiting.promise; + options.createEditor(false, "obsolete text"); + }); + handler.getFileHandler.mockReturnValue({ handleFile }); + const opening = openFile("obsolete", { signal: controller.signal }); + await vi.waitFor(() => expect(handleFile).toHaveBeenCalledOnce()); + controller.abort(); + waiting.resolve(); + await opening; + expect(createEditor).not.toHaveBeenCalled(); + }); + + it("keeps the latest loader visible when an aborted earlier read settles", async () => { + const obsoleteRead = deferred(); + const latestRead = deferred(); + readFile.mockReturnValueOnce(obsoleteRead.promise); + readFile.mockReturnValueOnce(latestRead.promise); + const obsolete = openFile("obsolete", { signal: controller.signal }); + await vi.waitFor(() => expect(readFile).toHaveBeenCalledTimes(1)); + controller.abort(); + const latest = openFile("latest"); + await vi.waitFor(() => expect(readFile).toHaveBeenCalledTimes(2)); + expect(loaderVisible).toBe(true); + const removals = titleLoader.removeTitleLoader.mock.calls.length; + + obsoleteRead.resolve("obsolete bytes"); + await obsolete; + expect(loaderVisible).toBe(true); + expect(titleLoader.removeTitleLoader).toHaveBeenCalledTimes(removals); + latestRead.resolve("latest bytes"); + await latest; + expect(loaderVisible).toBe(false); + }); + + it.each([0, 1])( + "keeps the indicator until both concurrent opens finish (first to finish: %s)", + async (first) => { + const reads = [deferred(), deferred()]; + readFile.mockReturnValueOnce(reads[0].promise); + readFile.mockReturnValueOnce(reads[1].promise); + const opens = [openFile("one"), openFile("two")]; + await vi.waitFor(() => expect(readFile).toHaveBeenCalledTimes(2)); + reads[first].resolve("bytes"); + await opens[first]; + expect(loaderVisible).toBe(true); + expect(titleLoader.removeTitleLoader).not.toHaveBeenCalled(); + reads[1 - first].resolve("bytes"); + await opens[1 - first]; + expect(loaderVisible).toBe(false); + expect(titleLoader.removeTitleLoader).toHaveBeenCalledOnce(); + }, + ); + + it("releases a cancelled open's loader without waiting for its read", async () => { + const reading = deferred(); + readFile.mockReturnValueOnce(reading.promise); + const opening = openFile("target", { signal: controller.signal }); + await vi.waitFor(() => expect(readFile).toHaveBeenCalledOnce()); + expect(loaderVisible).toBe(true); + controller.abort(); + expect(loaderVisible).toBe(false); + expect(titleLoader.removeTitleLoader).toHaveBeenCalledOnce(); + reading.resolve("bytes"); + await opening; + expect(titleLoader.removeTitleLoader).toHaveBeenCalledOnce(); + }); + + it("does not hide another open's loader when activating an existing tab", async () => { + const reading = deferred(); + readFile.mockReturnValueOnce(reading.promise); + const opening = openFile("loading"); + await vi.waitFor(() => expect(readFile).toHaveBeenCalledOnce()); + manager.getFile.mockReturnValue({ makeActive: vi.fn() }); + await openFile("existing"); + expect(loaderVisible).toBe(true); + expect(titleLoader.removeTitleLoader).not.toHaveBeenCalled(); + reading.resolve("bytes"); + await opening; + expect(loaderVisible).toBe(false); + }); +}); diff --git a/tests/unit/searchResultNavigation.test.js b/tests/unit/searchResultNavigation.test.js new file mode 100644 index 000000000..ace8ed897 --- /dev/null +++ b/tests/unit/searchResultNavigation.test.js @@ -0,0 +1,229 @@ +import { EditorState } from "@codemirror/state"; +import openFile from "lib/openFile"; +import navigateToResult from "sidebarApps/searchInFiles/navigateToResult"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("lib/openFile", () => ({ default: vi.fn() })); + +function deferred() { + let resolve; + let reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +const match = { + start: { row: 1, column: 2 }, + end: { row: 1, column: 5 }, +}; + +describe("project search result navigation", () => { + let manager; + let files; + + function addFile(uri, text = "first\n match here\nlast") { + const file = { + uri, + type: "editor", + loaded: true, + loading: false, + session: EditorState.create({ doc: text }), + load: vi.fn().mockResolvedValue(undefined), + }; + files.set(uri, file); + return file; + } + + function activate(file) { + manager.activeFile = file; + manager.editor = { state: file.session }; + } + + beforeEach(() => { + files = new Map(); + manager = { + activeFile: null, + editor: null, + getFile: vi.fn((uri) => files.get(uri)), + revealRange: vi.fn(() => true), + }; + vi.stubGlobal("editorManager", manager); + openFile.mockReset(); + openFile.mockImplementation(async (uri, { signal }) => { + if (signal.aborted) return; + const file = files.get(uri) || addFile(uri); + activate(file); + }); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it.each([false, true])( + "reveals the exact match through the scroll-restore-safe API (existing tab: %s)", + async (existing) => { + if (existing) addFile("target"); + expect(await navigateToResult("target", match)).toBe(true); + expect(openFile).toHaveBeenCalledWith("target", { + render: true, + signal: expect.any(AbortSignal), + }); + expect(manager.revealRange).toHaveBeenCalledWith(8, 11, { + y: "center", + userEvent: "select.search", + }); + }, + ); + + it("waits for a restored tab's final document instead of its preview", async () => { + const file = addFile("target", "preview"); + const loading = deferred(); + file.loaded = false; + file.loading = true; + file.load.mockImplementation(() => loading.promise); + const navigation = navigateToResult("target", match); + await vi.waitFor(() => expect(file.load).toHaveBeenCalled()); + expect(manager.revealRange).not.toHaveBeenCalled(); + + file.session = EditorState.create({ doc: "first\n match here\nlast" }); + file.loaded = true; + file.loading = false; + activate(file); + loading.resolve(); + expect(await navigation).toBe(true); + expect(manager.revealRange).toHaveBeenCalledWith(8, 11, expect.anything()); + }); + + it("reveals the latest result without waiting for an obsolete open", async () => { + const opening = deferred(); + openFile.mockImplementationOnce(async (uri, { signal }) => { + await opening.promise; + if (!signal.aborted) activate(addFile(uri)); + }); + const first = navigateToResult("slow", match); + await vi.waitFor(() => expect(openFile).toHaveBeenCalledTimes(1)); + const skipped = navigateToResult("intermediate", match); + const last = navigateToResult("latest", match); + await expect(last).resolves.toBe(true); + expect(manager.activeFile.uri).toBe("latest"); + expect(openFile.mock.calls[0][1].signal.aborted).toBe(true); + opening.resolve(); + + expect(await Promise.all([first, skipped, last])).toEqual([ + false, + false, + true, + ]); + expect(openFile.mock.calls.map(([uri]) => uri)).toEqual([ + "slow", + "intermediate", + "latest", + ]); + expect(manager.activeFile.uri).toBe("latest"); + expect(manager.revealRange).toHaveBeenCalledTimes(1); + }); + + it("reveals another file while an obsolete restored tab is still loading", async () => { + const file = addFile("slow"); + const loading = deferred(); + file.loaded = false; + file.loading = true; + file.load.mockImplementation(() => loading.promise); + const first = navigateToResult("slow", match); + await vi.waitFor(() => expect(file.load).toHaveBeenCalled()); + + await expect(navigateToResult("latest", match)).resolves.toBe(true); + expect(manager.activeFile.uri).toBe("latest"); + file.loaded = true; + file.loading = false; + loading.resolve(); + await expect(first).resolves.toBe(false); + expect(manager.activeFile.uri).toBe("latest"); + expect(manager.revealRange).toHaveBeenCalledTimes(1); + }); + + it("ignores late failures from an obsolete open", async () => { + const opening = deferred(); + openFile.mockImplementationOnce(() => opening.promise); + const first = navigateToResult("slow", match); + await expect(navigateToResult("latest", match)).resolves.toBe(true); + opening.reject(new Error("Obsolete read failed")); + await expect(first).resolves.toBe(false); + expect(manager.activeFile.uri).toBe("latest"); + }); + + it("uses the latest match when the same file is still loading", async () => { + const file = addFile("target"); + const loading = deferred(); + file.load.mockImplementationOnce(() => loading.promise); + const first = navigateToResult("target", match); + await vi.waitFor(() => expect(file.load).toHaveBeenCalled()); + const last = navigateToResult("target", { + start: { row: 2, column: 0 }, + end: { row: 2, column: 4 }, + }); + loading.resolve(); + expect(await Promise.all([first, last])).toEqual([false, true]); + expect(manager.revealRange).toHaveBeenCalledExactlyOnceWith( + 19, + 23, + expect.anything(), + ); + }); + + it.each(["switched", "closed", "failed"])( + "does not reveal into another document after the target is %s during loading", + async (action) => { + const file = addFile("target"); + const loading = deferred(); + file.load.mockImplementation(() => loading.promise); + const navigation = navigateToResult("target", match); + await vi.waitFor(() => expect(file.load).toHaveBeenCalled()); + if (action === "switched") activate(addFile("other")); + if (action === "closed") files.delete("target"); + if (action === "failed") file.loaded = false; + loading.resolve(); + expect(await navigation).toBe(false); + expect(manager.revealRange).not.toHaveBeenCalled(); + }, + ); + + it.each(["missing", "image"])( + "does not navigate the previous editor when opening a %s target", + async (type) => { + activate(addFile("previous")); + openFile.mockImplementationOnce(async () => { + if (type === "image") { + const file = addFile("target"); + file.type = "image"; + activate(file); + } + }); + expect(await navigateToResult("target", match)).toBe(false); + expect(manager.revealRange).not.toHaveBeenCalled(); + }, + ); + + it("handles multiline ranges and clamps stale coordinates", async () => { + addFile("target", "abc\ndef"); + await navigateToResult("target", { + start: { row: 0, column: 1 }, + end: { row: 1, column: 2 }, + }); + expect(manager.revealRange).toHaveBeenLastCalledWith(1, 6, expect.anything()); + await navigateToResult("target", { + start: { row: -1, column: -2 }, + end: { row: 100, column: 100 }, + }); + expect(manager.revealRange).toHaveBeenLastCalledWith(0, 7, expect.anything()); + }); + + it("continues navigating after an open rejects", async () => { + openFile.mockRejectedValueOnce(new Error("Read failed")); + await expect(navigateToResult("broken", match)).rejects.toThrow("Read failed"); + expect(await navigateToResult("target", match)).toBe(true); + expect(manager.activeFile.uri).toBe("target"); + }); +});