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);