yjs icon indicating copy to clipboard operation
yjs copied to clipboard

Undoing a deletion can fail to restore an attribute

Open gblaketx opened this issue 2 years ago • 1 comments

Describe the bug If you set an attribute on an XmlText and delete the node in the same transaction, then undo the change, the resulting update does not restore the original attribute.

To Reproduce Steps to reproduce the behavior:

  1. Create an XmlText with an attribute
  2. Set the attribute to a new value, then delete the XmlText
  3. Create a second, remote document with the same state as the first
  4. Undo the change in the local document
  5. Apply the undo update to the remote document
  6. The two docs are no longer equal

In code:

// 1. Set up a document containing an XmlText with a single attribute
const localYDoc = new Y.Doc();
const localSharedRoot = localYDoc.get("text", Y.XmlText);
const undoManager = new Y.UndoManager(localSharedRoot, { trackedOrigins: new Set([localOrigin]) });
const testEmbed = new Y.XmlText();
testEmbed.setAttribute("type", "paragraph");
localSharedRoot.insertEmbed(0, testEmbed);

// 2. Set the embed attribute, then delete the embed
localYDoc.transact(() => {
  testEmbed.setAttribute("type", "list-item");
  localSharedRoot.delete(0, 1);
}, localOrigin);

// 3. Set up a second yDoc to match the state of the first
const remoteYDoc = new Y.Doc();
Y.applyUpdate(remoteYDoc, Y.encodeStateAsUpdate(localYDoc));

// 4. Undo the change, tracking the update
const localUpdates: Uint8Array[] = [];
localYDoc.on("update", (update: Uint8Array) => {
  localUpdates.push(update);
});
undoManager.undo();

// 5. Apply the update to the second yDoc
expect(localUpdates).toHaveLength(1);
const [undoUpdate] = localUpdates;
Y.applyUpdate(remoteYDoc, undoUpdate);

// 6. The two yDocs are not equal. The delete sets are different.

Expected behavior Expect the two docs to remain equal after applying the undo update to both of them.

Screenshots The documents diverge, as can be seen by comparing their snapshots. (received is the remote doc, expected is the local,) image

Environment Information

  • Yjs version = 13.5.45

Additional context Ran into the problem while using Slate Yjs and attempting to undo a list creation. Creating the list would change the type of the target element, then move it into a wrapper node, which resulted in remove and insert operations using the element. As a workaround, we've updated our list creation logic to avoid this particular order of operations.

gblaketx avatar May 24 '23 01:05 gblaketx

hi, @gblaketx

I change your code & the issue is not reproduce.

const Y = require('yjs');
const localOrigin = 'localOrigin';

// 1. Set up a document containing an XmlText with a single attribute
const localYDoc = new Y.Doc();
const localSharedRoot = localYDoc.get("text", Y.XmlText);
const undoManager = new Y.UndoManager(localSharedRoot, { trackedOrigins: new Set([localOrigin]) });
const testEmbed = new Y.XmlElement();
testEmbed.setAttribute("type", "paragraph");
localSharedRoot.insertEmbed(0, testEmbed);

// 2. Set the embed attribute, then delete the embed
localYDoc.transact(() => {
  testEmbed.setAttribute("type", "list-item");
  // localSharedRoot.delete(0, 1);
}, localOrigin);

// 3. Set up a second yDoc to match the state of the first
const remoteYDoc = new Y.Doc();
remoteYDoc.get("text", Y.XmlText);
Y.applyUpdate(remoteYDoc, Y.encodeStateAsUpdate(localYDoc));

// 4. Apply the update to the second yDoc
localYDoc.on("update", (update) => {
  Y.applyUpdate(remoteYDoc, update);
});

console.log('--- before undo ---', {
  localYDoc: localYDoc.toJSON(),
  remoteYDoc: remoteYDoc.toJSON()
});

// 5. Undo the change, tracking the update
undoManager.undo();

// 6. The two yDocs are not equal. The delete sets are different.
console.log('--- after undo ---', {
  localYDoc: localYDoc.toJSON(),
  remoteYDoc: remoteYDoc.toJSON()
});
image

lijie1129 avatar Jun 30 '23 07:06 lijie1129