diff --git a/CHANGELOG.md b/CHANGELOG.md index 7731f8d5c..4d073a45b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,9 @@ The release run heads these entries with the version and opens a fresh between the steps that the reader never made. `odr.editing.undo()` and `redo()` now take back and replay the whole edit. +- **Fix**: the document view's editor recorded nothing an Android keyboard + typed, so a save lost it. A composition is now recorded after each change. + ## 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..3b7c9804c 100644 --- a/docs/design/document-editing.md +++ b/docs/design/document-editing.md @@ -156,8 +156,12 @@ 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, 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 679bb2461..b28ee3acd 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,12 +396,13 @@ 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` -(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. +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. 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 0c63e6440..b4eda931e 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -105,8 +105,13 @@ if (step === null) { return null; } - step.gesture = gesture; step.apply(); + return record(step); + } + + /// Puts @p step on the log without applying it: the page shows it already. + function record(step) { + step.gesture = gesture; done.push(step); undone.length = 0; odr.editing.changed(); @@ -1326,40 +1331,91 @@ }); } - // 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 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. - 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, each with the text it held before + var unrecorded = []; - root.addEventListener("compositionend", function () { - gesture += 1; - 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. + 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)); + gesture += 1; + 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) { + // what the browser wrote before this is a gesture of its own + recordWritten(); gesture += 1; var type = event.inputType; var at = rangeOf(event); @@ -1379,8 +1435,15 @@ return; } - // mid-composition and unstoppable; `compositionend` reconciles it - if (type === "insertCompositionText" || composing !== null) { + // 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 { + watch(at.start.run); + watch(at.end.run); + } return; } @@ -1512,9 +1575,11 @@ }, operations: operations, format: function (style) { + recordWritten(); return format(style, rangeOf({})); }, toggle: function (property) { + recordWritten(); return toggle(property, rangeOf({})); }, canUndo: function () { @@ -1524,6 +1589,7 @@ return undone.length > 0; }, undo: function () { + recordWritten(); if (done.length === 0) { return false; } @@ -1537,6 +1603,7 @@ return true; }, redo: function () { + recordWritten(); if (undone.length === 0) { return false; } diff --git a/test/browser/text/README.md b/test/browser/text/README.md index 563b69966..4c97dc4b4 100644 --- a/test/browser/text/README.md +++ b/test/browser/text/README.md @@ -60,6 +60,10 @@ 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 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 `execCommand("insertParagraph")` splits a paragraph without the editor ever diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html index 169f3a60a..2adc3c608 100644 --- a/test/browser/text/tests.html +++ b/test/browser/text/tests.html @@ -873,6 +873,90 @@ select(run(11).firstChild, 2); check("and Enter is taken", input("insertParagraph") === "taken"); + // ------------------------------------------------------ compositions + + // 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); + document.body.dispatchEvent( + new InputEvent("beforeinput", { + inputType: "insertCompositionText", + data: text, + bubbles: true, + cancelable: false, + }) + ); + target.firstChild.data = text; + // a browser leaves the caret 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 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()); + 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()); + + 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"); + 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