Fix collaborative undo correctness (fixes #2105) (#2500)

modules/history.js

@@ -1,5 +1,4 @@
 import { Scope } from 'parchment';
-import Delta, { Op } from 'quill-delta';
 import Quill from '../core/quill';
 import Module from '../core/module';
 
@@ -39,12 +38,14 @@ class History extends Module {
   change(source, dest) {
     if (this.stack[source].length === 0) return;
     const delta = this.stack[source].pop();
-    this.stack[dest].push(delta);
+    const base = this.quill.getContents();
+    const inverseDelta = delta.invert(base);
+    this.stack[dest].push(inverseDelta);
     this.lastRecorded = 0;
     this.ignoreChange = true;
-    this.quill.updateContents(delta[source], Quill.sources.USER);
+    this.quill.updateContents(delta, Quill.sources.USER);
     this.ignoreChange = false;
-    const index = getLastChangeIndex(this.quill.scroll, delta[source]);
+    const index = getLastChangeIndex(this.quill.scroll, delta);
     this.quill.setSelection(index);
   }
 
@@ -59,25 +60,19 @@ class History extends Module {
   record(changeDelta, oldDelta) {
     if (changeDelta.ops.length === 0) return;
     this.stack.redo = [];
-    let undoDelta = guessUndoDelta(changeDelta);
-    if (undoDelta == null) {
-      undoDelta = this.quill.getContents().diff(oldDelta);
-    }
+    let undoDelta = changeDelta.invert(oldDelta);
     const timestamp = Date.now();
     if (
       this.lastRecorded + this.options.delay > timestamp &&
       this.stack.undo.length > 0
     ) {
       const delta = this.stack.undo.pop();
-      undoDelta = undoDelta.compose(delta.undo);
-      changeDelta = delta.redo.compose(changeDelta);
+      undoDelta = undoDelta.compose(delta);
     } else {
       this.lastRecorded = timestamp;
     }
-    this.stack.undo.push({
-      redo: changeDelta,
-      undo: undoDelta,
-    });
+    if (undoDelta.length() === 0) return;
+    this.stack.undo.push(undoDelta);
     if (this.stack.undo.length > this.options.maxStack) {
       this.stack.undo.shift();
     }
@@ -88,14 +83,8 @@ class History extends Module {
   }
 
   transform(delta) {
-    this.stack.undo.forEach(change => {
-      change.undo = delta.transform(change.undo, true);
-      change.redo = delta.transform(change.redo, true);
-    });
-    this.stack.redo.forEach(change => {
-      change.undo = delta.transform(change.undo, true);
-      change.redo = delta.transform(change.redo, true);
-    });
+    transformStack(this.stack.undo, delta);
+    transformStack(this.stack.redo, delta);
   }
 
   undo() {
@@ -108,6 +97,18 @@ History.DEFAULTS = {
   userOnly: false,
 };
 
+function transformStack(stack, delta) {
+  let remoteDelta = delta;
+  for (let i = stack.length - 1; i >= 0; i -= 1) {
+    const oldDelta = stack[i];
+    stack[i] = remoteDelta.transform(oldDelta, true);
+    remoteDelta = oldDelta.transform(remoteDelta);
+    if (stack[i].length() === 0) {
+      stack.splice(i, 1);
+    }
+  }
+}
+
 function endsWithNewlineChange(scroll, delta) {
   const lastOp = delta.ops[delta.ops.length - 1];
   if (lastOp == null) return false;
@@ -133,21 +134,4 @@ function getLastChangeIndex(scroll, delta) {
   return changeIndex;
 }
 
-function guessUndoDelta(delta) {
-  const undoDelta = new Delta();
-  let failed = false;
-  delta.forEach(op => {
-    if (op.insert) {
-      undoDelta.delete(Op.length(op));
-    } else if (op.retain && op.attributes == null) {
-      undoDelta.retain(op.retain);
-    } else {
-      failed = true;
-      return false;
-    }
-    return true;
-  });
-  return failed ? null : undoDelta;
-}
-
 export { History as default, getLastChangeIndex };

package.json

@@ -37,7 +37,7 @@
     "eventemitter3": "^3.1.0",
     "extend": "^3.0.2",
     "parchment": "2.0.0-dev.0",
-    "quill-delta": "4.1.0"
+    "quill-delta": "4.2.0"
   },
   "devDependencies": {
     "@babel/core": "^7.2.2",

test/unit/modules/history.js

@@ -214,5 +214,43 @@ describe('History', function() {
       this.quill.history.redo();
       expect(this.quill.getText()).toEqual('abcd\n');
     });
+
+    it('correctly transform against remote changes', function() {
+      this.quill.history.options.delay = 0;
+      this.quill.history.options.userOnly = true;
+      this.quill.setText('b\n');
+      this.quill.insertText(1, 'd', Quill.sources.USER);
+      this.quill.insertText(0, 'a', Quill.sources.USER);
+      this.quill.insertText(2, 'c', Quill.sources.API);
+      expect(this.quill.getText()).toEqual('abcd\n');
+      this.quill.history.undo();
+      expect(this.quill.getText()).toEqual('bcd\n');
+      this.quill.history.undo();
+      expect(this.quill.getText()).toEqual('bc\n');
+      this.quill.history.redo();
+      expect(this.quill.getText()).toEqual('bcd\n');
+      this.quill.history.redo();
+      expect(this.quill.getText()).toEqual('abcd\n');
+    });
+
+    it('correctly transform against remote changes breaking up an insert', function() {
+      this.quill.history.options.delay = 0;
+      this.quill.history.options.userOnly = true;
+      this.quill.setText('\n');
+      this.quill.insertText(0, 'ABC', Quill.sources.USER);
+      this.quill.insertText(3, '4', Quill.sources.API);
+      this.quill.insertText(2, '3', Quill.sources.API);
+      this.quill.insertText(1, '2', Quill.sources.API);
+      this.quill.insertText(0, '1', Quill.sources.API);
+      expect(this.quill.getText()).toEqual('1A2B3C4\n');
+      this.quill.history.undo();
+      expect(this.quill.getText()).toEqual('1234\n');
+      this.quill.history.redo();
+      expect(this.quill.getText()).toEqual('1A2B3C4\n');
+      this.quill.history.undo();
+      expect(this.quill.getText()).toEqual('1234\n');
+      this.quill.history.redo();
+      expect(this.quill.getText()).toEqual('1A2B3C4\n');
+    });
   });
 });