From 5db6405e213b33aafa217e98a1a5e26df78e08c5 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 10 Sep 2026 17:15:30 +0200 Subject: [PATCH] fix(html): stray line numbers, and two refusals that described the old editor Both found by using the thing. Deleting across several lines of a `.txt` left a line number behind per line. `removeText` took `textNr.lastChild` away, and the renderer writes whitespace between the number cells - so what went was a text node, not a cell. `lastElementChild` is what reaches one, which the comment above `getPosition` already said about the lines and nobody carried across to the gutter. Nine lines deleted down to seven left eight numbers; a check now drives that. And two refusal messages still described the editor as it was before it could span runs. Code 1 said "new line not supported by this document" though Enter has been taken since then, and code 8 said "an edit has to lie inside one run of text" though it now marks a range reaching over a picture or a table - which reads as though editing across runs does not work, when it does. The codes are untouched; only what they say changed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KKFKbUVCYF2VhujdmjhhPW --- CHANGELOG.md | 9 ++++++++ docs/design/editing.md | 3 ++- src/odr/internal/html/frontend/editing.js | 8 ++++--- src/odr/internal/html/frontend/text.js | 14 +++++++----- test/browser/plaintext/tests.html | 28 +++++++++++++++++++++++ 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8031125d..dce85b746 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,15 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Fix**: deleting across several lines of a `.txt` left stray line numbers + in the gutter. The renderer writes whitespace between the number cells, so + `lastChild` there was a text node and the removal took that instead of a cell. + +- **Fix**: two refusal messages described what their code meant before the + editor could span runs. Code 1 said "new line not supported" though Enter is + taken, and code 8 said "an edit has to lie inside one run of text" though it + now marks a range reaching over a picture. The codes are unchanged. + - **Fix**: `odr.editing.getOperations()` and `odr.generateDiff()` stated `"version": 1` while `Document::edit` takes 2, so every save the browser produced was refused. A check page now asserts the version. diff --git a/docs/design/editing.md b/docs/design/editing.md index d9ef00905..e09736a1c 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -346,8 +346,9 @@ then intercepts `beforeinput` and takes the edits it can express as operations: | 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 | +| a range reaching over a picture or a table | refused, reason `range` - `replaceRange` takes runs and whole paragraphs away, so anything else caught between the ends would survive while the text around it went | | anything else the browser offers (a mark, a list, a drop) | refused, reason `unsupportedEdit` | -| an edit reaching over a picture or a table, or landing outside every run | refused, reason `range` | +| an edit 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 diff --git a/src/odr/internal/html/frontend/editing.js b/src/odr/internal/html/frontend/editing.js index df36c9099..66f62b975 100644 --- a/src/odr/internal/html/frontend/editing.js +++ b/src/odr/internal/html/frontend/editing.js @@ -16,16 +16,18 @@ var lastRefusal = null; // One space of codes, appended and never renumbered - `odr.onError` shares - // it, and holds 9. The host maps the code; the message is for a console. + // it, and holds 9. The host maps the code; the message is for a console, and + // says what the code means today rather than what it meant when it was + // added. var refusals = { - newLine: { code: 1, message: "new line not supported by this document" }, + newLine: { code: 1, message: "a line break inside a paragraph is not supported" }, formula: { code: 2, message: "cell holds a formula" }, rich: { code: 3, message: "cell holds more than one plain run" }, shapes: { code: 4, message: "cell holds a drawing" }, readOnly: { code: 5, message: "document cannot be edited" }, formulaInput: { code: 6, message: "typing a formula is not supported" }, unsupportedEdit: { code: 7, message: "this kind of edit is not supported" }, - range: { code: 8, message: "an edit has to lie inside one run of text" }, + range: { code: 8, message: "an edit cannot reach over a picture or a table" }, }; odr.onError = function (code, message) { diff --git a/src/odr/internal/html/frontend/text.js b/src/odr/internal/html/frontend/text.js index 057f3d4a0..d9d0d654a 100644 --- a/src/odr/internal/html/frontend/text.js +++ b/src/odr/internal/html/frontend/text.js @@ -27,7 +27,7 @@ self.textNr.appendChild(nrCell); } for (var j = nrCount; j > lineCount; --j) { - self.textNr.removeChild(self.textNr.lastChild); + self.textNr.removeChild(self.textNr.lastElementChild); } self.updateLineNumberHeight(); }); @@ -97,8 +97,10 @@ // Lines are the element children: formatted output puts a whitespace text // node between them, and counting or indexing those as lines is off by as - // much as a factor of two. The line is the ancestor the body owns and the - // offset is measured from its start: a search `` may sit in between. + // much as a factor of two. The gutter is written the same way, which is why + // a cell is reached by `lastElementChild` and never by `lastChild`. The line + // is the ancestor the body owns and the offset is measured from its start: a + // search `` may sit in between. TextEditor.prototype.getPosition = function (container, offset) { var line = container; while (line !== null && line.parentNode !== this.textBody) { @@ -210,9 +212,9 @@ ); line = line.nextElementSibling; - this.textNr.appendChild(document.createElement("div")); // the line is already in, so the count is the number the cell gets - this.textNr.lastChild.textContent = String(this.textBody.children.length); + this.textNr.appendChild(document.createElement("div")).textContent = + String(this.textBody.children.length); } if (i === 0) { @@ -242,7 +244,7 @@ for (var lineNr = from.line + 1; lineNr <= to.line; ++lineNr) { this.textBody.removeChild(firstLine.nextElementSibling); - this.textNr.removeChild(this.textNr.lastChild); + this.textNr.removeChild(this.textNr.lastElementChild); } }; diff --git a/test/browser/plaintext/tests.html b/test/browser/plaintext/tests.html index 698019b38..3ac3dd1fa 100644 --- a/test/browser/plaintext/tests.html +++ b/test/browser/plaintext/tests.html @@ -144,6 +144,34 @@ check("and joins the two", lines() === "firstXY line|second line|third line"); check("the gutter follows back", gutter.children.length === 3); + check( + "the gutter is written with whitespace between the cells", + gutter.lastChild.nodeType !== 1 + ); + + // Deleting across lines: the gutter has to lose one cell per line, and + // `lastChild` there is whitespace rather than a cell. + caretAt(0, 1); + (function () { + var r = document.createRange(); + r.setStart(body.children[0].firstChild, 1); + r.setEnd(body.children[2].firstChild, 1); + var s = window.getSelection(); + s.removeAllRanges(); + s.addRange(r); + })(); + check( + "deleting across three lines is taken", + input("deleteContentBackward") === "taken" + ); + check("the lines go", body.children.length === 1); + check("and the numbers with them", gutter.children.length === 1); + check("undo brings both back", odr.editing.undo() === true); + check( + "in step", + body.children.length === 3 && gutter.children.length === 3 + ); + // ---------------------------------------------------------- undo / redo check("undo takes the join back", odr.editing.undo() === true);