UndoPlugin's selection restoration doesn't work as expectation

[ljho01]

Please save me some time and use the following template. In 90% of all issues I can't reproduce the problem because I don't know what exactly you are doing, in which environment, or which y-* version is responsible. Just use the following template even if you think the problem is obvious.

Checklist

• Are you reporting a bug? Use github issues for bug reports and feature requests. For general questions, please use https://discuss.yjs.dev/
• Try to report your issue in the correct repository. Yjs consists of many modules. When in doubt, report it to https://github.com/yjs/yjs/issues/

Describe the bug
it seems the selection restoring code in UndoPlugin doesn't have any effect.

i think this code is supposed to work like this

1. beforeAllTransactions() hook in sync-plugin.js initialize binding.beforeTransactionSelection to current selection
2. stack-item-popped event handler in UndoManager changes binding.beforeTransactionSelection to selection information stored inStackItem's meta, which is saved when stack-item-added event occurs.
3. then restoreRelativeSelection() restores the selection information.

but i found that stack-item-popped event handler is executed after the restoreRelativeSelection(), because the stack-item-popped event is emited after the observeDeep handler(which executes restoreRelativeSelection()), eventually using the selection information right before the undoing action.

so i suggest checking the transaction and restore the selection information in '_typeChanged()` like this

       // @ts-ignore
       let tr = this._tr.replace(
         0,
         this.prosemirrorView.state.doc.content.size,
         new PModel.Slice(PModel.Fragment.from(fragmentContent), 0, 0)
       )
+      // restore selection if undo/redo
+      const yUndoPluginState = yUndoPluginKey.getState(this.prosemirrorView.state);
+
+      if (transaction.origin === yUndoPluginState?.undoManager) {
+        const manager = transaction.origin;
+        if (manager.undoing || manager.redoing) {
+          this.beforeTransactionSelection =
+            manager.currStackItem?.meta.get(this);
+        }
+      }
        restoreRelativeSelection(tr, this.beforeTransactionSelection, this);
        tr = tr.setMeta(ySyncPluginKey, {
          isChangeOrigin: true,
          isUndoRedoOperation: transaction.origin instanceof Y.UndoManager,
        });

To Reproduce
Steps to reproduce the behavior:

1. add unique id to relativeSelection by adding attributes when getRelativeSelection() executes
2. insert the console.log() before the restoreRelativeSelection() executes to check which relativeSelection is used.
3. check the relativeSelection of StackItem at the top of the undoStack.
4. do undo() and check whether the relativeSelection in 3 is restored or not.

Expected behavior
A clear and concise description of what you expected to happen.

• the id of relativeSelection if 4 should be same with that of 3.
• but will be the id of relativeSelection generated right before the undo transaction, since beforeAllTransaction hook always initialize the binding.beforeTransactionSelection to current one.

Screenshots

without fix

initial

after backspace

then undo, the cursor(selection) is in front of 'bark bark'

with fix

initial

after backspace

then undo

Environment Information

• "yjs": "13.6.24",
• "y-prosemirror": "1.3.4"

I'd like to submit a PR with the proposed fix above. would it be acceptable to open a PR with this change?

Let me know if you prefer another approach or want further tests.

Thanks!

• I'm a sponsor 💖
• This issue is a blocker for my project.