Prosemirror: Is this change to Transform.replace intentional or a bug?

[lastnigtic]

Issue details

The source code is as follows:

Transform.prototype.replace = function(from, to = from, slice = Slice.empty) {
  let step = replaceStep(this.doc, from, to, slice)
  if (step) this.step(step)
  return this
}

I found that after this commit ，the result of replaceStep has changed.

before:

after:

---

The doc is same after replace but the selection is different.

For example, use the insert to insert a block-left node at pos 1.Before this commit, the selection.from after inserting is 4, but after this commit, the selection.from is still 1.

[lastnigtic]

This wil cause the order of the nodes inserted continuously using this method to be reversed.

[marijnh]

This may be unintentional. I'll investigate when I have time—which might take a few weeks.

[jljorgenson18]

I had a similar bug with drag and drop where the selection is now collapsed after being dropped instead of being expanded and encompassing the entire dropped slice. It caused us to roll back prosemirror-transform to the version before the replace fit refactor. I wasn't sure if we were doing something wrong on our end so I didn't want to file something. However, I think it is probably related to the bug here.

[marijnh]

Seems you didn't provide any examples of inputs that produce the change in behavior, and the code you pasted is just the replace method from the library. I'm going to need some way to reproduce this, if you want me to investigate.

[lastnigtic]

Seems you didn't provide any examples of inputs that produce the change in behavior, and the code you pasted is just the replace method from the library. I'm going to need some way to reproduce this, if you want me to investigate.

Sorry for late reply.
You can perform the following operations at prosemirror.net to reproduce this case.

1. Delete all the content in editor.

2. Running the below code.

var {dispatch, state} = view 

var tr = state.tr.insert(1, state.schema.nodes['heading'].create(undefined, state.schema.text('1')))
tr = tr.insert(tr.selection.from, state.schema.nodes['heading'].create(undefined, state.schema.text('2')))

dispatch(tr)

Then you will see the order of 'heading' nodes are reversed.

[marijnh]

The doc is same after replace but the selection is different.

When I try this, I see the document is _different_. With the old code, there's an extra empty paragraph after the replacement, with the new code, there isn't. As for the document output, I think the new code behaves better.

But it does create a step that _covers_ the empty paragraph's closing token, rather than being purely an insert. And mapping behaves differently when you map a position directly in front of a replace that covers content versus when you map a position in front of an insertion—when mapping with a positive bias, it'll keep the position in front of a replace, but put it after an insertion. This usually is appropriate, but in this case it'll lead to odd results.

Note that if you had used tr.replaceSelectionWith instead of tr.insert, your selection would move in front of the inserted content, since that takes care to adjust the selection. In general, selection-based insertion operations should prefer replaceSelection, deleteSelection, and replaceSelectionWith, since the precise effect of the default selection mapping can be hard to predict.

It is awkward that the changes to the replace-fitting algorithm had side effects like this one, but those are caused by changes in output that are generally improvements, so I don't want to roll them back on that account.