diff --git a/CHANGELOG.md b/CHANGELOG.md index c79d846b4..b4082b4a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,17 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- A text document is edited the way a reader expects: typing, replacing and + deleting across runs and paragraphs, Enter, Backspace at a paragraph start, + and a plain-text paste that opens a paragraph per line. + +- **Undo and redo are the editor's.** `odr.editing.undo()` / `redo()` answer + for a text document, `odr.onEditChange` reports `canUndo` / `canRedo` + truthfully, and ctrl/cmd+Z is taken. + +- Enter is no longer refused. Reason `newLine` (code 1) now marks only a soft + line break, which no operation carries. + - A paragraph splits, merges and is inserted — `splitParagraph`, `mergeParagraph`, `insertParagraph`, and the matching `Document` methods in C++. Enter, Backspace at a paragraph start and a delete across paragraphs. diff --git a/docs/design/document-editing.md b/docs/design/document-editing.md index 596b61ba0..2f5d2c830 100644 --- a/docs/design/document-editing.md +++ b/docs/design/document-editing.md @@ -138,7 +138,27 @@ be what we would have to replay. every edit it was going to apply. So this work has to carry undo/redo, which until now was honestly refused (`canUndo` answered false and a host's button stayed grey). That is phase 3 item 2 of [`editing.md`](editing.md), and it -arrives here because it is no longer optional. +arrives here because it is no longer optional. One `beforeinput` is one undo +step; a browser coalesces a word, and matching that is a later refinement. + +### 6b. The page is the model, because the editor is the only one writing it + +Decision 8 of [`editing.md`](editing.md) called for a structured model beside +the page, with the DOM as its projection. The editor keeps no such second +structure: the runs and paragraphs are addressed in the page by +`data-odr-id`, and **that is the model**. + +**Why the second structure bought nothing:** what decision 8 was protecting +against is contenteditable inventing markup we cannot map back. Owning the +mutation removes that at the source — nothing but this editor writes the page, +so the page cannot drift into a shape the element tree has no name for. A +parallel model would have to be kept in step with the page anyway, and the +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. ### 7. Read-only engines say nothing @@ -272,8 +292,8 @@ Each step is a pull request that builds and tests on its own. **Landed.** 3. **Paragraphs split and merge.** `splitParagraph`, `mergeParagraph`, `insertParagraph`. **Landed.** -4. **The browser editor.** Model-first, owns the DOM mutation, records the ops, - and carries undo/redo (decision 6). +4. **The browser editor.** Owns the DOM mutation, records the ops, and carries + undo/redo (decisions 6 and 6b). **Landed.** 5. **pptx writes.** `save`, `is_editable`, `is_savable`, the capability row and the new hooks over `a:p` / `a:r`. @@ -302,6 +322,30 @@ behind. It is valid in both formats — the corpus is full of `` where a + paragraph holds nothing, `` where it holds something. An edited page + then looks like a re-rendered one, which is what makes the two comparable. + ## Open questions - **A list item** is a paragraph in a list. Enter at the end of one should make diff --git a/docs/design/editing.md b/docs/design/editing.md index c21c21a59..73a73a742 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -342,15 +342,18 @@ attribute. ### 13. One editable view, and every edit it cannot replay is refused `enable()` puts `contenteditable` on the **body**, not on each run. The editor -then intercepts `beforeinput` and refuses everything that is not the text of one -addressed run: +then intercepts `beforeinput` and takes the edits it can express as operations: | The edit | What happens | |---|---| -| text typed, replaced, pasted plain, deleted, composed — inside one run | allowed, and the run joins the log | -| a new line (`insertParagraph`, `insertLineBreak`) | refused, reason `newLine` | +| text typed, replaced, deleted — inside one run, across runs, across paragraphs | taken | +| Enter | taken: the paragraph splits where the caret sits | +| 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 composition (CJK, autocorrect, dictation) | let through and reconciled on `compositionend` | +| a soft line break (`insertLineBreak`) | refused, reason `newLine` - no operation carries one | | anything else the browser offers (a mark, a list, a drop) | refused, reason `unsupportedEdit` | -| an edit spanning two runs, or landing outside every run | refused, reason `range` | +| an edit reaching over a picture or a table, or landing outside every run | refused, reason `range` | **Why the whole view rather than a run at a time:** `contenteditable` per run makes every run its own editing host, and a host is a wall. The caret cannot @@ -365,52 +368,36 @@ over every run. it says *what* the edit is (`inputType`), it says *where* (`getTargetRanges()`), and it is cancelable. `text.js` already edits the plain-text view this way. -**The whitelist is closed, not open.** Only the input types that change the text -of one run are allowed; anything unrecognised is refused. An open list would let -a browser-specific `inputType` through to a `MutationObserver` that only watches -`characterData`, and a structural change would then be invisible to the log and -saved wrong. Refusing something we could have allowed costs a reader one -gesture; allowing something we cannot replay costs them their document. +**The whitelist is closed, not open.** Only the input types the editor can +express are taken; anything unrecognised is refused. Refusing something we could +have allowed costs a reader one gesture; allowing something we cannot replay +costs them their document. **The address is the whole guard.** No element is marked non-editable: an edit is -allowed because it lands inside a `x-s[data-odr-id]` run, so a picture, a table's -furniture, the gap between two paragraphs and the page box are all refused -without a single attribute of their own. That is decision 10's rule — mark the -exceptions, not the rest — applied to the caret instead of to a cell. - -**`input` is what the log collects on, not a `MutationObserver`.** The browser -raises `input` when *it* applied an edit; a script rewriting the page raises -none. That is the whole difference: `search.js` wraps every match in a ``, -which an observer watching `characterData` reads as nine edits — measured, and -it lit the host's save button and put nine no-op `setText` ops in the log. The -run is the one `beforeinput` named, or the one the caret sits in where no -`beforeinput` arrived; a run the editor cannot name at all raises code 9 rather -than being dropped. +allowed because it lands inside a `x-s[data-odr-id]` run and reaches over +nothing but runs, so a picture, a table's furniture and the page box are all +refused without a single attribute of their own. That is decision 10's rule — +mark the exceptions, not the rest — applied to the caret instead of to a cell. + +**The editor owns the edit.** It cancels the `beforeinput` and splices the page +itself, rather than letting the browser apply the change and reading the run +back. See decision 6 of [`document-editing.md`](document-editing.md) for why that had +to change. + +**Undo is the editor's**, because cancelling every edit leaves the browser's own +stack empty. Each step holds the operations it puts on the wire and the two +halves of taking it back, so `canUndo` and the chord now agree and a host's undo +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, is refused correctly. And a -composition cannot be cancelled at all — `insertCompositionText` is allowed and -reconciled afterwards, which is why the observer reports code 9 when text -changes where no op can name it, rather than dropping it in silence. Android -WebView's incomplete `beforeinput` (decision 8) is the reason that report -exists; verify it on a device before trusting the gate there. - -**Two limits a reader meets, and phase 3 is where both go:** - -- **Undo belongs to the browser, not to us.** Every allowed edit is one the - browser applied, so its own stack is the one that replays — ctrl+Z works, and - `chordKey` leaves the key alone because no editor claims it (decision 9). But - we cannot read that stack's depth, so `canUndo` is honestly false and a host's - undo *button* stays grey. Phase 3 item 2 is what fixes it: once the editor - records an op with its inverse, it answers `undo()` and joins the shared log. - Until then the button and the chord disagree, which is worse than either. -- **Backspace at the start of a run is refused.** Its target range reaches back - into the run before it, so the edit spans two and `range` refuses it — - merging two runs is not something `setText` can express. It did nothing under - the per-run hosts either; the difference is that it now says why. The op that - would fix it is `deleteRange` across runs, which needs the write-side adapter - work in phase 2, not a browser change. +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. ## Preliminary implementation plan (ODF / OOXML) @@ -458,9 +445,10 @@ Sequence within Phase 2: (a) delete element, (b) toggle mark on a range, ### Phase 3 — Browser editor -The frame is landed (decisions 9 to 12): the mode, the refusals, the callbacks -and the keyboard classes are in `frontend/editing.js`, and `document.js` -attaches the skeleton editor to it. What is left is the editor itself. +**Landed**, over [`document-editing.md`](document-editing.md)'s schema. The mode, the +refusals, the callbacks and the keyboard classes are in `frontend/editing.js`; +`document.js` holds the editor, which owns every edit and carries its own +undo. 1. Model keyed by `data-odr-id`; op recorder; composition-aware reconciliation. `beforeinput` is already the gate (decision 13); what is left is owning the diff --git a/src/odr/document.cpp b/src/odr/document.cpp index 07bba66be..05e81a4ef 100644 --- a/src/odr/document.cpp +++ b/src/odr/document.cpp @@ -212,14 +212,27 @@ void Document::edit(const std::string_view operations, } if (name == "insertText") { + const auto text = operation.at("text").get(); + const std::int64_t address = reserve(operation); + + // `parent` appends into an element rather than naming a run to sit + // beside + if (operation.contains("parent")) { + if (operation.contains("after") || operation.contains("before")) { + throw std::invalid_argument( + "insertText names `parent` or a run to sit beside, not both"); + } + const Element parent = element_of(operation, "parent"); + minted.emplace(address, append_text(parent, text).identifier()); + continue; + } + const bool after = operation.contains("after"); if (after == operation.contains("before")) { throw std::invalid_argument( - "insertText names one of `after` and `before`"); + "insertText names one of `after`, `before` and `parent`"); } - const std::int64_t address = reserve(operation); const Text anchor = text_of(operation, after ? "after" : "before"); - const auto text = operation.at("text").get(); const Text created = after ? insert_text_after(anchor, text) : insert_text_before(anchor, text); minted.emplace(address, created.identifier()); @@ -308,6 +321,14 @@ Text Document::insert_text_(const Text &anchor, const Placement where, return {adapter, identifier, adapter->text_adapter(identifier)}; } +Text Document::append_text(const Element &parent, + const std::string &text) const { + const internal::abstract::ElementAdapter *adapter = m_impl->element_adapter(); + const ElementIdentifier identifier = + adapter->element_append_text(check_(parent), text); + return {adapter, identifier, adapter->text_adapter(identifier)}; +} + /// The adapter @p paragraph answers to, refusing an element that is not a /// paragraph of this document. const internal::abstract::ParagraphAdapter * diff --git a/src/odr/document.hpp b/src/odr/document.hpp index 4d62f09b2..90c895013 100644 --- a/src/odr/document.hpp +++ b/src/odr/document.hpp @@ -82,6 +82,11 @@ class Document final { [[nodiscard]] Text insert_text_after(const Text &anchor, const std::string &text) const; + /// A run as the last child of @p parent. What a paragraph holding no run at + /// all is typed into. + [[nodiscard]] Text append_text(const Element &parent, + const std::string &text) const; + /// Splits @p paragraph after @p after - one of its descendants, or an /// element that does not exist to move every child - into a new paragraph /// of the same style. Refuses where an element between the two is one it diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index ebb63e3fb..c3ec31d95 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -115,6 +115,15 @@ class ElementAdapter { throw UnsupportedOperation(); } + /// A run holding @p text as the last child of @p element_id. What a + /// paragraph holding no run at all is typed into. + /// @throws UnsupportedOperation where the engine cannot write. + virtual ElementIdentifier + element_append_text([[maybe_unused]] const ElementIdentifier element_id, + [[maybe_unused]] const std::string &text) const { + throw UnsupportedOperation(); + } + [[nodiscard]] virtual const TextRootAdapter * text_root_adapter([[maybe_unused]] const ElementIdentifier element_id) const { return nullptr; diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index 2781a1f85..1d19f4ca8 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -1,6 +1,6 @@ -// The text editor, attached to `odr.editing`. The mode makes the whole view -// editable and refuses what it cannot replay: `setText` is the only op the -// file takes, so an edit has to land inside one addressed run. +// The text editor, attached to `odr.editing`. It owns the edit: it cancels +// what the browser was about to do and splices the page itself, so the markup +// stays what the renderer wrote. See `docs/design/document-editing.md`. (function () { "use strict"; @@ -13,81 +13,736 @@ } var root = document.body; - var modified = {}; - function idOf(run) { - return Number(run.getAttribute("data-odr-id")); + // ---------------------------------------------------------------- address + + // ids for the elements this session creates; the render wrote the positive + // ones, and replay tells the two apart by the sign + var lastMinted = 0; + + function mint() { + return --lastMinted; } - function operations() { - var ops = []; - for (var id in modified) { - if (Object.prototype.hasOwnProperty.call(modified, id)) { - ops.push({ - op: "setText", - id: Number(id), - // Not `innerText`: that is the rendered text, and it drops the - // trailing space a reader just typed. - text: modified[id].textContent, - }); - } - } - return ops; + function idOf(element) { + return element === null ? null : Number(element.getAttribute("data-odr-id")); } - /// The run @p node sits in, or null where it sits outside every run. + /// The run @p node sits in, or null where it sits outside every run. A + /// paragraph carries an address too, so the tag is part of the question. function runOf(node) { - if (node === null) { + if (node === null || node === undefined) { return null; } var element = node.nodeType === 1 ? node : node.parentElement; return element === null ? null : element.closest("x-s[data-odr-id]"); } - // The input types that only ever change the text of one run. - var textual = { - insertText: 1, - insertReplacementText: 1, - insertFromPaste: 1, - insertCompositionText: 1, - deleteContent: 1, - deleteContentBackward: 1, - deleteContentForward: 1, - deleteByCut: 1, - deleteWordBackward: 1, - deleteWordForward: 1, - deleteSoftLineBackward: 1, - deleteSoftLineForward: 1, - // Its stack holds the edits above and nothing else: the structural ones - // never happened. - historyUndo: 1, - historyRedo: 1, - }; + function paragraphOf(node) { + if (node === null || node === undefined) { + return null; + } + var element = node.nodeType === 1 ? node : node.parentElement; + return element === null ? null : element.closest("x-p[data-odr-id]"); + } + + /// The runs of @p paragraph, in document order - one under a span or a link + /// counts as the paragraph's. + function runsOf(paragraph) { + return Array.prototype.slice.call( + paragraph.querySelectorAll("x-s[data-odr-id]") + ); + } + + // ------------------------------------------------------------------- page + + /// The renderer ends a paragraph with `
` where it holds nothing and + /// `` where it holds something; keeping to that makes an edited page + /// look like a freshly rendered one. + function refreshLineBox(paragraph) { + var last = paragraph.lastChild; + while (last !== null && (last.nodeName === "BR" || last.nodeName === "WBR")) { + paragraph.removeChild(last); + last = paragraph.lastChild; + } + // a picture is content the same way text is, as the renderer has it + var holds = + paragraph.textContent !== "" || paragraph.firstElementChild !== null; + paragraph.appendChild(document.createElement(holds ? "wbr" : "br")); + } + + /// A copy of @p element carrying what it looks like and none of what it + /// holds. @p id is null for a wrapper no operation names. + function shellCopy(element, id) { + var copy = element.cloneNode(false); + if (id === null) { + copy.removeAttribute("data-odr-id"); + } else { + copy.setAttribute("data-odr-id", String(id)); + } + return copy; + } + + // -------------------------------------------------------------------- log + + // a step holds the operations it puts on the wire and the two halves of + // taking it back; `done` is what a save reads, `undone` what redo replays + var done = []; + var undone = []; + + function perform(step) { + if (step === null) { + return null; + } + step.apply(); + done.push(step); + undone.length = 0; + odr.editing.changed(); + return step; + } + + /// A copy of @p op carrying @p text: folding must not write into the + /// operation its step hands over again after an undo and a redo. + function withText(op, text) { + var copy = {}; + for (var field in op) { + if (Object.prototype.hasOwnProperty.call(op, field)) { + copy[field] = op[field]; + } + } + copy.text = text; + return copy; + } + + /// Folds what a save need not carry: several edits to one run are the text + /// it ends at, and a run created and typed into is one insert. Only adjacent + /// operations fold, so nothing between them can depend on it. + function coalesce(ops) { + var result = []; + for (var i = 0; i < ops.length; ++i) { + var op = ops[i]; + var last = result.length === 0 ? null : result[result.length - 1]; + if ( + last !== null && + op.op === "setText" && + (last.op === "setText" || last.op === "insertText") && + last.id === op.id + ) { + result[result.length - 1] = withText(last, op.text); + continue; + } + result.push(op); + } + return result; + } + + function operations() { + var ops = []; + for (var i = 0; i < done.length; ++i) { + for (var j = 0; j < done[i].ops.length; ++j) { + ops.push(done[i].ops[j]); + } + } + return coalesce(ops); + } + + // -------------------------------------------------------------- mutations + + function setRunText(run, text) { + var before = run.textContent; + if (text === before) { + return null; // an edit that changes nothing is not an operation + } + return { + ops: [{ op: "setText", id: idOf(run), text: text }], + apply: function () { + run.textContent = text; + }, + revert: function () { + run.textContent = before; + }, + }; + } + + /// A run beside @p anchor, cloned from it so it lands in the same parent - + /// which is what gives it the same style on replay. + function insertRun(anchor, where, text) { + var id = mint(); + var run = shellCopy(anchor, id); + run.textContent = text; + var op = { op: "insertText", text: text, id: id }; + op[where] = idOf(anchor); + return { + run: run, + ops: [op], + apply: function () { + anchor.parentNode.insertBefore( + run, + where === "after" ? anchor.nextSibling : anchor + ); + }, + revert: function () { + run.parentNode.removeChild(run); + }, + }; + } + + /// The first run of a paragraph that holds none: what a reader types into + /// after Enter. + function appendRun(paragraph, text) { + var id = mint(); + var run = document.createElement("x-s"); + run.setAttribute("data-odr-id", String(id)); + run.textContent = text; + return { + run: run, + ops: [{ op: "insertText", parent: idOf(paragraph), text: text, id: id }], + apply: function () { + // ahead of the line box, which is the paragraph's last child + paragraph.insertBefore(run, paragraph.lastChild); + refreshLineBox(paragraph); + }, + revert: function () { + paragraph.removeChild(run); + refreshLineBox(paragraph); + }, + }; + } + + function removeElement(element) { + var parent = element.parentNode; + var at = element.nextSibling; + return { + ops: [{ op: "removeElement", id: idOf(element) }], + apply: function () { + parent.removeChild(element); + }, + revert: function () { + parent.insertBefore(element, at); + }, + }; + } + + /// Splits @p element after @p stays - one of its children, or null to move + /// all of them - into a copy of itself. The line box is the paragraph's own + /// and stays out of the move. + function splitLevel(element, stays, id, undoLog) { + var copy = shellCopy(element, id); + element.parentNode.insertBefore(copy, element.nextSibling); + undoLog.push(function () { + copy.parentNode.removeChild(copy); + }); - // A new line has nowhere to go in a run, and the reader knows the key. - var named = { insertParagraph: "newLine", insertLineBreak: "newLine" }; + var node = stays === null ? element.firstChild : stays.nextSibling; + while (node !== null) { + var next = node.nextSibling; + if (node.nodeName !== "BR" && node.nodeName !== "WBR") { + (function (moved, from, at) { + undoLog.push(function () { + from.insertBefore(moved, at); + }); + })(node, element, next); + copy.appendChild(node); + } + node = next; + } + return copy; + } + + /// Splits @p paragraph after @p after - one of its runs, or null to move + /// everything. Every span and link on the way up is split too. + /// No `after` splits before every child, stated as an absent key. + function splitOp(paragraph, after, id) { + var op = { op: "splitParagraph", paragraph: idOf(paragraph), id: id }; + if (after !== null) { + op.after = idOf(after); + } + return op; + } + + function splitParagraph(paragraph, after) { + var id = mint(); + var undoLog = []; + var tail = null; + + return { + get tail() { + return tail; + }, + ops: [splitOp(paragraph, after, id)], + apply: function () { + undoLog = []; + var stays = after; + var level = after === null ? paragraph : after.parentNode; + while (level !== paragraph) { + splitLevel(level, stays, null, undoLog); + stays = level; + level = level.parentNode; + } + tail = splitLevel(paragraph, stays, id, undoLog); + refreshLineBox(paragraph); + refreshLineBox(tail); + }, + revert: function () { + for (var i = undoLog.length - 1; i >= 0; --i) { + undoLog[i](); + } + undoLog = []; + refreshLineBox(paragraph); + tail = null; + }, + }; + } + + /// @p paragraph takes the children of the paragraph after it, which goes. + function mergeParagraph(paragraph) { + var next = null; + var undoLog = []; + return { + ops: [{ op: "mergeParagraph", paragraph: idOf(paragraph) }], + apply: function () { + undoLog = []; + next = paragraph.nextElementSibling; + var parent = next.parentNode; + var at = next.nextSibling; + undoLog.push(function () { + parent.insertBefore(next, at); + }); - /// Where an edit lands: `run` is the one run it is confined to, null where - /// it spans two or lands outside every run. `id` is where it starts. - function target(event) { + var node = next.firstChild; + while (node !== null) { + var following = node.nextSibling; + if (node.nodeName !== "BR" && node.nodeName !== "WBR") { + (function (moved, from, back) { + undoLog.push(function () { + from.insertBefore(moved, back); + }); + })(node, next, following); + paragraph.insertBefore(node, paragraph.lastChild); + } + node = following; + } + parent.removeChild(next); + refreshLineBox(paragraph); + }, + revert: function () { + for (var i = undoLog.length - 1; i >= 0; --i) { + undoLog[i](); + } + undoLog = []; + refreshLineBox(paragraph); + refreshLineBox(next); + }, + }; + } + + function insertParagraph(after) { + var id = mint(); + var paragraph = shellCopy(after, id); + paragraph.appendChild(document.createElement("br")); + return { + paragraph: paragraph, + ops: [{ op: "insertParagraph", after: idOf(after), id: id }], + apply: function () { + after.parentNode.insertBefore(paragraph, after.nextSibling); + }, + revert: function () { + paragraph.parentNode.removeChild(paragraph); + }, + }; + } + + // ------------------------------------------------------------------ caret + + function placeCaret(run, offset) { + var node = run.firstChild; + if (node === null || node.nodeType !== 3) { + node = run.insertBefore(document.createTextNode(""), run.firstChild); + } + var range = document.createRange(); + range.setStart(node, Math.max(0, Math.min(offset, node.data.length))); + range.collapse(true); + var selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + } + + /// The offset into @p run's text that (@p container, @p offset) names. The + /// browser counts characters inside a text node and children inside an + /// element, either of which may sit under a nested wrapper. + function offsetInRun(run, container, offset) { + var counted = 0; + if (container.nodeType === 1) { + var child = container.firstChild; + for (var i = 0; i < offset && child !== null; ++i) { + counted += child.textContent.length; + child = child.nextSibling; + } + } else { + counted = offset; + } + for (var node = container; node !== run; node = node.parentNode) { + for ( + var previous = node.previousSibling; + previous !== null; + previous = previous.previousSibling + ) { + counted += previous.textContent.length; + } + } + return counted; + } + + /// Where an edit lands, in the terms an operation is written in: a run and + /// an offset into its text. Null outside every run and every paragraph. + function placeOf(container, offset) { + var run = runOf(container); + if (run !== null) { + return { + run: run, + offset: offsetInRun(run, container, offset), + paragraph: paragraphOf(run), + }; + } + + var paragraph = paragraphOf(container); + if (paragraph === null) { + return null; + } + // beside the line box: the end of the last run is where a reader means, + // and a paragraph holding none is where the first run goes + var runs = runsOf(paragraph); + if (runs.length === 0) { + return { run: null, offset: 0, paragraph: paragraph }; + } + var last = runs[runs.length - 1]; + return { + run: last, + offset: last.textContent.length, + paragraph: paragraph, + }; + } + + /// The range an event states, or the selection where it states none - which + /// is what a browser lacking `getTargetRanges` leaves us. + function rangeOf(event) { var ranges = typeof event.getTargetRanges === "function" ? event.getTargetRanges() : []; var range = ranges.length === 1 ? ranges[0] : undefined; - if (ranges.length === 0) { - // No target range: a composition, and some browsers for a paste. The - // caret is what the edit will land on. + if (range === undefined) { var selection = window.getSelection(); - if (selection !== null && selection.rangeCount > 0) { - range = selection.getRangeAt(0); + if (selection === null || selection.rangeCount === 0) { + return null; } + range = selection.getRangeAt(0); } - if (range === undefined) { - return { run: null, id: null }; + var start = placeOf(range.startContainer, range.startOffset); + var end = placeOf(range.endContainer, range.endOffset); + if (start === null || end === null) { + return null; + } + return { start: start, end: end }; + } + + // ------------------------------------------------------------------ edits + + /// Replaces what @p at covers with @p text, and answers where the caret + /// then sits. One run, several runs, or several paragraphs - the last case + /// merges what is left of the two ends into one paragraph. + function replaceRange(at, text) { + var start = at.start; + var end = at.end; + + // Everything the range covers has to be expressible before any of it is + // applied, or a refusal would leave half an edit on the page. + var between = + start.paragraph === end.paragraph + ? [] + : paragraphsBetween(start.paragraph, end.paragraph); + if (between === null) { + return null; } - var start = runOf(range.startContainer); + if (start.run !== end.run) { + // the far end has to be a run, and what lies between has to be text + if (start.run === null || end.run === null) { + return null; + } + if (!coversOnlyText(start.run, end.run)) { + return null; + } + } + + if (start.run === null) { + if (text === "") { + return start; + } + // a paragraph holding no run at all: the text opens one + var opened = perform(appendRun(start.paragraph, text)); + return { + run: opened.run, + offset: text.length, + paragraph: start.paragraph, + }; + } + + var head = start.run.textContent.slice(0, start.offset); + var caret = { + run: start.run, + offset: head.length + text.length, + paragraph: start.paragraph, + }; + + if (start.run === end.run) { + perform( + setRunText(start.run, head + text + end.run.textContent.slice(end.offset)) + ); + return caret; + } + + var tail = end.run.textContent.slice(end.offset); + perform(setRunText(start.run, head + text)); + + if (start.paragraph === end.paragraph) { + removeRunsBetween(start.paragraph, start.run, end.run); + perform(setRunText(end.run, tail)); + return caret; + } + + // Several paragraphs: what is left of each end joins, and everything + // between goes whole. + removeRunsBetween(start.paragraph, start.run, null); + removeRunsBetween(end.paragraph, null, end.run); + perform(setRunText(end.run, tail)); + + for (var i = 0; i < between.length; ++i) { + perform(removeElement(between[i])); + } + perform(mergeParagraph(start.paragraph)); + + return caret; + } + + /// Removes the runs of @p paragraph that lie strictly between @p after and + /// @p before; a null end means from the first run, or to the last. + function removeRunsBetween(paragraph, after, before) { + var runs = runsOf(paragraph); + var from = after === null ? 0 : runs.indexOf(after) + 1; + var to = before === null ? runs.length : runs.indexOf(before); + for (var i = from; i < to; ++i) { + perform(removeElement(runs[i])); + } + } + + // What a range may reach over: a run, a wrapper around one, a paragraph of + // them, and the line box. A picture or a table is not, and no sequence of + // operations takes one away. + var reachable = { "X-S": 1, A: 1, "X-P": 1, BR: 1, WBR: 1 }; + + /// Whether everything between @p from and @p to is text. + function coversOnlyText(from, to) { + var probe = document.createRange(); + probe.setStartAfter(from); + probe.setEndBefore(to); + var nodes = probe.cloneContents().querySelectorAll("*"); + for (var i = 0; i < nodes.length; ++i) { + if (reachable[nodes[i].tagName] !== 1) { + return false; + } + } + return true; + } + + /// The paragraphs strictly between @p first and @p last, or null where + /// something that is not a paragraph lies between them - a table, a picture + /// - which is a range no sequence of operations can express. + function paragraphsBetween(first, last) { + if (first.parentNode !== last.parentNode) { + return null; + } + var result = []; + for ( + var at = first.nextElementSibling; + at !== null && at !== last; + at = at.nextElementSibling + ) { + if (at.tagName !== "X-P" || at.getAttribute("data-odr-id") === null) { + return null; + } + result.push(at); + } + return result; + } + + /// Enter: what the caret covers goes, and the paragraph splits where it + /// then sits. Answers where the caret lands, which is the head of the new + /// paragraph. + function splitAt(at) { + var caret = replaceRange(at, ""); + if (caret === null) { + return null; + } + var paragraph = caret.paragraph; + + if (caret.run === null) { + var empty = perform(insertParagraph(paragraph)); + return { paragraph: empty.paragraph, run: null, offset: 0 }; + } + + var after = caret.run; + if (caret.offset >= caret.run.textContent.length) { + // the caret is at the end of its run: nothing has to be cut + } else if (caret.offset === 0) { + var runs = runsOf(paragraph); + var before = runs.indexOf(caret.run) - 1; + after = before < 0 ? null : runs[before]; + } else { + var whole = caret.run.textContent; + perform(setRunText(caret.run, whole.slice(0, caret.offset))); + perform(insertRun(caret.run, "after", whole.slice(caret.offset))); + } + + var split = perform(splitParagraph(paragraph, after)); + var tailRuns = runsOf(split.tail); return { - run: start !== null && start === runOf(range.endContainer) ? start : null, - id: start === null ? null : idOf(start), + paragraph: split.tail, + run: tailRuns.length === 0 ? null : tailRuns[0], + offset: 0, + }; + } + + /// Puts the caret where an edit left it; a paragraph holding no run has + /// nowhere but itself. + function restore(caret) { + if (caret.run !== null) { + placeCaret(caret.run, caret.offset); + return; + } + var range = document.createRange(); + range.setStart(caret.paragraph, 0); + range.collapse(true); + var selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + } + + // ------------------------------------------------------------------- gate + + // Every edit is one of two shapes: a range replaced by some text, or a + // paragraph split where the caret sits. `text` reads what to put in. + var replacing = { + insertText: function (event) { + return event.data === null ? "" : event.data; + }, + insertReplacementText: function (event) { + return event.data === null ? "" : event.data; + }, + deleteContent: empty, + deleteContentBackward: empty, + deleteContentForward: empty, + deleteByCut: empty, + deleteWordBackward: empty, + deleteWordForward: empty, + deleteSoftLineBackward: empty, + deleteSoftLineForward: empty, + deleteHardLineBackward: empty, + deleteHardLineForward: empty, + deleteEntireSoftLine: empty, + }; + + function empty() { + return ""; + } + + // no operation carries a soft line break + var named = { insertLineBreak: "newLine" }; + + // A delete whose range the browser did not state is one character, in the + // direction the key names. The word and line deletes are not on this list: + // guessing where a word ends would take away text the reader did not name. + var extending = { deleteContentBackward: -1, deleteContentForward: 1 }; + + /// The run before @p run, reaching into the paragraph before where it is + /// the first - which is what Backspace at a paragraph start does. + function runBefore(run) { + var paragraph = paragraphOf(run); + var runs = runsOf(paragraph); + var at = runs.indexOf(run); + if (at > 0) { + return { run: runs[at - 1], paragraph: paragraph }; + } + var previous = paragraph.previousElementSibling; + if (previous === null || previous.tagName !== "X-P") { + return null; + } + var before = runsOf(previous); + return before.length === 0 + ? null + : { run: before[before.length - 1], paragraph: previous }; + } + + function runAfter(run) { + var paragraph = paragraphOf(run); + var runs = runsOf(paragraph); + var at = runs.indexOf(run); + if (at + 1 < runs.length) { + return { run: runs[at + 1], paragraph: paragraph }; + } + var next = paragraph.nextElementSibling; + if (next === null || next.tagName !== "X-P") { + return null; + } + var after = runsOf(next); + return after.length === 0 ? null : { run: after[0], paragraph: next }; + } + + /// Grows a collapsed range by the one character a delete key takes, or by + /// the paragraph boundary it stands at - which merges and takes no + /// character. Null where there is nothing to take. + function extendForDelete(at, direction) { + if (at.start.run !== at.end.run || at.start.offset !== at.end.offset) { + return at; + } + var place = at.start; + if (place.run === null) { + return null; + } + if (direction < 0) { + if (place.offset > 0) { + return { + start: { run: place.run, offset: place.offset - 1, paragraph: place.paragraph }, + end: place, + }; + } + var before = runBefore(place.run); + if (before === null) { + return null; + } + // at the start of a paragraph the boundary itself is what goes, so the + // range takes no character with it + var back = before.paragraph === place.paragraph ? 1 : 0; + return { + start: { + run: before.run, + offset: before.run.textContent.length - back, + paragraph: before.paragraph, + }, + end: place, + }; + } + if (place.offset < place.run.textContent.length) { + return { + start: place, + end: { run: place.run, offset: place.offset + 1, paragraph: place.paragraph }, + }; + } + var after = runAfter(place.run); + if (after === null) { + return null; + } + var forward = after.paragraph === place.paragraph ? 1 : 0; + return { + start: place, + end: { run: after.run, offset: forward, paragraph: after.paragraph }, }; } @@ -95,83 +750,149 @@ if (event.cancelable) { event.preventDefault(); } - // The id keeps two refusals apart, so Enter in one run and then in - // another is heard twice. - odr.editing.refuse(reason, { id: at.id }); + // the id keeps two refusals apart, so the same key in one run and then in + // another is heard twice + odr.editing.refuse(reason, { + id: at === null || at.start.run === null ? null : idOf(at.start.run), + }); } - // Where the edit the gate just allowed will land, for `input` to record. - var pending = null; + // 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; + + root.addEventListener("compositionstart", function () { + var selection = window.getSelection(); + composing = + selection === null || selection.rangeCount === 0 + ? null + : runOf(selection.getRangeAt(0).startContainer); + }); + + root.addEventListener("compositionend", function () { + var run = composing; + composing = null; + if (!odr.editing.isEnabled()) { + return; + } + 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(9, "an edit landed where no operation can name it"); + return; + } + // whatever the browser built inside the run, its text is the operation + perform(setRunText(target, target.textContent)); + }); root.addEventListener("beforeinput", function (event) { - pending = null; - var at = target(event); + var type = event.inputType; + var at = rangeOf(event); + if (!odr.editing.isEnabled()) { refuse(event, "readOnly", at); return; } - if (!textual[event.inputType]) { - refuse(event, named[event.inputType] || "unsupportedEdit", at); + + if (type === "historyUndo" || type === "historyRedo") { + event.preventDefault(); + if (type === "historyUndo") { + odr.editing.undo(); + } else { + odr.editing.redo(); + } return; } - // Its own stack is what the browser replays; there is nothing to address. - if (event.inputType === "historyUndo" || event.inputType === "historyRedo") { + + // mid-composition and unstoppable; `compositionend` reconciles it + if (type === "insertCompositionText" || composing !== null) { return; } - if (at.run === null) { + + if (at === null) { refuse(event, "range", at); return; } - pending = at.run; - if (event.inputType === "insertFromPaste") { - // Whatever the clipboard holds, one run takes plain text on one line. - var text = event.dataTransfer + + if (type === "insertParagraph") { + var split = splitAt(at); + if (split === null) { + refuse(event, "range", at); + return; + } + event.preventDefault(); + restore(split); + return; + } + + if (type === "insertFromPaste") { + var pasted = event.dataTransfer ? event.dataTransfer.getData("text/plain") : null; - if (text === null) { + if (pasted === null) { refuse(event, "unsupportedEdit", at); - } else if (/[\r\n]/.test(text)) { - refuse(event, "newLine", at); - } else { - event.preventDefault(); - document.execCommand("insertText", false, text); + return; } + if (!paste(at, pasted)) { + refuse(event, "range", at); + return; + } + event.preventDefault(); + return; } - }); - /// The run the caret sits in, for an edit no `beforeinput` announced. - function selectedRun() { - var selection = window.getSelection(); - return selection === null || selection.rangeCount === 0 - ? null - : runOf(selection.getRangeAt(0).startContainer); - } + var text = replacing[type]; + if (text === undefined) { + refuse(event, named[type] || "unsupportedEdit", at); + return; + } - // `input` is the browser saying it applied an edit, which a script rewriting - // the page never raises - so a search highlighting nine matches leaves the - // log alone. A `MutationObserver` could not tell the two apart. - root.addEventListener("input", function () { - if (!odr.editing.isEnabled()) { + var covering = extending[type] === undefined + ? at + : extendForDelete(at, extending[type]); + if (covering === null) { + // nothing to take: the key does nothing rather than being refused + event.preventDefault(); return; } - // The caret first: the browser has just put it where the edit landed. A - // `beforeinput` whose edit changed nothing raises no `input`, so what it - // left in `pending` may be a run ago - it is the fallback, not the answer. - var run = selectedRun(); - if (run === null) { - run = pending; - } - pending = null; - if (run === null) { - // The gate refuses an edit it cannot name, so this is a hole in it: a - // WebView that raised no `beforeinput`, or a composition it cannot stop. - odr.onError(9, "an edit landed where no operation can name it"); + + var caret = replaceRange(covering, text(event)); + if (caret === null) { + refuse(event, "range", at); return; } - modified[idOf(run)] = run; - odr.editing.changed(); + event.preventDefault(); + restore(caret); }); + /// A paste is its lines: the first replaces the selection, each one after + /// it opens a paragraph. + function paste(at, pasted) { + var lines = pasted.split(/\r\n|\r|\n/); + var caret = replaceRange(at, lines[0]); + if (caret === null) { + return false; + } + for (var i = 1; i < lines.length; ++i) { + caret = splitAt({ start: caret, end: caret }); + if (caret !== null && lines[i] !== "") { + caret = replaceRange({ start: caret, end: caret }, lines[i]); + } + if (caret === null) { + // the lines before this one stand; a host replays onto a fresh decode + return false; + } + } + restore(caret); + return true; + } + + // ------------------------------------------------------------------ mode + odr.editing.attach({ enable: function () { root.setAttribute("contenteditable", "true"); @@ -180,8 +901,35 @@ root.removeAttribute("contenteditable"); }, operations: operations, + canUndo: function () { + return done.length > 0; + }, + canRedo: function () { + return undone.length > 0; + }, + undo: function () { + if (done.length === 0) { + return false; + } + var step = done.pop(); + step.revert(); + undone.push(step); + odr.editing.changed(); + return true; + }, + redo: function () { + if (undone.length === 0) { + return false; + } + var step = undone.pop(); + step.apply(); + done.push(step); + odr.editing.changed(); + return true; + }, committed: function () { - modified = {}; + done.length = 0; + undone.length = 0; }, }); })(); diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 01dd16a86..6e484d6ce 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -663,6 +663,17 @@ class ElementAdapter final : public AdapterBase { return new_id; } + [[nodiscard]] ElementIdentifier + element_append_text(const ElementIdentifier element_id, + const std::string &text) const override { + pugi::xml_node node = get_node(element_id); + const NodeSpan span = write_text_nodes(node, {}, text); + const auto &[new_id, unused_element, unused_text] = + m_registry->create_text_element(span.first, span.last); + m_registry->append_child(element_id, new_id); + return new_id; + } + void element_remove(const ElementIdentifier element_id) const override { TreeEditor(*m_registry).remove(element_id); } diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.cpp b/src/odr/internal/ooxml/text/ooxml_text_document.cpp index f7b25943b..0ccaeb489 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.cpp @@ -276,6 +276,17 @@ class ElementAdapter final : public AdapterBase { return new_id; } + [[nodiscard]] ElementIdentifier + element_append_text(const ElementIdentifier element_id, + const std::string &text) const override { + pugi::xml_node node = get_node(element_id); + const NodeSpan span = write_text_nodes(node, {}, text); + const auto &[new_id, unused_element, unused_text] = + m_registry->create_text_element(span.first, span.last); + m_registry->append_child(element_id, new_id); + return new_id; + } + void element_remove(const ElementIdentifier element_id) const override { TreeEditor(*m_registry).remove(element_id); } diff --git a/test/browser/text/README.md b/test/browser/text/README.md index 89ac6e404..bf20c1287 100644 --- a/test/browser/text/README.md +++ b/test/browser/text/README.md @@ -1,6 +1,6 @@ # text editing checks -What the emitted text editor allows and what it refuses can only be seen in a +What the emitted text editor does and what it refuses can only be seen in a browser, so these are run by hand rather than by `odr_test`. ```bash @@ -13,35 +13,45 @@ open http://localhost:8734/tests.html what runs is the file the library embeds. `editing.js` goes first, as the library writes it. -- **`tests.html`** — the mode makes the **whole view** editable, and the editor - refuses what it cannot replay. The fixture holds the shapes that decision - turns on: two runs beside each other, a run under a link, a run under a - style-only wrapper, and a paragraph holding a picture and no run at all. The - checks drive `beforeinput`, which is what a browser fires before it changes - anything, so a prevented one is an edit that never happened. +- **`tests.html`** — the editor **owns the edit**: it cancels what the browser + was about to do and splices the page itself, so the markup stays what the + renderer wrote and every change has an operation behind it. The fixture holds + the shapes that turns on: two runs beside each other, a run under a link, a + run under a style-only wrapper, a paragraph holding a picture and no run at + all, and the `` / `
` line box the renderer ends every paragraph + with. The checks drive `beforeinput`, which is what a browser fires before it + changes anything. Why the checks look the way they do: +- **`defaultPrevented` no longer says whether an edit was taken.** The editor + cancels the event either way — once because it is doing the edit itself, once + because it is refusing. So `input()` answers `"taken"` or `"refused"` by + watching the refusal channel, and the cancelling is checked once on its own. +- **The page is rebuilt between groups.** Every edit is a real edit, so a group + that ran before would decide what the next one starts from. `reset()` puts + the fixture back, clears the log and turns the mode on again. - **A synthetic `InputEvent` carries no target range.** `getTargetRanges()` is empty on an event the page constructs, so the editor falls back to the - selection — which is also what a browser lacking `getTargetRanges` gives it. - That is why each check sets the selection first. + selection — which is also what a browser lacking `getTargetRanges` gives it, + and what an Android WebView is reported to give it. That is why each check + sets the selection first, and it is the path `extendForDelete` exists for: a + Backspace whose range the browser did not state is one character, or the + paragraph boundary the caret stands at. +- **The word and line deletes are not extended.** Where a browser states no + range for `deleteWordBackward`, guessing where the word ends would take away + text the reader did not name, so nothing happens. - **The repeat suppression is part of the contract.** Two identical refusals within two seconds are one event ([`editing.md`](../../docs/design/editing.md) - decision 9), and a refusal is keyed by its run — so the two `newLine` checks - sit in different runs, and the one outside every run asserts the prevented - edit rather than a second event. -- **The gate and the collection are driven apart.** `beforeinput` is what - refuses; `input` is what collects, because a script rewriting the page raises - none — which is how a search highlighting nine matches leaves the log alone. - No script can raise the pair the way a key does, so a check drives one or the - other. + decision 9), and a refusal is keyed by its run. +- **The log is checked, not only the page.** What a save hands to + `Document::edit` is the point of the editor, so each group asserts the + operations as well as the text: which ops, in which order, naming which ids. + `document_edit_test.cpp` replays the same shapes in C++, which is what keeps + the two sides from drifting apart. **Scripted editing is not the editing a reader does, which is why no check uses -`execCommand`.** Chrome's scripted path differs from its trusted-input path -twice over: it raises no cancelable `beforeinput`, so -`execCommand("insertParagraph")` splits a paragraph the gate would have -refused; and it dissolves a run whose whole text it replaces, leaving the new -text outside every address. Trusted input does neither — a real Enter is -refused, and typing over a whole run keeps the run, its address and its style. -Both were checked by hand in a browser, and neither is reachable by a reader. +`execCommand`.** Chrome's scripted path raises no cancelable `beforeinput`, so +`execCommand("insertParagraph")` splits a paragraph without the editor ever +seeing it. Trusted input does not; a real Enter goes through the gate. Checked +by hand in a browser, and not reachable by a reader. diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html index 8d4060f15..3513a8bb2 100644 --- a/test/browser/text/tests.html +++ b/test/browser/text/tests.html @@ -12,28 +12,29 @@ > -
+ a run under a style-only wrapper, a paragraph holding a picture and no + run at all, and the `` / `
` line box the renderer ends every + paragraph with. The whole fixture is rebuilt before each group of + checks, because every edit is a real edit. --> +
first run a link and a tail and a tail bold - - third + - + />
@@ -45,19 +46,62 @@ diff --git a/test/src/document_edit_test.cpp b/test/src/document_edit_test.cpp index 567fc0571..57e89e3f0 100644 --- a/test/src/document_edit_test.cpp +++ b/test/src/document_edit_test.cpp @@ -609,3 +609,27 @@ TEST(DocumentEdit, a_paragraph_edit_refuses_another_documents_element) { EXPECT_THROW((void)document.split_paragraph(paragraph, Element()), std::invalid_argument); } + +/// The paragraph Enter just made holds no run to sit beside, so the operation +/// names the paragraph itself. +TEST(DocumentEdit, a_run_is_appended_into_a_paragraph_that_holds_none) { + const Document document = two_paragraph_text(); + + document.edit(ops(R"({"op":"insertParagraph","after":)" + + id_of(paragraph_at(document, 0)) + R"(,"id":-1},)" + + R"({"op":"insertText","parent":-1,"text":"typed",)" + R"("id":-2})")); + + EXPECT_EQ(paragraph_texts(document), + (std::vector{"one two three", "typed", "second"})); +} + +TEST(DocumentEdit, an_insert_naming_a_parent_and_a_run_to_sit_beside_refuses) { + const Document document = two_paragraph_text(); + + EXPECT_THROW(document.edit(ops( + R"({"op":"insertText","parent":)" + + id_of(paragraph_at(document, 0)) + R"(,"after":)" + + id_of(run_at(document, 0, 0)) + R"(,"text":"x","id":-1})")), + std::invalid_argument); +}