fix(preview): resolve relative images of imported documents correctly - #313
Merged
petertzy merged 5 commits intoOct 2, 2026
Merged
Conversation
openFile()'s conversion branch was the only refreshPreview call site that did not pass a base directory, so the fallback read activeTab.filePath from the closure created before setActiveTabId -- i.e. the previously active tab. Every other call site (content change, already-open tab, plain read, openTextAsTab) passes the path explicitly. The backend resolves relative image sources with os.path.abspath(os.path.join(base_dir, src)), so the wrong folder yields a broken file:// URL. If the previous tab was untitled, base_dir was undefined and the relative src resolved against the app origin in the sandboxed iframe. A converted tab cannot simply use filePath: it is created with filePath null on purpose so Save asks for a destination instead of overwriting the source .html, yet its image paths still belong to that file's folder. Add Tab.previewBaseDir, defaulting to the parent of filePath and set to the source folder for converted documents, and consult it in refreshPreview's fallback so the right directory survives later re-renders too (dark mode, font size, every keystroke). Export/docx export paths derive base_dir the same way and share the latent issue, but that is a separate behaviour change; this stays on the preview. Adds preview-base-dir.mjs plus 9 cases in tests/preview-base-dir.test.mjs; verified 3 fail without the previewBaseDir branch.
harsh-thakkar7
force-pushed
the
fix/openfile-converted-base-dir
branch
from
October 2, 2026 09:01
bf77e52 to
e35c92b
Compare
`next build` rejects `previewTab.previewBaseDir` on useEditor.ts:270 because the `PreviewTab` JSDoc typedef in tabLifecycle.mjs lists only the fields the helper itself reads. Tabs are handed back whole, so a field the *caller* consumes has to be declared even though resolveTabClose never touches it. Declare it optional (plain Markdown tabs have none) and note the pass-through rule in the typedef's doc comment so the next field does not repeat the miss. Adds a regression test covering the case that broke: closing a plain tab makes a converted (filePath === null) tab active again, and its base directory has to survive the hand-off or every relative image falls back to the app origin.
Owner
|
Approved after follow-up fixes. The original change correctly preserves the source directory for imported HTML/PDF/DOCX documents and passes it explicitly during the initial preview render. I also fixed two related lifecycle cases: switching to a converted tab after closing another tab, and saving a converted document to a new path. These cases now preserve or update the preview base directory correctly. Frontend tests pass (48/48) and lint passes. |
harsh-thakkar7
added a commit
to harsh-thakkar7/markdown-reader
that referenced
this pull request
Oct 2, 2026
`saveFile` captured `activeTab.content`, awaited `Files.write`, then applied a patch built from that stale snapshot. `updateTab` merged the patch into whatever the tab held *by then*, so `dirty: false` was stamped onto a buffer that had advanced during the round trip — the title bar claimed "saved" while the edits were still only in memory. Settle the tab from the buffer observed after the write, and only clear `dirty` when it still holds exactly what reached disk. Two related changes fall out of this: - The snapshot patch is replaced by a functional `setTabs`, so the comparison happens against live state. That removes `updateTab`'s last two call sites, making it dead: it is dropped rather than left as an unused helper. - `previewBaseDir` is re-pointed at the destination folder when a save moves the document, preserving the behaviour from petertzy#313. It moves inside `settleSavedTab` instead of being inlined at the two call sites, so the rule stays covered by the Node suite instead of sitting untested in TypeScript. frontend/tests/save-settle.test.mjs covers the compare-after-write rule and the base-directory move; 6 of 6 mutants killed, including "previewBaseDir only updated when clean" and "previewBaseDir never updated".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Opening an imported document (
.html,.pdf,.docx) rendered its relativeimages against the wrong folder, so they were broken in the preview.
Root cause
openFile()'s conversion branch was the onlyrefreshPreviewcall site that didnot pass a base directory:
The fallback inside
refreshPreviewthen usedactiveTab.filePath. But theclosure belongs to the render before the tab switch, and
setActiveTabIdhas not re-rendered yet — so it read the previously active tab's path.
Every other call site passes the path explicitly:
handleContentChangeopenFile(already-open tab)openFile(plain read)openTextAsTabopenFile(conversion)Why it cannot just use
filePathA converted tab is created with
filePath: null, on purpose — so Save asks for adestination instead of overwriting the source
.html. But its image paths arerelative to where that file lived.
The backend resolves them with
fix_image_paths():So the wrong
base_dirproduces concretely wrongfile://URLs. Verifiedend-to-end from the real HTML conversion:
If the previously active tab was untitled,
filePathisnull, sobase_dirwas
undefinedand the relativesrcresolved against the app origin insidethe sandboxed preview iframe — also broken.
Repro: keep
/elsewhere/old.mdactive, then open/work/notes/report.htmlwhich referenceschart.pngsitting beside it. Everyrelative image renders broken.
Changes
frontend/src/lib/preview-base-dir.mjs(new) —parentDirOf()andresolvePreviewBaseDir(), holding the rule in a plain ESM module so the Noderegression suite can exercise the exact code the app runs (same pattern as
http-timeout.mjs).frontend/src/hooks/useEditor.tsTabgainspreviewBaseDir, defaulting to the parent offilePath. For aconverted tab it is set to the folder the source file came from.
refreshPreview's fallback consultspreviewBaseDirbeforefilePath, so thecorrect directory survives later re-renders too — dark-mode toggles,
font-size changes and every keystroke all re-render through the fallback.
before the tab switch has taken effect.
Behaviour for plain Markdown tabs is unchanged:
previewBaseDiris justparentDirOf(filePath).I deliberately left the DOCX/PDF export paths (
useFileIO.ts,useActions.ts) alone. They derivebase_dirthe same way and so have the samelatent issue for converted documents, but that is a separate behaviour change and
this PR stays on the preview.
Testing
frontend/tests/preview-base-dir.test.mjs, 9 cases exercising the real helper:parentDirOffor POSIX and Windows separators, root-level, and no-directorypreviewBaseDirwins when both are presentnull, and missing arguments returnundefinedVerified 3 fail when the
previewBaseDirbranch is removed fromresolvePreviewBaseDir.Verification
npm test— 32 passed (was 23 onmain; this PR adds 9)npm run lint— 0 errors, 0 warnings on the changed filenpm run build— cleanpytest— 195 passed, unchanged