Problem
In the document editor (src/odr/internal/html/frontend/document.js), one undo does not take back one edit. If the reader edits inside a run and then presses undo, the page goes through intermediate states that the reader never made.
Cause
perform() pushes each step onto done as its own undo entry (document.js:101-110). But one beforeinput often performs several steps:
- Enter inside a run.
splitAt shortens the run, inserts its tail as a new run, and then splits the paragraph (document.js:1156-1160). That is three undo entries.
- Formatting part of a run.
markRange cuts the run at both ends and then styles each run (document.js:673-689).
- Typing over a selection that covers several runs or paragraphs.
replaceRange sets the text of both ends, removes what lies between, and merges the paragraphs (document.js:1022-1039).
undo() and redo() pop one entry at a time (document.js:1519-1538). So each undo reverts only a part of the edit, and the page shows the state between two parts.
This goes against the design note: docs/design/document-editing.md:140 says "One beforeinput is one undo step". The browser test codifies the current behaviour: test/browser/text/tests.html:378-379 expects two more undo steps after a split ("undo goes on to the run that was cut", "and to the run that was shortened").
Steps to reproduce
- Open an ODT and turn editing on.
- Put the caret in the middle of a run.
- Press Enter.
- Press Ctrl+Z three times.
Expected: the first Ctrl+Z puts the paragraph back as it was, and there is nothing more to undo.
Actual: the first Ctrl+Z joins the paragraphs, but the text stays cut into two runs. The next two Ctrl+Z take back the new run and the shortened run one after the other.
Possible fix
Give every step that one event handler performs the same gesture number, and let undo() and redo() take back or replay all steps of that gesture. A microtask can close the gesture after the handler returns, so no call site has to change:
var gesture = 0;
var gestureOpen = false;
function perform(step) {
if (step === null) {
return null;
}
if (!gestureOpen) {
gestureOpen = true;
gesture += 1;
Promise.resolve().then(function () {
gestureOpen = false;
});
}
step.gesture = gesture;
step.apply();
done.push(step);
undone.length = 0;
odr.editing.changed();
return step;
}
undo() then pops entries while done[done.length - 1].gesture is the same, and redo() does the same on undone. tests.html:378-380 must then expect that the second undo returns false.
Found while testing the editor on opendocument.app with a local 7.0.0 build.
Problem
In the document editor (
src/odr/internal/html/frontend/document.js), one undo does not take back one edit. If the reader edits inside a run and then presses undo, the page goes through intermediate states that the reader never made.Cause
perform()pushes each step ontodoneas its own undo entry (document.js:101-110). But onebeforeinputoften performs several steps:splitAtshortens the run, inserts its tail as a new run, and then splits the paragraph (document.js:1156-1160). That is three undo entries.markRangecuts the run at both ends and then styles each run (document.js:673-689).replaceRangesets the text of both ends, removes what lies between, and merges the paragraphs (document.js:1022-1039).undo()andredo()pop one entry at a time (document.js:1519-1538). So each undo reverts only a part of the edit, and the page shows the state between two parts.This goes against the design note:
docs/design/document-editing.md:140says "Onebeforeinputis one undo step". The browser test codifies the current behaviour:test/browser/text/tests.html:378-379expects two more undo steps after a split ("undo goes on to the run that was cut", "and to the run that was shortened").Steps to reproduce
Expected: the first Ctrl+Z puts the paragraph back as it was, and there is nothing more to undo.
Actual: the first Ctrl+Z joins the paragraphs, but the text stays cut into two runs. The next two Ctrl+Z take back the new run and the shortened run one after the other.
Possible fix
Give every step that one event handler performs the same gesture number, and let
undo()andredo()take back or replay all steps of that gesture. A microtask can close the gesture after the handler returns, so no call site has to change:undo()then pops entries whiledone[done.length - 1].gestureis the same, andredo()does the same onundone.tests.html:378-380must then expect that the second undo returnsfalse.Found while testing the editor on opendocument.app with a local 7.0.0 build.