implement fix for #500. extends #515

package-lock.json

@@ -2481,9 +2481,9 @@
       }
     },
     "node_modules/lib0": {
-      "version": "0.2.72",
+      "version": "0.2.73",
-      "resolved": "https://registry.npmjs.org/lib0/-/lib0-0.2.72.tgz",
+      "resolved": "https://registry.npmjs.org/lib0/-/lib0-0.2.73.tgz",
-      "integrity": "sha512-JPnUxl15tO6jrBASQ92+uDyQzW4ISMhDORq6mLovBYxESEWQCj5SnC8oYkELboGbU1ZqCIEEDwCL6mYqWNzdOA==",
+      "integrity": "sha512-aJJIElCLWnHMcYZPtsM07QoSfHwpxCy4VUzBYGXFYEmh/h2QS5uZNbCCfL0CqnkOE30b7Tp9DVfjXag+3qzZjQ==",
       "dependencies": {
         "isomorphic.js": "^0.2.4"
       },

src/structs/Item.js

@@ -23,11 +23,12 @@ import {
   readContentType,
   addChangedTypeToTransaction,
   isDeleted,
-  DeleteSet, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction // eslint-disable-line
+  StackItem, DeleteSet, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction // eslint-disable-line
 } from '../internals.js'
 
 import * as error from 'lib0/error'
 import * as binary from 'lib0/binary'
+import * as array from 'lib0/array'
 
 /**
  * @todo This should return several items
@@ -120,6 +121,12 @@ export const splitItem = (transaction, leftItem, diff) => {
   return rightItem
 }
 
+/**
+ * @param {Array<StackItem>} stack
+ * @param {ID} id
+ */
+const isDeletedByUndoStack = (stack, id) => array.some(stack, /** @param {StackItem} s */ s => isDeleted(s.deletions, id))
+
 /**
  * Redoes the effect of this operation.
  *
@@ -128,12 +135,13 @@ export const splitItem = (transaction, leftItem, diff) => {
  * @param {Set<Item>} redoitems
  * @param {DeleteSet} itemsToDelete
  * @param {boolean} ignoreRemoteMapChanges
+ * @param {import('../utils/UndoManager.js').UndoManager} um
  *
  * @return {Item|null}
  *
  * @private
  */
-export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemoteMapChanges) => {
+export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemoteMapChanges, um) => {
   const doc = transaction.doc
   const store = doc.store
   const ownClientID = doc.clientID
@@ -153,7 +161,7 @@ export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemo
   // make sure that parent is redone
   if (parentItem !== null && parentItem.deleted === true) {
     // try to undo parent if it will be undone anyway
-    if (parentItem.redone === null && (!redoitems.has(parentItem) || redoItem(transaction, parentItem, redoitems, itemsToDelete, ignoreRemoteMapChanges) === null)) {
+    if (parentItem.redone === null && (!redoitems.has(parentItem) || redoItem(transaction, parentItem, redoitems, itemsToDelete, ignoreRemoteMapChanges, um) === null)) {
       return null
     }
     while (parentItem.redone !== null) {
@@ -200,16 +208,13 @@ export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemo
   } else {
     right = null
     if (item.right && !ignoreRemoteMapChanges) {
-      left = item.right
+      left = item
       // Iterate right while right is in itemsToDelete
       // If it is intended to delete right while item is redone, we can expect that item should replace right.
-      while (left !== null && left.right !== null && isDeleted(itemsToDelete, left.right.id)) {
+      while (left !== null && left.right !== null && (left.right.redone || isDeleted(itemsToDelete, left.right.id) || isDeletedByUndoStack(um.undoStack, left.right.id) || isDeletedByUndoStack(um.redoStack, left.right.id))) {
         left = left.right
-      }
+        // follow redone
-      // follow redone
+        while (left.redone) left = getItemCleanStart(transaction, left.redone)
-      // trace redone until parent matches
-      while (left !== null && left.redone !== null) {
-        left = getItemCleanStart(transaction, left.redone)
       }
       if (left && left.right !== null) {
         // It is not possible to redo this item because it conflicts with a

src/utils/UndoManager.js

@@ -17,7 +17,7 @@ import * as time from 'lib0/time'
 import * as array from 'lib0/array'
 import { Observable } from 'lib0/observable'
 
-class StackItem {
+export class StackItem {
   /**
    * @param {DeleteSet} deletions
    * @param {DeleteSet} insertions
@@ -101,7 +101,7 @@ const popStackItem = (undoManager, stack, eventType) => {
         }
       })
       itemsToRedo.forEach(struct => {
-        performedChange = redoItem(transaction, struct, itemsToRedo, stackItem.insertions, undoManager.ignoreRemoteMapChanges) !== null || performedChange
+        performedChange = redoItem(transaction, struct, itemsToRedo, stackItem.insertions, undoManager.ignoreRemoteMapChanges, undoManager) !== null || performedChange
       })
       // We want to delete in reverse order so that children are deleted before
       // parents, so we have more information available when items are filtered.

tests/undo-redo.tests.js

@@ -654,7 +654,6 @@ export const testSpecialDeletionCase = tc => {
 export const testUndoDeleteInMap = (tc) => {
   const { map0 } = init(tc, { users: 3 })
   const undoManager = new Y.UndoManager(map0, { captureTimeout: 0 })
-
   map0.set('a', 'a')
   map0.delete('a')
   map0.set('a', 'b')
@@ -662,24 +661,17 @@ export const testUndoDeleteInMap = (tc) => {
   map0.set('a', 'c')
   map0.delete('a')
   map0.set('a', 'd')
-
   t.compare(map0.toJSON(), { a: 'd' })
-
   undoManager.undo()
   t.compare(map0.toJSON(), {})
-
   undoManager.undo()
   t.compare(map0.toJSON(), { a: 'c' })
-
   undoManager.undo()
   t.compare(map0.toJSON(), {})
-
   undoManager.undo()
   t.compare(map0.toJSON(), { a: 'b' })
-
   undoManager.undo()
   t.compare(map0.toJSON(), {})
-
   undoManager.undo()
   t.compare(map0.toJSON(), { a: 'a' })
 }