Transforms.removeNodes(editor) does not delete the current selection when it includes void nodes

[markogresak]

Do you want to request a feature or report a bug?

Bug

What's the current behavior?

The location where Transforms.removeNodes(editor) is applied is offset. The target content is not entirely removed, but it removes the content before it. I have reproduced it here: https://codesandbox.io/s/slate-reproductions-forked-52x60?file=/index.js

Recording


If you do not trust the code which plays the scenario, you can try it out manually. Steps to reproduce:

1. Open the example at https://codesandbox.io/s/slate-reproductions-forked-52x60?file=/index.js.
2. Click at the beginning of the "Paragraph 2".
3. Hold down ⇧ Shift and press the ↓ (down key) 3 times.
4. Press ← Backspace.

Recording of manual steps


The result is not exactly the same, but it's also wrong.

Slate: 0.58.4 Browser: Chrome (and Firefox too, so I assume any browser) OS: MacOS (but I assume it's the same on other platforms)

What's the expected behavior?

Transforms.removeNodes(editor) removes exactly what's selected, nothing more, nothing less.

[markogresak]

It's also possible to reproduce this with the images example at https://www.slatejs.org/examples/images.

Steps:

1. Duplicate the first paragraph of the editor.
2. Select the first paragraph and the image below[0].
3. Press backspace.

A recording: https://share.getcloudapp.com/NQurnA90 (Apologies for a 3rd party link - Github took too long to upload the rather large gif and it won't accept the mp4 recording).

[0]: The selection must show the image as selected with the selection highlight, as shown on the screenshot below

[kamilmielnik]

The result is not exactly the same, but it's also wrong.

Note: Hitting ← Backspace calls Editor.deleteFragment which uses Transforms.delete - not Transforms.removeNodes.

---

Transforms.removeNodes(editor) does not delete the current selection when it includes void nodes

Note: voids: true parameter should be passed (Transforms.removeNodes(editor, { voids: true })).

---

Stilll, it does not work as expected. hanging: true parameter does the trick, but its' purpose may not be wanted, so that's not a valid workaround.

---

Conclusion

I believe that Editor.deleteFragment should pass voids: true parameter to Transforms.delete.

---

I overrode it (as a plugin) and it improved things a bit.

However it does not work well if void node is at the beginning or an end of the selection. I narrowed it down to this line, which excludes these nodes from deletion: https://github.com/ianstormtaylor/slate/blob/0bbe121d76c5c2313d939de8a7ebed3bfd37f62d/packages/slate/src/transforms/text.ts#L132

I don't know yet what code should be there, but I'm pretty sure it's not Path.isCommon.