mirror of
https://github.com/abort-retry-ignore/joplock.git
synced 2026-10-19 10:13:51 +00:00
docs: record reconcile log triage + round-trip fragility lessons
Some checks failed
Build and push Joplock image / build-and-push (push) Has been cancelled
Some checks failed
Build and push Joplock image / build-and-push (push) Has been cancelled
Add a 'note shows Edited on open' triage section to Debugging Guidance: the three post-load reconcile log lines and what each means, how to round-trip a stored body offline to find the cause, and the list of known-lossy constructs that are by design and must not be 'fixed' with more markers. Correct the blank-line marker entry, which still described the pre-fix behaviour of rewriting markers 'empty or not' — that is exactly what silently destroyed text typed into a marker. Also document the two Turndown traps that caused real corruption: flanking-whitespace expulsion next to atomic children (NBSP-encoded sentinel, do not simplify) and blockquote soft breaks needing the '>' prefix carried across the restore.
This commit is contained in:
parent
21166ff0e6
commit
c3e0790dda
1 changed files with 26 additions and 1 deletions
|
|
@ -356,11 +356,13 @@ Follow-up (still not done, out of scope):
|
|||
- DOM IDs and class names are part of the editor contract with inline JS (`#cm-host`, `#note-body`, `#tinymce-slot`, `#tinymce-host`, `#editor-toolbar`).
|
||||
- Rendered-mode HTML (TinyMCE content) must remain convertible back to markdown with acceptable fidelity via `tinymceToMarkdown()`/Turndown.
|
||||
- Checkbox, code block, and blank-line handling are easy to regress.
|
||||
- **Blank-line markers between blocks are `<p class="md-blank-line"><br></p>`, NOT bare `<div><br></div>`.** `injectBlankLineBlocks()` (`app/markdownRenderer.js`) emits extra blank lines as empty paragraphs because TinyMCE's schema preserves empty `<p>` natively; a bare `<div><br></div>` got normalised/merged/dropped around block-level images, which swallowed spacing between images after a few markdown⇄render round-trips. The Turndown `blankLine` rule (`public/app.js`, and the preview-path copy in `tests/previewRoundTrip.test.js`) matches `P|DIV.md-blank-line`; in `getTurndown()` both `blankLine` and `emptyP` emit the same `\x00BL\x00` sentinel so precedence is moot. **Two TinyMCE quirks made image spacing collapse anyway (both fixed):** (1) **TinyMCE strips the `<br>`** from the marker on `setContent`, leaving an empty `<p class="md-blank-line"></p>` that Turndown drops — so `tinymceToMarkdown()` pre-normalises any `md-blank-line` paragraph (empty or not) to the `❤BR❤` sentinel shape before Turndown. (2) **Sized/raw-HTML images** (Turndown emits `<img … width=… />` for resized images) are markdown-it *HTML blocks* rendered OUTSIDE any `<p>`; a loose block `<img>` next to markers gets absorbed into an adjacent paragraph by TinyMCE, so `postProcess()` wraps any line that is a lone `<img>` in its own `<p>`. Regression coverage: `tests/appRuntime.test.js` "image spacing …", "sized … survive 6 mode switches", "br-stripped … marker".
|
||||
- **Blank-line markers between blocks are `<p class="md-blank-line"><br></p>`, NOT bare `<div><br></div>`.** `injectBlankLineBlocks()` (`app/markdownRenderer.js`) emits extra blank lines as empty paragraphs because TinyMCE's schema preserves empty `<p>` natively; a bare `<div><br></div>` got normalised/merged/dropped around block-level images, which swallowed spacing between images after a few markdown⇄render round-trips. The Turndown `blankLine` rule (`public/app.js`, and the preview-path copy in `tests/previewRoundTrip.test.js`) matches `P|DIV.md-blank-line`; in `getTurndown()` both `blankLine` and `emptyP` emit the same `\x00BL\x00` sentinel so precedence is moot. **Two TinyMCE quirks made image spacing collapse anyway (both fixed):** (1) **TinyMCE strips the `<br>`** from the marker on `setContent`, leaving an empty `<p class="md-blank-line"></p>` that Turndown drops — so `tinymceToMarkdown()` pre-normalises a **blank** `md-blank-line` paragraph (empty, whitespace/` `-only, or just `<br>`) to the `❤BR❤` sentinel shape before Turndown. **This normalisation MUST stay conditional.** A marker is a real, focusable paragraph in the iframe, so clicking the gap between two blocks puts the caret inside it and typing puts the new text there; rewriting markers unconditionally (the old "empty or not" behaviour) replaced that text with the sentinel and **silently destroyed it** — type a line after a checklist, leave the note, come back, gone. When a marker holds real content, leave the paragraph verbatim: with text present neither the `blankLine` nor the `emptyP` rule matches, so it converts as an ordinary paragraph. Coverage: `tests/appRuntime.test.js` "keeps text typed INTO a blank-line marker (data-loss regression)" + "still collapses a genuinely blank blank-line marker". (2) **Sized/raw-HTML images** (Turndown emits `<img … width=… />` for resized images) are markdown-it *HTML blocks* rendered OUTSIDE any `<p>`; a loose block `<img>` next to markers gets absorbed into an adjacent paragraph by TinyMCE, so `postProcess()` wraps any line that is a lone `<img>` in its own `<p>`. Regression coverage: `tests/appRuntime.test.js` "image spacing …", "sized … survive 6 mode switches", "br-stripped … marker".
|
||||
- The code modal is outside the fragment-swapped editor so it survives swaps; it uses CM6 (`_initCodeModalCM`) which requires `window.CM` (now loaded).
|
||||
- Both markdown mode (CM6) AND rendered mode (TinyMCE) open this same custom full-screen CM6 code modal (`openCodeModal`/`submitCode`) for *editing* the code text/language, NOT TinyMCE's built-in `codesample` dialog. The toolbar uses a custom `jop_code` button; clicking an existing `<pre>` in rendered mode routes through `tinyMCEInsertCodeBlock()` → `openCodeModal()`. On submit in rendered mode, `submitCode()` (TinyMCE branch, `_codeTinyMCE`/`_codeTinyMCEBookmark`) inserts `<pre class="language-x">code</pre>` via `ed.insertContent()` — the `codesample` plugin's `SetContent` handler then highlights it with Prism. Do NOT reintroduce hljs highlighting of rendered-mode blocks (`highlightTinyMCECodeBlocks` was removed); Prism owns rendered-mode coloring. Do not reintroduce `ed.execCommand('mceCodeSample')` (that opens the built-in dialog).
|
||||
- On htmx editor-panel swap, `_cmView` is destroyed in `htmx:afterSwap` and re-mounted by `initEditorPanel()` (via `mountMarkdownEditor`) on `htmx:afterSettle` when the note opens in markdown mode. Keep that destroy/remount ordering intact.
|
||||
- `#tinymce-host` is `position:fixed` and repositioned via `positionTinyMCEHost()`; if it looks detached, check that function and the `#tinymce-slot` rect, not CSS alone.
|
||||
- **Turndown expels "flanking" whitespace, and it gets that wrong next to atomic children.** `flankingWhitespace()` derives an element's edge whitespace from `node.textContent`, which skips `<img>`/`<br>` (they contribute no text). For `<a><img/> Label</a>` it reports *leading* whitespace even though the whitespace is interior to the produced markdown (` Label`), so `replacementForNode()`'s `content.trim()` cannot remove it — yet it is still prepended. The space was therefore **duplicated on every round-trip and grew one character per note open**, corrupting the stored body. `tinymceToMarkdown()` hides such whitespace behind a sentinel (`_protectInlineLeadingSpace` → `_restoreProtectedSpace`) that **encodes the character code**: these runs are frequently NBSP, and restoring a generic `' '` would itself change the body and keep the note permanently dirty. Do not "simplify" that sentinel back to a plain space, and do not narrow its character class to `[ \t]` — Turndown's `edgeWhitespace` uses `\s`, which matches NBSP.
|
||||
- **Soft breaks inside a blockquote must re-apply the `>` prefix.** `> b\n> c` is ONE quoted paragraph with a soft break, so the renderer emits `<blockquote><p>b<br>c</p></blockquote>`. The `❤BR❤` sentinel is restored to `\n` *after* Turndown has prefixed the lines it emitted, so a naive global `split/join` produced `> b\nc` — the second line escaping the blockquote entirely (real content corruption plus a permanent dirty-on-open diff). The restore is line-aware and carries the leading `>` run across the break; keep it that way. Bare `> ` lines also get their insignificant trailing space stripped.
|
||||
|
||||
## Mobile UI Model
|
||||
|
||||
|
|
@ -595,6 +597,29 @@ If a UI action appears broken, check:
|
|||
- Check `#mobile-editor-status`
|
||||
- Check htmx save success/failure handlers and upload progress handlers
|
||||
|
||||
### If a note shows "Edited" (or autosaves) immediately on open
|
||||
|
||||
`_savedHash` is snapshotted in `initEditorPanel()` from the **raw server body**, before CM6/TinyMCE has loaded. 820ms after `setContent`, the reconcile timer in `_setTinyMCEContent()` syncs the editor back to markdown and compares hashes. So **any** non-idempotent markdown⇄HTML round-trip looks like a user edit.
|
||||
|
||||
With `debugLogging` on, the reconcile prints exactly which side it took — this is the fastest way to triage:
|
||||
|
||||
| Log line | Meaning |
|
||||
|---|---|
|
||||
| `post-load reconcile: form clean, nothing to do` | Round-trip is idempotent. Save state is not the problem; look elsewhere. |
|
||||
| `post-load reconcile: load normalisation only, re-baselining hash` | Round-trip is lossy but nobody typed. Hash is re-baselined: no `markEdited`, no save. Expected for notes with constructs HTML cannot represent. |
|
||||
| `post-load reconcile: user typed during window, marking edited` | A real edit landed inside the 800ms quiet window and is being saved. Correct behaviour. |
|
||||
|
||||
If you see `user typed during window` when the note was untouched, the `_tinymceUserTypedSinceLoad` flag is being set spuriously — it must only be wired to `keydown`/`paste`/`cut`/`drop`. Never wire it to `input` or `SetContent`: `editor.setContent()` fires both, which reintroduces the phantom edit. Navigation-only keys (arrows, modifiers) must stay filtered out.
|
||||
|
||||
To find out *why* a note is non-idempotent, round-trip its stored body offline:
|
||||
`renderMarkdown(body)` → `tinymceToMarkdown(html)` → diff against `body`. `tests/appRuntime.test.js` has the harness (`makeTurndownCtx()` + `runWithDeps()`) for running the real `public/app.js` functions under JSDOM.
|
||||
|
||||
Known-lossy constructs that are **by design** and will always re-baseline (do not "fix" them by adding markers):
|
||||
|
||||
- an indented ` ``` ` fence loses its indent (HTML cannot carry fence indentation)
|
||||
- a blank line after an ATX heading is collapsed by `_applyHeadingSpacing()`
|
||||
- encrypted/vault note wrappers strip HTML comments — irrelevant in practice, since locked notes never load plaintext into TinyMCE
|
||||
|
||||
## Verification
|
||||
|
||||
- Run tests: `npm test`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue