From 847d19477b8e772c1603ffefe8982b4ee6922e09 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 18 Sep 2026 21:06:02 +0200 Subject: [PATCH 1/2] fix(html): record what an Android keyboard types into a document An Android keyboard holds a composition open on the word under the caret for as long as the caret stays there. The document editor lost that input in two ways: - On `compositionend` it compared a run with itself, so the step was always empty and was dropped. - While a composition was open, it let every `beforeinput` through without a record, including the keys, Enter and Backspace that it can cancel. So scope `paragraph` did not hold there either. The page showed the text, but the log was empty, and a save wrote the document as it was. Now the editor notes a run's text before the browser writes into it, and records the change after the `input`. Only an event that cannot be cancelled goes through to the browser. Before the editor acts, and before an undo or a redo, it records what the browser wrote. Checked with the browser checks and on an Android 12 emulator with Gboard. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01F6AQY2k86AaPq12nxBfN7A --- CHANGELOG.md | 8 ++ docs/design/document-editing.md | 10 +- docs/design/editing.md | 11 +- src/odr/internal/html/frontend/document.js | 122 ++++++++++++++++----- test/browser/text/README.md | 8 ++ test/browser/text/tests.html | 75 +++++++++++++ 6 files changed, 203 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96380c327..5139f33c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Fix**: the document view's editor recorded nothing an Android keyboard + typed. The keyboard holds a composition open on the word under the caret, + and the editor compared a run with itself when the composition ended, and + let every key, Enter and Backspace through unrecorded while it was open. A + save wrote the document as it was. A composition is now recorded after each + change the browser makes, and an edit that can be cancelled is the editor's + even while a composition is open, so scope `paragraph` holds there too. + ## v7.0.0 - 2026-09-13 - **Breaking**: `DocumentPath`, `Element::document_path()` and diff --git a/docs/design/document-editing.md b/docs/design/document-editing.md index d20c5e837..1766994df 100644 --- a/docs/design/document-editing.md +++ b/docs/design/document-editing.md @@ -156,8 +156,14 @@ place the two could disagree is exactly the bug it was meant to catch. **Where it earns its keep:** a composition cannot be cancelled, so the browser *does* write inside a run. With the page as the model there is nothing to -reconcile — `compositionend` reads the run's text and that is the operation. -With a parallel model that same case would be a merge. +reconcile — the editor notes the run's text before the browser writes, reads it +after the `input`, and the difference is the operation. With a parallel model +that same case would be a merge. + +It is read back after every change rather than once at `compositionend`, +because an Android keyboard holds a composition open on the word under the +caret until the caret leaves it: a save or an undo in between would miss the +word, and the key events that arrive meanwhile are ones the editor owns. ### 7. Read-only engines say nothing diff --git a/docs/design/editing.md b/docs/design/editing.md index 095f0ce6c..d3b0a0b38 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -352,7 +352,7 @@ then intercepts `beforeinput` and takes the edits it can express as operations: | Backspace at the start of a paragraph | taken: the paragraph merges into the one before it | | a paste of plain text, over as many lines as it holds | taken: each line after the first opens a paragraph | | a mark - ctrl/cmd+B, I, U, or `odr.editing.format` - under scope `document` | taken: a run covered in part is cut, and the covered runs are restyled | -| a composition (CJK, autocorrect, dictation) | let through and reconciled on `compositionend` | +| a composition (CJK, autocorrect, dictation, an Android keyboard) | let through, and each change recorded after its `input` | | a soft line break (`insertLineBreak`) | refused, reason `newLine` - no operation carries one | | a range reaching over a picture | taken: the frame carries an address, so the picture goes with the text | | a range reaching over a text box or a table | refused, reason `range` - it holds text of its own, which the reader did not mean to lose | @@ -396,9 +396,12 @@ button is live. One `beforeinput` is one step. **Known holes, both narrow.** A scripted `document.execCommand` can bypass the gate, because Chrome does not fire a cancelable `beforeinput` for every command; trusted input, which is all a reader has, goes through it. And a composition -cannot be cancelled at all, so the editor lets it finish and reads the run back -on `compositionend`; a composition that landed where no run can name it raises -code 9 rather than being dropped. Android WebView's incomplete `beforeinput` +cannot be cancelled at all, so the editor lets the browser write and records +the run's text after each `input`; a run the browser took out of the page +raises `unnameableEdit` rather than being dropped. Only an event that cannot be +cancelled is let through: an Android keyboard holds a composition open on the +word under the caret, so a key, Enter or Backspace arrives while one is open, +and the editor still owns it. Android WebView's incomplete `beforeinput` (decision 8) is the reason that report exists, and the reason a delete whose range the browser did not state is extended by one character rather than refused; verify both on a device. diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index 6670c540b..dfcbeea93 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -103,6 +103,11 @@ return null; } step.apply(); + return record(step); + } + + /// Puts @p step on the log without applying it: the page shows it already. + function record(step) { done.push(step); undone.length = 0; odr.editing.changed(); @@ -1321,36 +1326,88 @@ }); } - // a composition cannot be cancelled, so the browser writes and we read the - // run back afterwards; this is the run it started in - var composing = null; + // A composition cannot be cancelled, so the browser writes inside a run and + // the editor records what it wrote. An Android keyboard holds one open on + // the word under the caret for as long as the caret stays there, so each + // change is recorded after its `input`, not when the composition ends. - root.addEventListener("compositionstart", function () { - var selection = window.getSelection(); - composing = - selection === null || selection.rangeCount === 0 - ? null - : runOf(selection.getRangeAt(0).startContainer); - }); + // the runs the browser may write into before the log hears of it, each with + // the text it held before + var unrecorded = []; - root.addEventListener("compositionend", function () { - var run = composing; - composing = null; - if (!odr.editing.isEnabled()) { + function watch(run) { + if (run === null) { return; } + for (var i = 0; i < unrecorded.length; ++i) { + if (unrecorded[i].run === run) { + return; + } + } + unrecorded.push({ run: run, before: run.textContent }); + } + + function watchSelection() { var selection = window.getSelection(); - var landed = - selection === null || selection.rangeCount === 0 - ? null - : runOf(selection.getRangeAt(0).startContainer); - var target = landed !== null ? landed : run; - if (target === null) { - odr.onError(odr.errorCodes.unnameableEdit, "an edit landed where no operation can name it"); + if (selection !== null && selection.rangeCount > 0) { + watch(runOf(selection.getRangeAt(0).startContainer)); + } + } + + /// One step for what the browser wrote into the watched runs, since it is + /// on the page already. + function recordWritten() { + var changes = []; + for (var i = 0; i < unrecorded.length; ++i) { + var entry = unrecorded[i]; + var after = entry.run.textContent; + if (after === entry.before) { + continue; + } + if (!entry.run.isConnected) { + odr.onError(odr.errorCodes.unnameableEdit, "an edit landed where no operation can name it"); + continue; + } + changes.push({ run: entry.run, before: entry.before, after: after }); + } + unrecorded = []; + if (changes.length === 0) { return; } - // whatever the browser built inside the run, its text is the operation - perform(setRunText(target, target.textContent)); + record({ + ops: changes.map(function (change) { + return { op: "setText", id: idOf(change.run), text: change.after }; + }), + apply: function () { + changes.forEach(function (change) { + change.run.textContent = change.after; + }); + }, + revert: function () { + changes.forEach(function (change) { + change.run.textContent = change.before; + }); + }, + }); + } + + root.addEventListener("compositionstart", function () { + if (odr.editing.isEnabled()) { + watchSelection(); + } + }); + + root.addEventListener("input", function () { + if (odr.editing.isEnabled()) { + recordWritten(); + } + }); + + // for a browser that writes a composition without an `input` for it + root.addEventListener("compositionend", function () { + if (odr.editing.isEnabled()) { + recordWritten(); + } }); root.addEventListener("beforeinput", function (event) { @@ -1372,11 +1429,21 @@ return; } - // mid-composition and unstoppable; `compositionend` reconciles it - if (type === "insertCompositionText" || composing !== null) { + // a composition, and whatever else the browser will not let go of: it + // writes, and the `input` after it is recorded + if (!event.cancelable) { + if (at === null) { + watchSelection(); + } else { + watch(at.start.run); + watch(at.end.run); + } return; } + // what the browser wrote before this goes on the log ahead of it + recordWritten(); + if (at === null) { refuse(event, "range", at); return; @@ -1505,9 +1572,11 @@ }, operations: operations, format: function (style) { + recordWritten(); return format(style, rangeOf({})); }, toggle: function (property) { + recordWritten(); return toggle(property, rangeOf({})); }, canUndo: function () { @@ -1517,6 +1586,7 @@ return undone.length > 0; }, undo: function () { + recordWritten(); if (done.length === 0) { return false; } @@ -1527,6 +1597,7 @@ return true; }, redo: function () { + recordWritten(); if (undone.length === 0) { return false; } @@ -1538,6 +1609,7 @@ }, committed: function () { dropPending(); + unrecorded = []; done.length = 0; undone.length = 0; }, diff --git a/test/browser/text/README.md b/test/browser/text/README.md index 563b69966..3f0578c16 100644 --- a/test/browser/text/README.md +++ b/test/browser/text/README.md @@ -60,6 +60,14 @@ Why the checks look the way they do: the script; the same dispatch is what drops a pending mark when the caret moved. +- **A composition is driven the way a browser fires one**: a `beforeinput` + that cannot be cancelled, the run's text written by hand, and the `input` + after it. An Android keyboard holds a composition open on the word under the + caret, so the group also sends the keys a reader presses meanwhile - an + insert and Enter, which can be cancelled - and checks the editor still owns + them. What a real keyboard does was checked on an Android emulator with + Gboard, which no page here can stand in for. + **Scripted editing is not the editing a reader does, which is why no check uses `execCommand`.** Chrome's scripted path raises no cancelable `beforeinput`, so `execCommand("insertParagraph")` splits a paragraph without the editor ever diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html index 72cf297e0..d5b2fb74a 100644 --- a/test/browser/text/tests.html +++ b/test/browser/text/tests.html @@ -865,6 +865,81 @@ select(run(11).firstChild, 2); check("and Enter is taken", input("insertParagraph") === "taken"); + // ------------------------------------------------------ compositions + + // What a browser does for a composition, which cannot be cancelled: a + // `beforeinput` nobody may cancel, the text written into the run, and + // the `input` after it. An Android keyboard keeps one open on the word + // under the caret, so a composition is how most of its text arrives. + function compose(id, text) { + var target = run(id); + select(target.firstChild, target.firstChild.length); + document.body.dispatchEvent( + new InputEvent("beforeinput", { + inputType: "insertCompositionText", + data: text, + bubbles: true, + cancelable: false, + }) + ); + target.firstChild.data = text; + // replacing the data puts the caret at 0; a browser leaves it after + // what it wrote + select(target.firstChild, text.length); + document.body.dispatchEvent( + new InputEvent("input", { inputType: "insertCompositionText", bubbles: true }) + ); + } + + reset(); + note("a composition"); + select(run(31).firstChild, 5); + document.body.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + compose(31, "thirdx"); + check("what the browser wrote is on the log", ops().length === 1, ops()); + check( + "as the run's new text", + ops()[0].op === "setText" && ops()[0].id === 31 && ops()[0].text === "thirdx", + ops() + ); + check("and the host hears of it", changes.length > 0 && changes[changes.length - 1].canUndo); + compose(31, "thirdxy"); + check("a second change folds into the same operation", ops().length === 1, ops()); + check("naming the text it ends at", ops()[0].text === "thirdxy", ops()); + + // the keyboard still holds the composition open, and the next key + // arrives as an edit the editor owns + check("an insert while it is open is taken", input("insertText", "!") === "taken"); + check("into the run", run(31).textContent === "thirdxy!", run(31).textContent); + check("and on the log", ops()[0].text === "thirdxy!", ops()); + check("Enter while it is open is taken", input("insertParagraph") === "taken"); + check("and splits the paragraph", texts() === "first run a link and a tail|bold|thirdxy!||", texts()); + document.body.dispatchEvent(new CompositionEvent("compositionend", { bubbles: true })); + + check("undo takes back the split", odr.editing.undo() && texts() === "first run a link and a tail|bold|thirdxy!|", texts()); + check("then the insert", odr.editing.undo() && run(31).textContent === "thirdxy"); + check("then each change the browser wrote", odr.editing.undo() && run(31).textContent === "thirdx"); + check("back to the text before it", odr.editing.undo() && run(31).textContent === "third"); + check("and redo writes it again", odr.editing.redo() && run(31).textContent === "thirdx"); + + // a browser that ends a composition without an `input` for what it wrote + reset(); + select(run(11).firstChild, 5); + document.body.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + run(11).firstChild.data = "first rune "; + document.body.dispatchEvent(new CompositionEvent("compositionend", { bubbles: true })); + check("the end of the composition reads the run back", ops().length === 1 && ops()[0].text === "first rune ", ops()); + + // scope `paragraph` still gates an edit made while a composition is open + reset(); + document.body.setAttribute("data-odr-editing-scope", "paragraph"); + select(run(31).firstChild, 2); + document.body.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + check("Enter while it is open is refused under scope paragraph", input("insertParagraph") === "refused"); + check("so the paragraph stays whole", texts() === "first run a link and a tail|bold|third|", texts()); + document.body.dispatchEvent(new CompositionEvent("compositionend", { bubbles: true })); + document.body.removeAttribute("data-odr-editing-scope"); + // ---------------------------------------------- what is not an edit // A script rewriting the page is not a reader typing, so the log stays From 776826b937ca3d6e0010bf4a738195a3a21c7414 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 19 Sep 2026 08:48:11 +0200 Subject: [PATCH 2/2] fix(html): a recorded composition is its own undo step The `beforeinput` handler started a gesture and then recorded what the browser wrote without an `input`, so that text and the key after it had one gesture, and one undo took back both. The handler now records the text first. WebKit can fire a composition `beforeinput` that can be cancelled. The editor refused it, so the browser typed nothing. The editor now lets every composition type through. `committed()` no longer drops text that the browser wrote but the log does not hold yet, because the save did not contain that text. The comments, the design notes and the changelog entry are shorter. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01C725vurM2ZqBY6ddV2FMZ7 --- CHANGELOG.md | 7 +----- docs/design/document-editing.md | 6 ++---- docs/design/editing.md | 12 +++++------ src/odr/internal/html/frontend/document.js | 25 +++++++++------------- test/browser/text/README.md | 8 ++----- test/browser/text/tests.html | 25 +++++++++++++++------- 6 files changed, 37 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ded4a97b..4d073a45b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,12 +30,7 @@ The release run heads these entries with the version and opens a fresh `redo()` now take back and replay the whole edit. - **Fix**: the document view's editor recorded nothing an Android keyboard - typed. The keyboard holds a composition open on the word under the caret, - and the editor compared a run with itself when the composition ended, and - let every key, Enter and Backspace through unrecorded while it was open. A - save wrote the document as it was. A composition is now recorded after each - change the browser makes, and an edit that can be cancelled is the editor's - even while a composition is open, so scope `paragraph` holds there too. + typed, so a save lost it. A composition is now recorded after each change. ## v7.0.0 - 2026-09-13 diff --git a/docs/design/document-editing.md b/docs/design/document-editing.md index 1766994df..3b7c9804c 100644 --- a/docs/design/document-editing.md +++ b/docs/design/document-editing.md @@ -160,10 +160,8 @@ reconcile — the editor notes the run's text before the browser writes, reads i after the `input`, and the difference is the operation. With a parallel model that same case would be a merge. -It is read back after every change rather than once at `compositionend`, -because an Android keyboard holds a composition open on the word under the -caret until the caret leaves it: a save or an undo in between would miss the -word, and the key events that arrive meanwhile are ones the editor owns. +It is read back after every change, not once at `compositionend`, because an +Android keyboard holds a composition open on the word under the caret. ### 7. Read-only engines say nothing diff --git a/docs/design/editing.md b/docs/design/editing.md index 16d57f358..b28ee3acd 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -398,13 +398,11 @@ gate, because Chrome does not fire a cancelable `beforeinput` for every command; trusted input, which is all a reader has, goes through it. And a composition cannot be cancelled at all, so the editor lets the browser write and records the run's text after each `input`; a run the browser took out of the page -raises `unnameableEdit` rather than being dropped. Only an event that cannot be -cancelled is let through: an Android keyboard holds a composition open on the -word under the caret, so a key, Enter or Backspace arrives while one is open, -and the editor still owns it. Android WebView's incomplete `beforeinput` -(decision 8) is the reason that report exists, and the reason a delete whose -range the browser did not state is extended by one character rather than -refused; verify both on a device. +raises `unnameableEdit` rather than being dropped. A key that arrives while a +composition is open is still the editor's. Android WebView's incomplete +`beforeinput` (decision 8) is the reason that report exists, and the reason a +delete whose range the browser did not state is extended by one character +rather than refused; verify both on a device. ### 14. The scope is host policy, and the page refuses past it diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index 9c42a3b26..b4eda931e 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -1331,13 +1331,11 @@ }); } - // A composition cannot be cancelled, so the browser writes inside a run and - // the editor records what it wrote. An Android keyboard holds one open on - // the word under the caret for as long as the caret stays there, so each - // change is recorded after its `input`, not when the composition ends. + // A composition cannot be cancelled, so the browser writes and the editor + // records each change after its `input`: an Android keyboard holds one open + // on the word under the caret for as long as the caret stays there. - // the runs the browser may write into before the log hears of it, each with - // the text it held before + // the runs the browser may write into, each with the text it held before var unrecorded = []; function watch(run) { @@ -1359,8 +1357,7 @@ } } - /// One step for what the browser wrote into the watched runs, since it is - /// on the page already. + /// One step for what the browser wrote into the watched runs. function recordWritten() { var changes = []; for (var i = 0; i < unrecorded.length; ++i) { @@ -1417,6 +1414,8 @@ }); root.addEventListener("beforeinput", function (event) { + // what the browser wrote before this is a gesture of its own + recordWritten(); gesture += 1; var type = event.inputType; var at = rangeOf(event); @@ -1436,9 +1435,9 @@ return; } - // a composition, and whatever else the browser will not let go of: it - // writes, and the `input` after it is recorded - if (!event.cancelable) { + // the browser writes these itself, and the `input` after it is recorded; + // WebKit may let a composition be cancelled, but it is not an edit we own + if (!event.cancelable || /Composition/.test(type)) { if (at === null) { watchSelection(); } else { @@ -1448,9 +1447,6 @@ return; } - // what the browser wrote before this goes on the log ahead of it - recordWritten(); - if (at === null) { refuse(event, "range", at); return; @@ -1622,7 +1618,6 @@ }, committed: function () { dropPending(); - unrecorded = []; done.length = 0; undone.length = 0; }, diff --git a/test/browser/text/README.md b/test/browser/text/README.md index 3f0578c16..4c97dc4b4 100644 --- a/test/browser/text/README.md +++ b/test/browser/text/README.md @@ -61,12 +61,8 @@ Why the checks look the way they do: moved. - **A composition is driven the way a browser fires one**: a `beforeinput` - that cannot be cancelled, the run's text written by hand, and the `input` - after it. An Android keyboard holds a composition open on the word under the - caret, so the group also sends the keys a reader presses meanwhile - an - insert and Enter, which can be cancelled - and checks the editor still owns - them. What a real keyboard does was checked on an Android emulator with - Gboard, which no page here can stand in for. + that cannot be cancelled, the run written by hand, and the `input` after it. + A real Android keyboard was checked on an emulator with Gboard. **Scripted editing is not the editing a reader does, which is why no check uses `execCommand`.** Chrome's scripted path raises no cancelable `beforeinput`, so diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html index 3846e4c3d..2adc3c608 100644 --- a/test/browser/text/tests.html +++ b/test/browser/text/tests.html @@ -875,10 +875,7 @@ // ------------------------------------------------------ compositions - // What a browser does for a composition, which cannot be cancelled: a - // `beforeinput` nobody may cancel, the text written into the run, and - // the `input` after it. An Android keyboard keeps one open on the word - // under the caret, so a composition is how most of its text arrives. + // a `beforeinput` that cannot be cancelled, the run written, an `input` function compose(id, text) { var target = run(id); select(target.firstChild, target.firstChild.length); @@ -891,8 +888,7 @@ }) ); target.firstChild.data = text; - // replacing the data puts the caret at 0; a browser leaves it after - // what it wrote + // a browser leaves the caret after what it wrote select(target.firstChild, text.length); document.body.dispatchEvent( new InputEvent("input", { inputType: "insertCompositionText", bubbles: true }) @@ -915,8 +911,7 @@ check("a second change folds into the same operation", ops().length === 1, ops()); check("naming the text it ends at", ops()[0].text === "thirdxy", ops()); - // the keyboard still holds the composition open, and the next key - // arrives as an edit the editor owns + // the composition is still open, and the next key is the editor's check("an insert while it is open is taken", input("insertText", "!") === "taken"); check("into the run", run(31).textContent === "thirdxy!", run(31).textContent); check("and on the log", ops()[0].text === "thirdxy!", ops()); @@ -938,6 +933,20 @@ document.body.dispatchEvent(new CompositionEvent("compositionend", { bubbles: true })); check("the end of the composition reads the run back", ops().length === 1 && ops()[0].text === "first rune ", ops()); + reset(); + select(run(11).firstChild, 5); + document.body.dispatchEvent(new CompositionEvent("compositionstart", { bubbles: true })); + run(11).firstChild.data = "first rune "; + select(run(11).firstChild, 10); + check("a key after text written without an input is taken", input("insertText", "!") === "taken"); + check("and undone apart from that text", odr.editing.undo() && run(11).textContent === "first rune ", run(11).textContent); + document.body.dispatchEvent(new CompositionEvent("compositionend", { bubbles: true })); + + // WebKit may let a composition's `beforeinput` be cancelled + reset(); + select(run(31).firstChild, 5); + check("a composition that can be cancelled is let through", input("insertCompositionText", "x") === "taken" && !prevented); + // scope `paragraph` still gates an edit made while a composition is open reset(); document.body.setAttribute("data-odr-editing-scope", "paragraph");