fix(browse): editor lifecycle — dispose on switch, guard unsaved edits, kill leaks

The markdown/YAML preview editors were never disposed when switching to a
non-editor file: dispose() was only called from inside the same plugin's
render(), so md→PDF/image/YAML overwrote the pane via innerHTML and leaked
the Toast UI instance, its DOM, and document-level resizer drag listeners.
Unsaved edits were also discarded silently on any file switch (including
arrow-key auto-preview), and debounced change handlers could resolve after
an editor was disposed and write the wrong file's dirty/hash state.

preview.js now owns editor lifecycle centrally in renderInline:
- disposeEditors() up front before replacing the pane (fixes the leak for
  every md/yaml → anything switch).
- dirty guard: deliberate switches (click/Enter/menu) confirm before
  discarding; auto previews (keyboard cursor walking the tree, opts.auto)
  leave the dirty editor in place rather than nagging per keystroke;
  re-selecting the file already being edited is a no-op.
- a renderSeq token bails late-arriving loads so a slow file can't paint
  stale content into the pane after a newer selection.
- clearPreview() exposed and used by rescope (events.js) and popstate
  (app.js) so those resets dispose the editor instead of leaking it.
- beforeunload warns when an editor is dirty at page exit.

preview-markdown.js: per-mount AbortController wired into the resizer
document listeners so dispose() detaches them even mid-drag; debounced
change/save/convert handlers guard `currentInstance !== instance` so a
disposed editor's callbacks can't corrupt the active file; expose
isDirty()/currentNode().

preview-yaml.js: track dirty/node state, guard the change handler the same
way, expose dispose()/isDirty()/currentNode().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
ZDDC 2026-06-03 14:46:31 -05:00
parent 2f211d748f
commit cfb2fab401
5 changed files with 184 additions and 25 deletions

View file

@ -139,8 +139,14 @@
window.app.state.lastPreviewedNodeId = null;
tree.setRoot(es);
tree.render();
// Route through clearPreview so a live editor is disposed
// (not leaked) when back/forward swaps scope.
var pmod = window.app.modules.preview;
if (pmod && pmod.clearPreview) pmod.clearPreview();
else {
var previewBody = document.getElementById('previewBody');
if (previewBody) previewBody.innerHTML = '';
}
var previewTitle = document.getElementById('previewTitle');
if (previewTitle) previewTitle.textContent = 'No file selected';
// Reapply view mode for the new URL (incoming/ → grid, etc).

View file

@ -449,7 +449,10 @@
// selection-only; their preview is "expand to see inside".
if (nextNode && !nextNode.isDir && !nextNode.isZip
&& previewModule) {
previewModule.showFilePreview(nextNode);
// auto:true — keyboard cursor walking the tree. If an
// editor has unsaved edits, the preview module leaves it
// in place rather than prompting on every keystroke.
previewModule.showFilePreview(nextNode, { auto: true });
state.lastPreviewedNodeId = nextId;
}
// Scroll the now-selected row into view.
@ -1408,9 +1411,14 @@
tree.setRoot(entries);
tree.render();
// Reset the preview pane so the user sees an "empty selection"
// state at the new scope instead of the previous file.
// state at the new scope instead of the previous file. Route
// through clearPreview so a live editor is disposed (not leaked).
var pmod = previewMod();
if (pmod && pmod.clearPreview) pmod.clearPreview();
else {
var previewBody = document.getElementById('previewBody');
if (previewBody) previewBody.innerHTML = '';
}
var previewTitle = document.getElementById('previewTitle');
if (previewTitle) previewTitle.textContent = 'No file selected';
var previewMeta = document.getElementById('previewMeta');

View file

@ -64,12 +64,30 @@
}
function dispose() {
if (currentInstance && currentInstance.editor) {
if (currentInstance) {
// Tear down the document-level resizer drag listeners (added
// lazily on mousedown). They're normally removed on mouseup,
// but a dispose mid-drag — or any switch away — would otherwise
// strand them pointing at the dead shell. The AbortController
// removes whatever is still attached in one call.
if (currentInstance.ac) {
try { currentInstance.ac.abort(); } catch (_) { /* ignore */ }
}
if (currentInstance.editor) {
try { currentInstance.editor.destroy(); } catch (_) { /* ignore */ }
}
}
currentInstance = null;
}
function isDirty() {
return !!(currentInstance && currentInstance.dirty);
}
function currentNode() {
return currentInstance ? currentInstance.node : null;
}
// ── Front matter ────────────────────────────────────────────────────────
// Lightweight YAML front-matter parser. Same envelope as mdedit's:
// `---\n…\n---\n`, key:value lines, simple `[a, b, c]` arrays.
@ -564,15 +582,20 @@
}));
}
currentInstance = {
// One AbortController per mount — wired into the document-level
// resizer listeners below so dispose() can detach them all at once.
var ac = new AbortController();
var instance = {
editor: editor,
container: container,
dirty: false,
node: node,
hash: initialHash,
tocEl: tocBody,
fmEl: fmTextarea
fmEl: fmTextarea,
ac: ac
};
currentInstance = instance;
if (!writableMode) {
saveBtn.disabled = true;
@ -609,8 +632,8 @@
resizer.classList.add('is-dragging');
startX = e.clientX;
startW = sidebar.getBoundingClientRect().width;
document.addEventListener('mousemove', onMove);
document.addEventListener('mouseup', onUp);
document.addEventListener('mousemove', onMove, { signal: ac.signal });
document.addEventListener('mouseup', onUp, { signal: ac.signal });
e.preventDefault();
});
resizer.addEventListener('keydown', function (e) {
@ -654,8 +677,8 @@
fmResizer.classList.add('is-dragging');
startY = e.clientY;
startH = fmSection.getBoundingClientRect().height;
document.addEventListener('mousemove', onMove);
document.addEventListener('mouseup', onUp);
document.addEventListener('mousemove', onMove, { signal: ac.signal });
document.addEventListener('mouseup', onUp, { signal: ac.signal });
e.preventDefault();
});
fmResizer.addEventListener('keydown', function (e) {
@ -670,7 +693,8 @@
// ── Change tracking + auto-rerender ────────────────────────────────
function markDirty(isDirty) {
currentInstance.dirty = isDirty;
if (currentInstance !== instance) return; // editor replaced
instance.dirty = isDirty;
// Re-read canSave at every transition, not via a closure-captured
// value, so the gate reflects current write authority — see the
// matching pattern in preview-yaml.js.
@ -678,29 +702,40 @@
dirtyEl.textContent = isDirty ? '● modified' : '';
}
// The debounced handlers can resolve AFTER this editor was disposed
// and a new file mounted (the timer + the await both outlive the
// switch). Bail when we're no longer the active instance so we never
// call into a destroyed Toast UI editor or write the wrong file's
// dirty/hash state.
var onChange = debounce(async function () {
if (currentInstance !== instance) return;
var body = editor.getMarkdown();
var h = await hashContent(assembleContent(fmTextarea.value, body));
markDirty(h !== currentInstance.hash);
if (currentInstance !== instance) return;
markDirty(h !== instance.hash);
renderToc(tocBody, body, editor);
}, 250);
editor.on('change', onChange);
var onFmChange = debounce(async function () {
if (currentInstance !== instance) return;
var body = editor.getMarkdown();
var h = await hashContent(assembleContent(fmTextarea.value, body));
markDirty(h !== currentInstance.hash);
if (currentInstance !== instance) return;
markDirty(h !== instance.hash);
}, 250);
fmTextarea.addEventListener('input', onFmChange);
// ── Save ───────────────────────────────────────────────────────────
async function save() {
if (!currentInstance.dirty || !canSave(node)) return;
if (currentInstance !== instance) return;
if (!instance.dirty || !canSave(node)) return;
var content = assembleContent(fmTextarea.value, editor.getMarkdown());
try {
statusEl.textContent = 'Saving…';
await saveContent(node, content);
currentInstance.hash = await hashContent(content);
if (currentInstance !== instance) return; // switched away mid-save
instance.hash = await hashContent(content);
markDirty(false);
statusEl.textContent = 'Saved ' + new Date().toLocaleTimeString();
if (window.zddc && window.zddc.toast) {
@ -732,7 +767,7 @@
convertBtns.forEach(function (a) {
a.addEventListener('click', async function (e) {
var fmt = a.dataset.fmt;
if (!currentInstance.dirty) {
if (!instance.dirty) {
// Clean — let the browser handle the click. The
// server's response (DOCX/HTML/PDF bytes, 422,
// 503, etc.) lands in whatever target the user
@ -751,7 +786,7 @@
}
statusEl.textContent = 'Saving before download…';
try { await save(); } catch (_) { /* save() surfaces its own error */ }
if (currentInstance.dirty) return; // save failed; toast already shown
if (currentInstance !== instance || instance.dirty) return; // save failed / switched away
statusEl.textContent = 'Downloading ' + fmt.toUpperCase() + '…';
// Re-trigger the click. dirty=false now so the handler
// exits early on the second pass and the browser
@ -763,6 +798,8 @@
window.app.modules.markdown = {
render: render,
dispose: dispose
dispose: dispose,
isDirty: isDirty,
currentNode: currentNode
};
})();

View file

@ -378,12 +378,24 @@
// ── Mount ───────────────────────────────────────────────────────────────
var currentEditor = null;
var currentDirty = false;
var currentNodeRef = null;
function dispose() {
// CM doesn't have an explicit destroy(); GC handles it once
// the host element is removed. Clear our reference so a stale
// editor doesn't keep handlers alive.
currentEditor = null;
currentDirty = false;
currentNodeRef = null;
}
function isDirty() {
return currentDirty;
}
function currentNode() {
return currentNodeRef;
}
async function render(node, container, ctx) {
@ -499,6 +511,8 @@
// Force an initial lint pass now that _zddcNode is set.
editor.performLint();
currentEditor = editor;
currentNodeRef = node;
currentDirty = false;
if (!writable) {
saveBtn.disabled = true;
@ -514,12 +528,16 @@
var initialHash = await hashContent(text);
function markDirty(isDirty) {
if (currentEditor !== editor) return; // editor replaced
currentDirty = isDirty;
saveBtn.disabled = !isDirty || !canSave(node);
dirtyEl.textContent = isDirty ? '● modified' : '';
}
editor.on('change', async function () {
if (currentEditor !== editor) return; // switched away
var h = await hashContent(editor.getValue());
if (currentEditor !== editor) return; // replaced during await
markDirty(h !== initialHash);
});
@ -564,6 +582,9 @@
window.app.modules.yamledit = {
handles: handles,
render: render
render: render,
dispose: dispose,
isDirty: isDirty,
currentNode: currentNode
};
})();

View file

@ -76,8 +76,62 @@
return { url: URL.createObjectURL(blob), fromServer: false };
}
// ── Editor lifecycle helpers ─────────────────────────────────────────────
// The markdown and YAML plugins each mount a long-lived editor into the
// preview pane. Switching files (or clearing the pane) must dispose the
// live editor first — otherwise the Toast UI instance, its DOM, and its
// document-level resizer listeners leak when we overwrite the container.
function editorModules() {
var m = window.app.modules;
return [m.markdown, m.yamledit].filter(Boolean);
}
function disposeEditors() {
editorModules().forEach(function (mod) {
if (mod.dispose) { try { mod.dispose(); } catch (_) { /* ignore */ } }
});
}
// The editor module (if any) holding unsaved edits, else null.
function dirtyEditor() {
var mods = editorModules();
for (var i = 0; i < mods.length; i++) {
if (mods[i].isDirty && mods[i].isDirty()) return mods[i];
}
return null;
}
function samePreviewNode(a, b) {
if (!a || !b) return false;
if (a === b) return true;
if (a.url && b.url) return a.url === b.url;
return a.name === b.name && a.parentId === b.parentId;
}
// Tear down any live editor and blank the pane. Used by callers that
// reset the preview directly (rescope, popstate) so they don't leak the
// editor or strand its dirty state.
function clearPreview() {
disposeEditors();
var container = document.getElementById('previewBody');
if (container) container.innerHTML = '';
}
// Warn before a full page unload (reload / close / external nav) drops
// unsaved editor changes. SPA-internal switches are guarded in
// renderInline; this catches the browser-level exit.
window.addEventListener('beforeunload', function (e) {
if (dirtyEditor()) { e.preventDefault(); e.returnValue = ''; }
});
// ── Inline rendering ────────────────────────────────────────────────────
// Bumped on every renderInline entry; a render that loses the race
// (a newer selection started while its bytes were in flight) bails
// before writing stale content into the shared pane.
var renderSeq = 0;
function renderEmpty(container, msg) {
container.innerHTML = '<div class="preview-empty">' + escapeHtml(msg) + '</div>';
}
@ -87,13 +141,37 @@
+ escapeHtml(msg) + '</div>';
}
async function renderInline(node) {
async function renderInline(node, opts) {
opts = opts || {};
var container = document.getElementById('previewBody');
var titleEl = document.getElementById('previewTitle');
var metaEl = document.getElementById('previewMeta');
var popoutBtn = document.getElementById('previewPopout');
if (!container) return;
// Guard unsaved editor edits before we tear the editor down.
var dm = dirtyEditor();
if (dm) {
var cur = dm.currentNode ? dm.currentNode() : null;
if (samePreviewNode(cur, node)) {
// Re-selecting the file we're already editing — don't reload
// and clobber the in-progress edits.
return;
}
if (opts.auto) {
// Keyboard/auto preview (cursor walking the tree): leave the
// dirty editor in place rather than prompting on every key.
return;
}
var label = cur ? cur.name : 'this file';
if (!window.confirm('Discard unsaved changes to ' + label + '?')) return;
}
// Safe to replace the pane now: dispose any live editor so its
// instance + document-level listeners don't leak.
disposeEditors();
var seq = ++renderSeq;
if (titleEl) titleEl.textContent = node.name;
if (metaEl) {
var meta = [];
@ -134,6 +212,7 @@
if (ext === 'pdf' || ext === 'html' || ext === 'htm') {
try {
var info = await getBlobUrl(node);
if (seq !== renderSeq) return;
var sandbox = (ext === 'pdf') ? '' : ' sandbox="allow-same-origin allow-popups allow-popups-to-escape-sandbox"';
container.innerHTML = '<iframe class="preview-iframe" src="' + escapeHtml(info.url) + '"' + sandbox + '></iframe>';
} catch (e) {
@ -146,6 +225,7 @@
if (preview && preview.isImage(ext) && !preview.isTiff(ext)) {
try {
var imgInfo = await getBlobUrl(node);
if (seq !== renderSeq) return;
container.innerHTML = '<img class="preview-image" alt="' + escapeHtml(node.name)
+ '" src="' + escapeHtml(imgInfo.url) + '">';
} catch (e) {
@ -157,6 +237,7 @@
if (preview && preview.isTiff(ext)) {
try {
var tiffBuf = await getArrayBuffer(node);
if (seq !== renderSeq) return;
container.innerHTML = '';
await preview.renderTiff(document, container, tiffBuf, { fileName: node.name });
} catch (e) {
@ -168,6 +249,7 @@
if (preview && preview.isZip(ext)) {
try {
var zipBuf = await getArrayBuffer(node);
if (seq !== renderSeq) return;
container.innerHTML = '';
await preview.renderZipListing(document, container, zipBuf, { fileName: node.name });
} catch (e) {
@ -182,6 +264,7 @@
if (preview && preview.isOffice(ext)) {
try {
var officeBuf = await getArrayBuffer(node);
if (seq !== renderSeq) return;
container.innerHTML = '';
if (ext === 'docx') {
await preview.renderDocx(document, container, officeBuf, { fileName: node.name });
@ -197,6 +280,7 @@
if (preview && preview.isText(ext)) {
try {
var txtBuf = await getArrayBuffer(node);
if (seq !== renderSeq) return;
var text = new TextDecoder('utf-8', { fatal: false }).decode(txtBuf);
var MAX = 200000;
if (text.length > MAX) {
@ -217,6 +301,7 @@
// Unknown type — offer a download link.
try {
var fallbackInfo = await getBlobUrl(node);
if (seq !== renderSeq) return;
container.innerHTML =
'<div class="preview-empty">'
+ 'No inline preview for <code>.' + escapeHtml(ext) + '</code>. '
@ -358,11 +443,13 @@
if (node.isDir) return;
opts = opts || {};
if (opts.popup) return renderInPopup(node);
return renderInline(node);
return renderInline(node, opts);
}
window.app.modules.preview = {
showFilePreview: showFilePreview,
// Tear down any live editor + blank the pane (rescope / popstate).
clearPreview: clearPreview,
// Expose for the markdown plugin so it can read file bytes.
getArrayBuffer: getArrayBuffer
};