From abf3fab1b6a479d01d688bb0d5dd7678aec695c6 Mon Sep 17 00:00:00 2001 From: dkuhnert <32337621+dkuhnert@users.noreply.github.com> Date: Wed, 23 Feb 2022 14:47:04 +0100 Subject: [PATCH 1/2] cleanup redundant text attributes when delete attributes fixes #392 --- src/types/YText.js | 14 ++++++++++++++ tests/undo-redo.tests.js | 38 ++++++++++++++++++++++++++++++++++++++ tests/y-text.tests.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/src/types/YText.js b/src/types/YText.js index ae6474b9..7e24f3ed 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -164,6 +164,20 @@ const insertNegatedAttributes = (transaction, parent, currPos, negatedAttributes const doc = transaction.doc const ownClientId = doc.clientID negatedAttributes.forEach((val, key) => { + // check if we really need to create attributes + // (the attribute may be set the desired value already) + let n = currPos.right + while( + n !== null && (n.deleted === true || n.content.constructor === ContentFormat) + ) { + if (!n.deleted && equalAttrs(currPos.currentAttributes.get(/** @type {ContentFormat} */ (n.content).key) ?? null, /** @type {ContentFormat} */ (n.content).value)) { + n.delete(transaction) + return + } + n = n.right + } + + // create negated attribute const left = currPos.left const right = currPos.right const nextFormat = new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentFormat(key, val)) diff --git a/tests/undo-redo.tests.js b/tests/undo-redo.tests.js index 40e6a718..4394e9ab 100644 --- a/tests/undo-redo.tests.js +++ b/tests/undo-redo.tests.js @@ -527,3 +527,41 @@ export const testUndoBlockBug = tc => { undoManager.redo() // {"text":{}} t.compare(design.toJSON(), { text: { blocks: { text: '4' } } }) } + +/** + * Undo text formatting delete should not corrupt peer state. + * + * @see https://github.com/yjs/yjs/issues/392 + * @param {t.TestCase} tc + */ +export const testUndoDeleteTextFormat = tc => { + const doc = new Y.Doc() + const text = doc.getText() + text.insert(0, 'Attack ships on fire off the shoulder of Orion.') + const doc2 = new Y.Doc() + const text2 = doc2.getText(); + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + const undoManager = new Y.UndoManager(text) + + text.format(13, 7, { bold: true }) + undoManager.stopCapturing() + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + + text.format(16, 4, { bold: null }) + undoManager.stopCapturing() + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + + undoManager.undo() + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + + const expect = [ + { insert: 'Attack ships ' }, + { + insert: 'on fire', + attributes: { bold: true } + }, + { insert: ' off the shoulder of Orion.' } + ] + t.compare(text.toDelta(), expect) + t.compare(text2.toDelta(), expect) +} diff --git a/tests/y-text.tests.js b/tests/y-text.tests.js index cf60a106..4f2fe090 100644 --- a/tests/y-text.tests.js +++ b/tests/y-text.tests.js @@ -607,6 +607,35 @@ export const testFormattingBug = async tc => { console.log(text1.toDelta()) } +/** + * Delete formatting should not leave redundant formatting items. + * + * @param {t.TestCase} tc + */ +export const testDeleteFormatting = tc => { + const doc = new Y.Doc() + const text = doc.getText() + text.insert(0, 'Attack ships on fire off the shoulder of Orion.') + + const doc2 = new Y.Doc() + const text2 = doc2.getText() + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + + text.format(13, 7, { bold: true }) + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + + text.format(16, 4, { bold: null }) + Y.applyUpdate(doc2, Y.encodeStateAsUpdate(doc)) + + const expected = [ + { insert: 'Attack ships ' }, + { insert: 'on ', attributes: { bold: true } }, + { insert: 'fire off the shoulder of Orion.' } + ] + t.compare(text.toDelta(), expected) + t.compare(text2.toDelta(), expected) +} + // RANDOM TESTS let charCounter = 0 From fddb620d4180e219ec0b711564c95d3587786332 Mon Sep 17 00:00:00 2001 From: dkuhnert <32337621+dkuhnert@users.noreply.github.com> Date: Wed, 23 Feb 2022 18:20:26 +0100 Subject: [PATCH 2/2] cleanup redundant text attributes when delete attributes fixes #392 --- src/types/YText.js | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/types/YText.js b/src/types/YText.js index 7e24f3ed..83bf0d19 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -164,20 +164,6 @@ const insertNegatedAttributes = (transaction, parent, currPos, negatedAttributes const doc = transaction.doc const ownClientId = doc.clientID negatedAttributes.forEach((val, key) => { - // check if we really need to create attributes - // (the attribute may be set the desired value already) - let n = currPos.right - while( - n !== null && (n.deleted === true || n.content.constructor === ContentFormat) - ) { - if (!n.deleted && equalAttrs(currPos.currentAttributes.get(/** @type {ContentFormat} */ (n.content).key) ?? null, /** @type {ContentFormat} */ (n.content).value)) { - n.delete(transaction) - return - } - n = n.right - } - - // create negated attribute const left = currPos.left const right = currPos.right const nextFormat = new Item(createID(ownClientId, getState(doc.store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentFormat(key, val)) @@ -305,7 +291,8 @@ const formatText = (transaction, parent, currPos, length, attributes) => { const negatedAttributes = insertAttributes(transaction, parent, currPos, attributes) // iterate until first non-format or null is found // delete all formats with attributes[format.key] != null - while (length > 0 && currPos.right !== null) { + // also check the attributes after the first non-format as we do not want to insert redundant negated attributes there + while (currPos.right !== null && (length > 0 || currPos.right.content.constructor === ContentFormat)) { if (!currPos.right.deleted) { switch (currPos.right.content.constructor) { case ContentFormat: {