insertFragment fails on nested blocks

[Soreine]

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

A bug

What's the current behavior?

As of slate@0.31.7 go to http://slatejs.org/#/rich-text and triple click the quote "A wise quote." to select the whole line, then paste any piece of plain text here. This triggers insertFragmentAtRange which fails

Uncaught TypeError: Cannot read property 'key' of undefined
    at Changes.insertFragmentAtRange (http://slatejs.org/build.prod.js:5089:8501)
    at e.value (http://slatejs.org/build.prod.js:5125:2404)
    at e.Change.(anonymous function) [as insertFragmentAtRange] (http://slatejs.org/build.prod.js:5125:2880)
    at Changes.insertFragment (http://slatejs.org/build.prod.js:5086:1769)
    at e.value (http://slatejs.org/build.prod.js:5125:2404)
    at e.Change.(anonymous function) [as insertFragment] (http://slatejs.org/build.prod.js:5125:2880)
    at Object.f [as onPaste] (http://slatejs.org/build.prod.js:5032:5358)
    at t.value (http://slatejs.org/build.prod.js:5161:2517)
    at http://slatejs.org/build.prod.js:5001:5650
    at e.value (http://slatejs.org/build.prod.js:5125:2404)

What's the expected behavior?

It should not fail

This makes it hard to paste in code blocks or lists for example, depending on the nested structure your app is using.

[tobiasandersen]

This might be a stupid question, but what is the expected behavior here? Should it insert the pasted fragment into the quote-block (1), or should it replace the quote with the fragment (2/3)?

Let's say I paste the word "Hello", which of these are "correct"?

1:

2:

3:

Personally I feel like (1) makes more sense. But maybe there're other situations where that behavior could be problematic?

[dmitrizzle]

I'd expect (1) as that's what every other rich text editor does to my knowledge: paste and match style. Since that's what plain text is: data with no style association, meaning we have to assume some kind of style to it, which is usually derived from the location of paste.

[tobiasandersen]

I had a look at how Dropbox Paper handles it. If I at this point paste the word "Hello":

I actually end up with:

So I guess we have a fourth candidate:
4:

[ianstormtaylor]

I think (1) is the most expected.

There's a common issue that is when a user triple-clicks, they are usually referring to the idea of "the entire block", but the browser happens to put the focus point at the start of the next. Which explains the Dropbox Paper behavior, even though it's non-obvious.

That said, it might be harder to get that working to start, and we could start with the Dropbox Paper behavior if so.

[benjycui]

Most of us expect 1. But why do we delete start block when selection is hanging?

slate/packages/slate/src/changes/at-range.js

Lines 244 to 250 in fb8075a

	// If the selection is hanging, just remove the start block, otherwise
	// merge the end block into it.
	if (isHanging) {
	change.removeNodeByKey(startBlock.key, { normalize: false })
	} else {
	change.mergeNodeByKey(endBlock.key, { normalize: false })
	}

IMO, we just need to clean children of start block, and we don't need to delete start block or merge end block.

[zhujinxuan]

@benjycui Because the hanging selection is when someone triple click in a block and select that block. Then it is alike select the whole block and then delete.

I make some fixes related, hopefully they will fix hanging->insert problems in next merge.
#1747 #1742 #1741

[ianstormtaylor]

See #2746

[ianstormtaylor]

I believe that this may be fixed by #3093, which has changed a lot of the logic in Slate and slate-react especially. I'm going to close this out, but as always, feel free to open a new issue if it persists for you. Thanks for understanding.