You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Describe the bug
The cleanupFormattingGap function assumes that attribute values are primitives that can be meaningfully compared using ===, which can sometimes result in Yjs failing to delete ContentFormat items whose values are objects. In Yjs 13 (I was unable to reproduce this in Yjs 14), these ContentFormat items are sometimes* subsequently deleted by other code, but as part of a different transaction.
*This can happen when some code reads the event.changes getter property of some event object, which seems to have the side-effect of causing Yjs's internal data structure to be normalised.
This means that if an application is using an UndoManager scoped to a single transaction origin, performing an undo can succeed in restoring deleted text but fail to restore the ContentFormat item that was deleted in a separate transaction. This often results in text formatting being incorrectly extended to the end of the type.
Here's a demo of this happening in our application:
image.mp4
To Reproduce (Yjs 13.6.30)
import*asYfrom'yjs'// v13.6.30constUNDO_ORIGIN=Symbol()constyDoc=newY.Doc()constparagraph=yDoc.getText()constundoManager=newY.UndoManager(paragraph,{trackedOrigins: newSet([UNDO_ORIGIN]),})// It's important to apply the format in two steps so that the two// { bold: { active: false }} objects are differentparagraph.insert(0,'one two three',{bold: {active: false}})paragraph.format(4,9,{bold: {active: true}})paragraph.format(7,6,{bold: {active: false}})paragraph.observe(event=>{event.changes// Cause the internal data structure to be normalised})// The ContentFormat for { bold: { active: false }} is erroneously deleted as// part of a separate transaction with an origin of nullyDoc.transact(()=>paragraph.delete(2,9),UNDO_ORIGIN)// The UndoManager successfully undoes everything except the deletion of the// ContentFormat for { bold: { active: false }}undoManager.undo()console.dir(paragraph.toDelta(),{depth: null})
Expected behavior cleanupFormattingGap should mark the ContentFormat for { bold: { active: false } } as deleted even though the { active: false } object is not referentially equal to the { active: false } object of the preceding text.
Environment Information
The issue occurs in all tested browsers and recent Node.js versions.
Yjs version: See above.
Workaround
Users impacted by this can use their package manager or patch-package to resolve this issue by patching Yjs.
In the function cleanupFormattingGap, apply the following change, which uses the same equalAttrs comparison function that is used elsewhere in Yjs's code for comparing attribute values, which handles object attribute values correctly.
- if (endFormats.get(key) !== content || startAttrValue === value) {+ if (endFormats.get(key) !== content || equalAttrs(startAttrValue, value)) {
Describe the bug
The
cleanupFormattingGapfunction assumes that attribute values are primitives that can be meaningfully compared using===, which can sometimes result in Yjs failing to deleteContentFormatitems whose values are objects. In Yjs 13 (I was unable to reproduce this in Yjs 14), theseContentFormatitems are sometimes* subsequently deleted by other code, but as part of a different transaction.*This can happen when some code reads the
event.changesgetter property of some event object, which seems to have the side-effect of causing Yjs's internal data structure to be normalised.This means that if an application is using an
UndoManagerscoped to a single transaction origin, performing an undo can succeed in restoring deleted text but fail to restore theContentFormatitem that was deleted in a separate transaction. This often results in text formatting being incorrectly extended to the end of the type.Here's a demo of this happening in our application:
image.mp4
To Reproduce (Yjs 13.6.30)
Expected:
Actual:
To reproduce (latest commit from
main)Expected:
Actual:
Expected behavior
cleanupFormattingGapshould mark theContentFormatfor{ bold: { active: false } }as deleted even though the{ active: false }object is not referentially equal to the{ active: false }object of the preceding text.Environment Information
Workaround
Users impacted by this can use their package manager or
patch-packageto resolve this issue by patching Yjs.In the function
cleanupFormattingGap, apply the following change, which uses the sameequalAttrscomparison function that is used elsewhere in Yjs's code for comparing attribute values, which handles object attribute values correctly.