From 24eddb2d75cc9e37b0e561cb068bf0f90b0d14e5 Mon Sep 17 00:00:00 2001
From: Kevin Jahns <kevin.jahns@pm.me>
Date: Fri, 4 Feb 2022 12:41:13 +0100
Subject: [PATCH] fix concurrent formatting / cleanup bug

---
 src/types/YText.js    |  6 +++---
 tests/y-text.tests.js | 15 ++++++++++++---
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/types/YText.js b/src/types/YText.js
index c0339003..ae6474b9 100644
--- a/src/types/YText.js
+++ b/src/types/YText.js
@@ -369,7 +369,7 @@ const cleanupFormattingGap = (transaction, start, curr, startAttributes, currAtt
             // 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) {
+            if (!reachedEndOfCurr && (currAttributes.get(key) || null) === value && (startAttributes.get(key) || null) !== value) {
               currAttributes.delete(key)
             }
           }
@@ -693,7 +693,7 @@ export class YTextEvent extends YEvent {
                     } else {
                       attributes[key] = value
                     }
-                  } else {
+                  } else if (value !== null) {
                     item.delete(transaction)
                   }
                 }
@@ -719,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)
                   }
                 }
diff --git a/tests/y-text.tests.js b/tests/y-text.tests.js
index 7a8c448a..cf60a106 100644
--- a/tests/y-text.tests.js
+++ b/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 } }]])
 }
 
 /**