13.5.27

Calling cleanupFormattingGap should not make a copy of the
attributes because it needs to be able to update them.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "yjs",
-  "version": "13.5.25",
+  "version": "13.5.27",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "yjs",
-  "version": "13.5.25",
+  "version": "13.5.27",
   "description": "Shared Editing Library",
   "main": "./dist/yjs.cjs",
   "module": "./dist/yjs.mjs",

src/types/YText.js

@@ -338,14 +338,16 @@ const formatText = (transaction, parent, currPos, length, attributes) => {
  *
  * @param {Transaction} transaction
  * @param {Item} start
- * @param {Item|null} end exclusive end, automatically iterates to the next Content Item
+ * @param {Item|null} curr exclusive end, automatically iterates to the next Content Item
  * @param {Map<string,any>} startAttributes
- * @param {Map<string,any>} endAttributes This attribute is modified!
+ * @param {Map<string,any>} currAttributes
  * @return {number} The amount of formatting Items deleted.
  *
  * @function
  */
-const cleanupFormattingGap = (transaction, start, end, startAttributes, endAttributes) => {
+const cleanupFormattingGap = (transaction, start, curr, startAttributes, currAttributes) => {
+  let end = curr
+  const endAttributes = map.copy(currAttributes)
   while (end && (!end.countable || end.deleted)) {
     if (!end.deleted && end.content.constructor === ContentFormat) {
       updateCurrentAttributes(endAttributes, /** @type {ContentFormat} */ (end.content))
@@ -353,7 +355,11 @@ const cleanupFormattingGap = (transaction, start, end, startAttributes, endAttri
     end = end.right
   }
   let cleanups = 0
+  let reachedEndOfCurr = false
   while (start !== end) {
+    if (curr === start) {
+      reachedEndOfCurr = true
+    }
     if (!start.deleted) {
       const content = start.content
       switch (content.constructor) {
@@ -363,6 +369,9 @@ const cleanupFormattingGap = (transaction, start, end, startAttributes, endAttri
             // Either this format is overwritten or it is not necessary because the attribute already existed.
             start.delete(transaction)
             cleanups++
+            if (!reachedEndOfCurr && (currAttributes.get(key) || null) === value && (startAttributes.get(key) || null) !== value) {
+              currAttributes.delete(key)
+            }
           }
           break
         }
@@ -465,7 +474,7 @@ const deleteText = (transaction, currPos, length) => {
     currPos.forward()
   }
   if (start) {
-    cleanupFormattingGap(transaction, start, currPos.right, startAttrs, map.copy(currPos.currentAttributes))
+    cleanupFormattingGap(transaction, start, currPos.right, startAttrs, currPos.currentAttributes)
   }
   const parent = /** @type {AbstractType<any>} */ (/** @type {Item} */ (currPos.left || currPos.right).parent)
   if (parent._searchMarker) {
@@ -684,7 +693,7 @@ export class YTextEvent extends YEvent {
                     } else {
                       attributes[key] = value
                     }
-                  } else {
+                  } else if (value !== null) {
                     item.delete(transaction)
                   }
                 }
@@ -710,7 +719,7 @@ export class YTextEvent extends YEvent {
                     } else {
                       attributes[key] = value
                     }
-                  } else {
+                  } else if (attr !== null) { // this will be cleaned up automatically by the contextless cleanup function
                     item.delete(transaction)
                   }
                 }

src/utils/UndoManager.js

@@ -186,6 +186,7 @@ export class UndoManager extends Observable {
         }
       })
       const now = time.getUnixTime()
+      let didAdd = false
       if (now - this.lastChange < captureTimeout && stack.length > 0 && !undoing && !redoing) {
         // append change to last stack op
         const lastOp = stack[stack.length - 1]
@@ -194,6 +195,7 @@ export class UndoManager extends Observable {
       } else {
         // create a new stack op
         stack.push(new StackItem(transaction.deleteSet, insertions))
+        didAdd = true
       }
       if (!undoing && !redoing) {
         this.lastChange = now
@@ -204,7 +206,9 @@ export class UndoManager extends Observable {
           keepItem(item, true)
         }
       })
-      this.emit('stack-item-added', [{ stackItem: stack[stack.length - 1], origin: transaction.origin, type: undoing ? 'redo' : 'undo', changedParentTypes: transaction.changedParentTypes }, this])
+      if (didAdd) {
+        this.emit('stack-item-added', [{ stackItem: stack[stack.length - 1], origin: transaction.origin, type: undoing ? 'redo' : 'undo', changedParentTypes: transaction.changedParentTypes }, this])
+      }
     })
   }
 

tests/y-text.tests.js

@@ -6,6 +6,10 @@ import * as math from 'lib0/math'
 const { init, compare } = Y
 
 /**
+ * In this test we are mainly interested in the cleanup behavior and whether the resulting delta makes sense.
+ * It is fine if the resulting delta is not minimal. But applying the delta to a rich-text editor should result in a
+ * synced document.
+ *
  * @param {t.TestCase} tc
  */
 export const testDeltaAfterConcurrentFormatting = tc => {
@@ -14,12 +18,17 @@ export const testDeltaAfterConcurrentFormatting = tc => {
   testConnector.flushAllMessages()
   text0.format(0, 3, { bold: true })
   text1.format(2, 2, { bold: true })
-  let delta = null
+  /**
+   * @type {any}
+   */
+  const deltas = []
   text1.observe(event => {
-    delta = event.delta
+    if (event.delta.length > 0) {
+      deltas.push(event.delta)
+    }
   })
   testConnector.flushAllMessages()
-  t.compare(delta, [])
+  t.compare(deltas, [[{ retain: 3, attributes: { bold: true } }, { retain: 2, attributes: { bold: null } }]])
 }
 
 /**
@@ -138,6 +147,28 @@ export const testNotMergeEmptyLinesFormat = tc => {
   ])
 }
 
+/**
+ * @param {t.TestCase} tc
+ */
+export const testPreserveAttributesThroughDelete = tc => {
+  const ydoc = new Y.Doc()
+  const testText = ydoc.getText('test')
+  testText.applyDelta([
+    { insert: 'Text' },
+    { insert: '\n', attributes: { title: true } },
+    { insert: '\n' }
+  ])
+  testText.applyDelta([
+    { retain: 4 },
+    { delete: 1 },
+    { retain: 1, attributes: { title: true } }
+  ])
+  t.compare(testText.toDelta(), [
+    { insert: 'Text' },
+    { insert: '\n', attributes: { title: true } }
+  ])
+}
+
 /**
  * @param {t.TestCase} tc
  */