Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion docs/design/editing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/odr/internal/html/frontend/editing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 8 additions & 6 deletions src/odr/internal/html/frontend/text.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down Expand Up @@ -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 `<mark>` 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 `<mark>` may sit in between.
TextEditor.prototype.getPosition = function (container, offset) {
var line = container;
while (line !== null && line.parentNode !== this.textBody) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
};

Expand Down
28 changes: 28 additions & 0 deletions test/browser/plaintext/tests.html
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading