Skip to content

fix(export): gate the export menu on the buffer, not the render cache - #441

Merged
PathGao merged 1 commit into
masterfrom
fix/export-gate-reads-the-wrong-buffer
Aug 3, 2026
Merged

fix(export): gate the export menu on the buffer, not the render cache#441
PathGao merged 1 commit into
masterfrom
fix/export-gate-reads-the-wrong-buffer

Conversation

@PathGao

@PathGao PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Two reads of the wrong one of Tab's three same-shaped string fields. #437 documents the distinction and reports both of these as out of scope; this is the fix. Expect a small rebase against it — it touches the field comments, this touches addTab and the menu.

field holds read by
rawContent the live Markdown editor, preview, exports, save_file_content
originalContent that buffer as it last came off / went onto disk isDirty
content the rendered preview HTML {@html sanitizedHtml}

Verified independently before relying on it: grep -rn '\.content\b' src returns exactly two reads of activeTab.contenthtmlContent at MarkdownViewer.svelte:239, which is correct, and the export gate below. That grep is also the answer to "does any other menu item gate on the wrong field": no.

1. The export menu gated on the render cache

{#if currentFile !== '' || (tabManager.activeTab && tabManager.activeTab.content)}

exportAsHtml gates on ctx.rawContent (export.ts:376). content is a cache of a render, refreshed only by the effect at MarkdownViewer.svelte:2363:

if (tab && (tab.isSplit || (isEditing && settings.showToc)) && tab.rawContent !== undefined) {

So in plain edit mode with the TOC closed, nothing re-renders, and for a buffer that has never been rendered content is ''. Add currentFile === '' and both halves of the gate are false while rawContent holds the document. settings.showToc defaults to false and settings.newFileDefaultMode defaults to true, so Ctrl+T, then type is exactly that state. Reachable since #421 let the preview work without saving first; before that an untitled tab had nothing to export either way.

Reproduced in the running app

Vite dev server, Tauri IPC stubbed by a temporary shim (not committed), same tab and same buffer in both, only the gate expression changed between the two:

gate Home menu contents
…activeTab.content (master) 主页 · 新建文件 · 打开文件 · 保存 · 另存为 · 窗口标签 · 将所有窗口合并到此处 · 退出
…activeTab.rawContent (this PR) the same, plus 导出为 HTML and 导出为 PDF

Tab: 无标题 1, dirty, # Hello Markpad in the editor, TOC closed, not split.

The state matrix

scripts/renderedHtmlField.test.ts evaluates the shipping expressions rather than matching their text: the {#if} is lifted from the parsed TitleBar AST, the refresh condition from the parsed MarkdownViewer AST, and exportAsHtml's early return from the TypeScript AST — all three run against the real TabManager. Rewrite any of them however you like and the tests follow. content is brought to what the running app would hold (refreshed only when the shipping condition says so) rather than being set by hand, so the repro is derived, not staged.

Five states, asking "is the export possible but hidden?":

state master this PR
untitled, edited, TOC closed hidden offered
untitled, edited, TOC open offered offered
untitled, split view offered offered
untitled, reading mode offered offered
saved file, edited, TOC closed offered offered

Only the first row changes. The menu is allowed to be looser than the export (a tab with a path whose file has not been read yet still lists the entries, and the export then no-ops — pinned by its own test); being stricter is the defect.

Red on master: 4 of 9 tests. Mutation check on the fixed gate:

gate replaced with tests failing
…activeTab.content (master) 5
…activeTab.originalContent 4
true 2
false 4
activeTab && activeTab.rawContent (drops the path half) 1

2. addTab seeded the HTML field from a Markdown-shaped argument

addTab(path, content = '') assigned its argument to content, rawContent and originalContent alike (tabs.svelte.ts:342). That was coherent when the three were one field.

Caller survey, done first:

  • In the app, two callers, both pass ''documentSession.svelte.ts:254 explicitly, MarkdownViewer.svelte:1434 by default. Nothing shipped broken.
  • In scripts/, ~25 callers pass real Markdown (tabPathIdentity, reopenDirtyDocument, foldStatePerDocument, externalChangeReload) and depend on it landing in rawContent / originalContent. That is what says the argument is the buffer, not the render.
  • No windowSession / documentSession / tab-transfer path constructs through it: window restore goes through restoreState, cross-window arrival through insertTransferredTabbuildTransferredTab.

Chosen: rename the parameter to rawContent and stop seeding content. Rename alone leaves the wrong assignment; dropping the seed alone leaves a call site that reads as if content were the thing being passed. A new shape for the two buffers is a bigger change than this defect justifies, and renaming contentrenderedHtml across the codebase is deliberately not attempted here.

Nothing depends on the seeded value: createFileHistory(path, _content) ignores it, the only reader is htmlContentsanitizedHtml{@html}, and addNewTab, restoreState, addHomeTab and buildTransferredTab all already start content at '' and let the first render fill it. This makes addTab the fifth, not the exception — asserted directly by every Tab construction site starts the rendered-HTML field empty.

On the security framing, plainly: content reaches the DOM through MarkdownViewer.svelte:250, sanitizedHtml = $derived(sanitizeMarkdownHtml(htmlContent)), and that is what {@html} at :3450 injects. The sanitizer is between this field and the sink in every case, and both live callers pass ''. This is a correctness fix — the signature permitted a value the field does not mean — not the closing of a hole.

Mutation check on addTab, whole suite:

mutation tests failing
content: rawContent (the old seed) 2
rawContent: '' 1
originalContent: '' 4

Verification

npm test 537/537 (528 before, +9). npm run check 632 files / 0 errors / 0 warnings. npm run build clean; the new template comment is stripped from the production bundle.

Not covered

  • The PDF path's own rendering. syncPreviewForPrint (MarkdownViewer.svelte:1957) re-renders before printing, which is why Export PDF is correct to be enabled in the newly-reachable state too. Not exercised here — the tests establish which branch the menu takes, not what the PDF contains.
  • Whether the entries are visible given CSS, and the macOS native menu, which has no export items.
  • syncPreviewForPrint's counterpart for HTML. exportAsHtml renders from rawContent directly, so it needs none, but that was read rather than exercised.
  • The naming itself. content still means rendered HTML and rawContent still means the document. That rename touches many files and belongs in its own discussion.
  • Nothing in docs(tabs): say what content, rawContent and originalContent each hold #437's added comments is made wrong by this change; the content line becomes more true, and its addTab line numbers shift.

🤖 Generated with Claude Code

`Tab` carries three same-shaped strings. `rawContent` is the Markdown, and
it is what the editor edits, what reaches disk, and what the exports read.
`content` is the rendered preview HTML, injected via `{@html}`. Two places
read the wrong one.

1. TitleBar.svelte gated Export as HTML / Export as PDF on
   `tabManager.activeTab.content`, while `exportAsHtml` gates on
   `ctx.rawContent`. `content` is only refreshed while the preview is on
   screen — `tab.isSplit || (isEditing && settings.showToc)` — so for an
   untitled buffer being edited with the TOC closed it is still `''` while
   `rawContent` holds the user's text. `showToc` and `newFileDefaultMode`
   default such that Ctrl+T then typing is exactly that state, and the menu
   hid two commands that would have produced a file. Reachable since #421
   let the preview work without saving first.

2. `addTab(path, content = '')` assigned its argument to `content`,
   `rawContent` and `originalContent` alike — coherent when the three were
   one field, but it let Markdown into the field that is injected as HTML.
   Both callers in the app pass `''`, so nothing shipped broken; the value
   is sanitized at the sink either way. The parameter is now `rawContent`
   and `content` starts empty, as at every other Tab construction site.

scripts/renderedHtmlField.test.ts evaluates the real gate, the real
refresh condition and `exportAsHtml`'s real precondition against the real
TabManager. Four of its nine tests are red on master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao merged commit 25e0048 into master Aug 3, 2026
4 checks passed
@PathGao
PathGao deleted the fix/export-gate-reads-the-wrong-buffer branch August 3, 2026 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant