Repository navigation
Fix: read_dom_changes/try_patch don't detect or clean up text typed next to a non-text atom in an otherwise-empty block - #19
Open
gwing33 wants to merge 4 commits into
Conversation
…mpty block read_dom_changes()/find_empty_block_text() only handled a block with *zero* tracked children (e.g. typing into the paragraph created by pressing Enter). A block whose only tracked child is a non-text atom (e.g. an inline checkbox) fell through the same gap: no ViewDesc::Text exists there either, for collect_text_changes to diff against, or for the children.is_empty() check to fire, so text typed right next to the atom was a real DOM node the model never learned about -- invisible until some unrelated re-render exposed it as an orphan. Generalize the check from "no tracked children" to "no tracked *text* children", and compute the insertion position from the size of any tracked (atom) children that precede it, instead of always assuming content_start. try_patch() had the mirror-image gap on the write side: it only cleared foreign DOM nodes (our trailing <br>, or previously-untracked typed text) when the old ViewDesc children list was fully empty, so a block with an atom-only history never got that cleanup and would grow a duplicate stray text node on every subsequent keystroke. Replace the "clear everything" step with a new strip_untracked_text() that removes only text nodes and our trailing-break marker, leaving atom elements (e.g. the checkbox's <input>) untouched in place, and widen its guard the same way. New test: atom_text_sync.rs, mirroring empty_block.rs's existing pattern with a minimal atom-bearing schema.
…apped `wire_events`'s keydown handler only ever treated `"Enter"`, `"Backspace"`, and `"Delete"` as unconditionally structural (always `prevent_default()`, regardless of whether a bound keymap command claimed the key). Every other printable key was free to reach native contenteditable handling whenever the current selection was a whole atom (a real `Selection::Node`, or its degraded live-DOM `Selection::Text` shape, or a bare caret one level inside an atom's own content) — letting the browser's own "replace selected/typed-into content" behavior run over it. For an atom with a real DOM subtree (not a true leaf), this can produce genuinely corrupt markup that neither this library nor any consumer's `read_dom_changes` has tracked meaning for. `NodeType::is_atom()` already existed but was never actually consulted anywhere in the DOM/Leptos adapters. Add `selection_touches_an_atom`, a fully generic helper (no consumer-schema knowledge — just `NodeType::is_atom()`), and fold it into the keydown handler's own `structural` check so any key is blocked the same way Enter/Backspace/ Delete already were, whenever the live selection is a whole atom. Confirmed live by a consuming app: selecting an atom-like decorator node (e.g. via a click) and then typing a character or pressing Space no longer reaches native contenteditable handling at all.
`apply_view_action` only ever pushed a `ViewAction`'s result into the reactive `state` signal, never synchronously into the live view/DOM the way the `keydown` handler's own `next: Option<EditorState>` handling already does. The actual DOM sync was left entirely to the reactive `Effect` in `TainoEditor`'s own body, which runs as a separate, async microtask. This is a real, confirmed-live race: a pointer-driven correction (e.g. a `ViewPlugin` fixing up a `mouseup` that landed the browser's own native caret somewhere wrong) could be outrun by the very next synchronous keystroke, which would then act against the DOM's still-stale caret position for one more character before the async effect caught up. Reproduced directly: typing immediately after such a correction landed each subsequent character progressively one position further behind, visibly scrambling the typed word. `apply_view_action` now takes the same `runtime` handle the `keydown` closure already has, and mirrors its synchronous push exactly — for both `ViewAction::Select` and `ViewAction::Command` — before also updating the reactive signal.
`read_dom_changes` detects a typed edit by diffing the OLD model text against the NEW live DOM text as two plain strings (`find_diff`'s longest-common-prefix + longest-common-suffix), with no awareness of where the browser's own caret actually is. This is provably ambiguous whenever the inserted text shares characters with its own immediate neighborhood. Example: typing "typed" into "plain paragraph", by the 3rd keystroke the model has "typlain paragraph" and the live DOM has "typplain paragraph" (the typed "p" landed right before an already-existing "p" two characters later, from "...lain..."). "Insert p at 2" and "insert p at 3" produce the IDENTICAL resulting text — a genuine ambiguity, not a fluke — and the unanchored longest-common-prefix match always resolves it by greedily extending the prefix as far as equality allows, picking the later position, which is wrong here. The text still renders correctly either way; what silently breaks is the model's own notion of where the caret ended up, compounding into a visibly scrambled word after a few such keystrokes (confirmed live: "typed" re-rendered as "tyedp"). Add `find_diff_anchored`, layered in front of the original `find_diff` (kept, unmodified, as the fallback): when the text grew (a plain insertion, the overwhelmingly common typing case) and the browser's own current collapsed caret is known, the edit's boundaries are derived directly from that caret — which always sits exactly at the end of what was just typed — instead of guessed at via string matching. The guess is still verified against both strings before being trusted, falling straight through to the original `find_diff` whenever that verification fails, the text didn't grow, or no caret is available (composition, paste, spellcheck/autocomplete replacing a whole word while the caret sits elsewhere, or deletions).
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
1 participant
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.
The bug
A textblock whose only tracked child is a non-text atom (e.g. an inline checkbox
<input>) silently loses any text typed immediately next to that atom — it's a real, correctly-placed DOM text node, but it never syncs into the document model. Nothing errors; the text just visually appears, then vanishes or duplicates the next time the view re-renders for any unrelated reason.Root cause
Two mirror-image gaps, both keyed on the same wrong assumption: "a block with nothing to diff against is empty."
Detection side —
find_empty_block_text(read_dom_changes's fallback path, after the normal tracked-text-run diff finds nothing changed): only fired when a block's tracked children list was completely empty (e.g. typing into a fresh paragraph after pressing Enter). A block whose one tracked child is a non-text atom isn't empty by that check, so text typed next to the atom was never even looked at.Cleanup side —
try_patch: before rebuilding a block's children from a new document, it only stripped foreign/orphaned DOM nodes (leftover typed text, the synthetic trailing<br>) when the old children list was fully empty. An atom-only block never qualified, so once the fix above got the typed text into the model, the original untracked DOM text node was never cleaned up — it stuck around as a duplicate, growing by one stray character on every subsequent keystroke.The fix
find_empty_block_text: widened the guard from "no tracked children" to "no tracked text children" (empty or atom-only), and the insertion position is now computed from the size of any tracked atom children that precede the new text, instead of always assuming the very start of the block.try_patch: replaced the blanket "remove every child" cleanup with a newstrip_untracked_texthelper, using the same widened guard. It removes only text nodes and the trailing-<br>marker, leaving any atom's own element (e.g. the checkbox<input>) untouched in place — sopatch_children's reused-descriptor path still finds it exactly where it expects.Testing
crates/taino-edit-dom/tests/atom_text_sync.rs, following the existingempty_block.rspattern: mounts a minimal schema with an inline atom, simulates the browser inserting text next to it, and assertsread_dom_changes()produces the correct insert (after the atom, not before) with no duplication on re-render.CONTRIBUTING.mdrequires green:cargo fmt --all -- --check,cargo clippy --all-targets --all-features -- -D warnings,cargo test --all-features(200+ host tests across every crate, all passing),cargo doc --no-deps --all-features. Alsocargo publish --dry-runontaino-edit-dom.taino-edit-dom's own browser test suite (wasm-bindgen-testin headless Chromium viascripts/wasm-test.sh) needs a Linux-only toolchain I don't have available, so I couldn't run it directly. Verified the fix live instead against a real downstream consumer (a checkbox atom + live-typed label, in a real browser via Playwright) — reproduced the exact duplication/loss symptom before the fix and confirmed it's gone after.ROADMAP.md's "Known issues" section updated in this same commit, following the file's own established format for a fixed bug.