fix(browse): save-button gate reads canSave at click time

The markdown editor's save handlers (markDirty, save(), convertBtns
intercept) referenced a bare identifier `writable` that never existed
in their scope — the captured variable was named `writableMode`. JS
silently evaluates `!undefined` to true, so saveBtn.disabled stayed
true forever and Ctrl-S was a no-op. The download-as-* intercept
treated every dirty file as read-only and offered the "save a copy
elsewhere" toast.

YAML editor had the matching-name pattern (`writable` defined and
referenced) so the symptom was hidden, but the same stale-closure
shape: capture once at mount, never re-read when the underlying tree
node's writable bit changed.

Fix both: gating logic reads canSave(node) fresh at every click, not
from a closure. Mount-time captures stay for initial UI shape
(read-only banner, CodeMirror readOnly:'nocursor') where the decision
is correct at the moment it's applied.

Codify the pattern in AGENTS.md § "JS module pattern":
no bundler + no reactivity layer ⇒ closures don't refresh ⇒ read
fresh in handlers, never cache.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
ZDDC 2026-05-18 17:08:47 -05:00
parent c240bf30a5
commit 85e6eb152c
3 changed files with 32 additions and 4 deletions

View file

@ -165,6 +165,27 @@ All JS is vanilla, no bundlers. Files are IIFEs, registered on `window.app.modul
**Exception:** archive uses plain globals (`APP_STATE`, top-level functions) — not the IIFE/modules pattern. **Exception:** archive uses plain globals (`APP_STATE`, top-level functions) — not the IIFE/modules pattern.
**State values used inside event handlers must be read fresh from the source of truth, never captured at mount.** No bundler, no reactivity layer — closures don't get refreshed when the underlying state mutates. Cache the *node*, re-read the *bit* at click time:
```javascript
// Wrong — `writable` is whatever canSave returned at mount, even if
// the tree node's bit later flips to true (e.g. admin toggle reload
// re-fetched the listing).
var writable = canSave(node);
saveBtn.addEventListener('click', function () {
if (!writable) return; // STALE
});
// Right — re-read at click time.
saveBtn.addEventListener('click', function () {
if (!canSave(node)) return; // current
});
```
It's fine to use mount-time captures for *initial UI shape* (read-only banner, CodeMirror `readOnly:'nocursor'`, etc.) — those decisions are correct at the moment they're applied. The rule is specifically about gating logic in handlers that fire *after* mount.
This pattern bit twice in the markdown + YAML editors before we caught it: the typo `writable` (undefined) vs `writableMode` (captured) made every save click a no-op. Re-reading the source of truth would have surfaced the bug at click time instead of silently disabling save.
## ZDDC filename parsers ## ZDDC filename parsers
All parsing/formatting goes through `shared/zddc.js`, exposed as `window.zddc`. Tools call it directly — no per-tool wrappers. All parsing/formatting goes through `shared/zddc.js`, exposed as `window.zddc`. Tools call it directly — no per-tool wrappers.

View file

@ -664,7 +664,10 @@
// ── Change tracking + auto-rerender ──────────────────────────────── // ── Change tracking + auto-rerender ────────────────────────────────
function markDirty(isDirty) { function markDirty(isDirty) {
currentInstance.dirty = isDirty; currentInstance.dirty = isDirty;
saveBtn.disabled = !isDirty || !writable; // 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.
saveBtn.disabled = !isDirty || !canSave(node);
dirtyEl.textContent = isDirty ? '● modified' : ''; dirtyEl.textContent = isDirty ? '● modified' : '';
} }
@ -685,7 +688,7 @@
// ── Save ─────────────────────────────────────────────────────────── // ── Save ───────────────────────────────────────────────────────────
async function save() { async function save() {
if (!currentInstance.dirty || !writable) return; if (!currentInstance.dirty || !canSave(node)) return;
var content = assembleContent(fmTextarea.value, editor.getMarkdown()); var content = assembleContent(fmTextarea.value, editor.getMarkdown());
try { try {
statusEl.textContent = 'Saving…'; statusEl.textContent = 'Saving…';
@ -731,7 +734,7 @@
} }
// Dirty: intercept, save, retry. // Dirty: intercept, save, retry.
e.preventDefault(); e.preventDefault();
if (!writable) { if (!canSave(node)) {
if (window.zddc && window.zddc.toast) { if (window.zddc && window.zddc.toast) {
window.zddc.toast( window.zddc.toast(
'This source is read-only — save a copy elsewhere first.', 'This source is read-only — save a copy elsewhere first.',

View file

@ -504,7 +504,7 @@
var initialHash = await hashContent(text); var initialHash = await hashContent(text);
function markDirty(isDirty) { function markDirty(isDirty) {
saveBtn.disabled = !isDirty || !writable; saveBtn.disabled = !isDirty || !canSave(node);
dirtyEl.textContent = isDirty ? '● modified' : ''; dirtyEl.textContent = isDirty ? '● modified' : '';
} }
@ -515,6 +515,10 @@
async function save() { async function save() {
if (saveBtn.disabled) return; if (saveBtn.disabled) return;
// Re-check authority at click time, not via the mount-time
// `writable` capture — the listing may have re-evaluated
// (e.g. user toggled admin mode without a hard reload).
if (!canSave(node)) return;
var content = editor.getValue(); var content = editor.getValue();
try { try {
statusEl.textContent = 'Saving…'; statusEl.textContent = 'Saving…';