From e29eaeec6430e6bcf7080030029e7c48a9d0d7b6 Mon Sep 17 00:00:00 2001 From: Raunak Raj <71929976+bajrangCoder@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:45:29 +0530 Subject: [PATCH 1/3] fix: race condition when moving to exact line/col --- src/sidebarApps/searchInFiles/index.js | 22 +- .../searchInFiles/navigateToResult.js | 58 ++++++ tests/unit/searchResultNavigation.test.js | 189 ++++++++++++++++++ 3 files changed, 251 insertions(+), 18 deletions(-) create mode 100644 src/sidebarApps/searchInFiles/navigateToResult.js create mode 100644 tests/unit/searchResultNavigation.test.js 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..e2a279811 --- /dev/null +++ b/src/sidebarApps/searchInFiles/navigateToResult.js @@ -0,0 +1,58 @@ +import openFile from "lib/openFile"; + +let latestRequest = 0; +let pendingNavigation = Promise.resolve(); + +/** Open a search match and reveal its zero-based row/column range. */ +export default function navigateToResult(url, position) { + const request = ++latestRequest; + + // Opening a file activates it. Serialize opens so a slow, older request + // cannot activate its tab after the user's latest result has been revealed. + pendingNavigation = pendingNavigation + .catch(() => {}) + .then(async () => { + if (request !== latestRequest) return false; + await openFile(url, { render: true }); + + const file = editorManager.getFile(url, "uri"); + if ( + request !== latestRequest || + file?.type !== "editor" || + editorManager.activeFile !== file + ) { + return false; + } + + // Restored tabs may still contain an empty document or a loading preview. + // load() reuses the in-flight load and resolves after the final state swap. + await file.load(); + if ( + request !== latestRequest || + !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); + // This cancels delayed tab scroll restoration and scrollbar locks before + // selecting, scrolling, and focusing the target editor (including panes). + return editorManager.revealRange(from, to, { + y: "center", + userEvent: "select.search", + }); + }); + return pendingNavigation; +} + +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/searchResultNavigation.test.js b/tests/unit/searchResultNavigation.test.js new file mode 100644 index 000000000..cf91dce56 --- /dev/null +++ b/tests/unit/searchResultNavigation.test.js @@ -0,0 +1,189 @@ +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) => { + 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 }); + 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("keeps the latest result active when an earlier file opens slowly", async () => { + const opening = deferred(); + openFile.mockImplementationOnce(async (uri) => { + await opening.promise; + 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); + opening.resolve(); + + expect(await Promise.all([first, skipped, last])).toEqual([ + false, + false, + true, + ]); + expect(openFile.mock.calls.map(([uri]) => uri)).toEqual(["slow", "latest"]); + expect(manager.activeFile.uri).toBe("latest"); + expect(manager.revealRange).toHaveBeenCalledTimes(1); + }); + + 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"); + }); +}); From f572b21024f503a888ec1263b56ff64b457726a6 Mon Sep 17 00:00:00 2001 From: Raunak Raj <71929976+bajrangCoder@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:53:17 +0530 Subject: [PATCH 2/3] fix(search): cancel obsolete result navigation --- src/lib/openFile.js | 26 +++- .../searchInFiles/navigateToResult.js | 85 ++++++----- tests/unit/openFileCancellation.test.js | 143 ++++++++++++++++++ tests/unit/searchResultNavigation.test.js | 52 ++++++- 4 files changed, 256 insertions(+), 50 deletions(-) create mode 100644 tests/unit/openFileCancellation.test.js diff --git a/src/lib/openFile.js b/src/lib/openFile.js index 24dc8979d..a19eec1ac 100644 --- a/src/lib/openFile.js +++ b/src/lib/openFile.js @@ -24,6 +24,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 +34,8 @@ import appSettings from "./settings"; */ export default async function openFile(file, options = {}) { + const { signal } = options; + if (signal?.aborted) return; try { let uri = typeof file === "string" ? file : file.uri; if (!uri) return; @@ -116,9 +119,11 @@ export default async function openFile(file, options = {}) { 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 +157,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 +180,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 = (
{}) - .then(async () => { - if (request !== latestRequest) return false; - await openFile(url, { render: true }); + 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; + } - const file = editorManager.getFile(url, "uri"); - if ( - request !== latestRequest || - 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; + } - // Restored tabs may still contain an empty document or a loading preview. - // load() reuses the in-flight load and resolves after the final state swap. - await file.load(); - if ( - request !== latestRequest || - !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); - // This cancels delayed tab scroll restoration and scrollbar locks before - // selecting, scrolling, and focusing the target editor (including panes). - return editorManager.revealRange(from, to, { - y: "center", - userEvent: "select.search", - }); + 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", }); - return pendingNavigation; + } catch (error) { + if (signal.aborted) return false; + throw error; + } finally { + if (currentNavigation === navigation) currentNavigation = undefined; + } } function positionToOffset(doc, { row, column }) { diff --git a/tests/unit/openFileCancellation.test.js b/tests/unit/openFileCancellation.test.js new file mode 100644 index 000000000..d7735cfc8 --- /dev/null +++ b/tests/unit/openFileCancellation.test.js @@ -0,0 +1,143 @@ +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; + + beforeEach(() => { + controller = new AbortController(); + 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: { showTitleLoader: vi.fn(), removeTitleLoader: vi.fn() }, + }, + "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(); + }); +}); diff --git a/tests/unit/searchResultNavigation.test.js b/tests/unit/searchResultNavigation.test.js index cf91dce56..ace8ed897 100644 --- a/tests/unit/searchResultNavigation.test.js +++ b/tests/unit/searchResultNavigation.test.js @@ -52,7 +52,8 @@ describe("project search result navigation", () => { }; vi.stubGlobal("editorManager", manager); openFile.mockReset(); - openFile.mockImplementation(async (uri) => { + openFile.mockImplementation(async (uri, { signal }) => { + if (signal.aborted) return; const file = files.get(uri) || addFile(uri); activate(file); }); @@ -65,7 +66,10 @@ describe("project search result navigation", () => { async (existing) => { if (existing) addFile("target"); expect(await navigateToResult("target", match)).toBe(true); - expect(openFile).toHaveBeenCalledWith("target", { render: true }); + expect(openFile).toHaveBeenCalledWith("target", { + render: true, + signal: expect.any(AbortSignal), + }); expect(manager.revealRange).toHaveBeenCalledWith(8, 11, { y: "center", userEvent: "select.search", @@ -92,16 +96,19 @@ describe("project search result navigation", () => { expect(manager.revealRange).toHaveBeenCalledWith(8, 11, expect.anything()); }); - it("keeps the latest result active when an earlier file opens slowly", async () => { + it("reveals the latest result without waiting for an obsolete open", async () => { const opening = deferred(); - openFile.mockImplementationOnce(async (uri) => { + openFile.mockImplementationOnce(async (uri, { signal }) => { await opening.promise; - activate(addFile(uri)); + 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([ @@ -109,11 +116,44 @@ describe("project search result navigation", () => { false, true, ]); - expect(openFile.mock.calls.map(([uri]) => uri)).toEqual(["slow", "latest"]); + 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(); From 7a8a3bda1bf2796540605985d187c1853a152c8f Mon Sep 17 00:00:00 2001 From: Raunak Raj <71929976+bajrangCoder@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:02:00 +0530 Subject: [PATCH 3/3] fix(search): preserve loader during concurrent file opens --- src/lib/openFile.js | 23 ++++++- tests/unit/openFileCancellation.test.js | 82 ++++++++++++++++++++++++- 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/lib/openFile.js b/src/lib/openFile.js index a19eec1ac..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 @@ -36,6 +38,7 @@ 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; @@ -115,7 +118,7 @@ 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(); @@ -478,10 +481,26 @@ export default async function openFile(file, options = {}) { } catch (error) { if (!signal?.aborted) console.error(error); } finally { - loader.removeTitleLoader(); + releaseTitleLoader?.(); } } +/** Keep the shared indicator visible while any file open still needs it. */ +function acquireTitleLoader(signal) { + if (loadingFileCount++ === 0) loader.showTitleLoader(); + let released = false; + const release = () => { + // 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/tests/unit/openFileCancellation.test.js b/tests/unit/openFileCancellation.test.js index d7735cfc8..4f8cd26a3 100644 --- a/tests/unit/openFileCancellation.test.js +++ b/tests/unit/openFileCancellation.test.js @@ -36,9 +36,20 @@ describe("openFile cancellation", () => { 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"); @@ -56,7 +67,7 @@ describe("openFile cancellation", () => { "dialogs/alert": {}, "dialogs/confirm": {}, "dialogs/loader": { - default: { showTitleLoader: vi.fn(), removeTitleLoader: vi.fn() }, + default: titleLoader, }, "palettes/changeEncoding": {}, "utils/encodings": { decode, detectEncoding }, @@ -140,4 +151,73 @@ describe("openFile cancellation", () => { 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); + }); });