Slate: Nested blocks issue

Created on 17 Jan 2017  路  8Comments  路  Source: ianstormtaylor/slate

There is problem with transformations of doubly nested blocks (document - documentNode - nestedNode - doubleNestedNode). There is problem in following cases:

  • caret at end of the doubleNestedNode, press delete
  • caret at beggining of (not first) node, press backspace
  • paste selection of more than one block
Uncaught Error: Could not find a node with key "47".
    at Document.assertNode
    at Object.normalizeNodeByKey
    at Transform.(anonymous function) [as normalizeNodeByKey] 
    at Object.deleteAtRange
    at Transform.(anonymous function) [as deleteAtRange]
    at Object.deleteBackwardAtRange
    at Transform.(anonymous function) [as deleteBackwardAtRange] 
    at Object.deleteCharBackwardAtRange
    at Transform.(anonymous function) [as deleteCharBackwardAtRange] 
    at Object.deleteCharBackward

Double nested blocks have actually multiple issues and strange behaviors that regular nested blocks does not have. For example backspace with caret at first double nested block deletes without producing error, but deletes whole content of nestedBlock keeping only first doubleNestedBlock.

I am aware that this could be solved with custom plugin, but since Slate supports unlimited nesting of blocks, this should be probably solved in core.

bug

Most helpful comment

@Mangatt agreed, there appears to be a class of issues related to deeply nested block handling.

I opened #917 to address one aspect of it. Here's what I found (again, just one aspect of a greater issue):

  • Insert a fragment containing two sibling leaf blocks nested more than one layer deep.
  • Example: A list with a single list item, and that list item contains two paragraphs with text.
  • Transform.insertFragmentAtRange checks the first and last leaf blocks, and if they are not the same, it removes the parent of the first leaf block pending a potential node split. The intention is to add the first leaf node parent back in after the split.
  • In this case, the first and last leaf node share the same parent, so both nodes are inadvertently removed. I believe this is due to this line, where the iterator for finding lonelyParent simply requires the parent to contain only one node. This doesn't account for that one node having multiple children.
  • We then attempt to move the last leaf node into the document, but it's been removed from the fragment, hence the error associated with moveNodeByKey.

I've minimally reproduced this in a Fiddle: https://jsfiddle.net/erquhart/L05uz8qp/2/

My PR addresses this particular case, but there seem to be many more. For example, if you paste into a selection, which requires a split, you get new errors associated with marks.

The ideal approach is to find the underlying shared assumptions that cause deeply nested fragments to be mishandled.

@ianstormtaylor we'll likely need your help getting this going. Do you recall running into any issues like this?

All 8 comments

To be precise, error is produced everytime when editor tries to merge multiple doubleNestedBlocks.

Hey @Mangatt thanks for the report, can you create a smallest-possible JSFiddle to reproduce this so that it's easier for people to see what causes the bug? That will help in terms of it being resolved. Check out the Contributing.md file for a template to use. Thanks!

Here it is, @ianstormtaylor :

https://jsfiddle.net/3mherqht/1/

Place cursor at beginning of line 2, right before text DoubleNested2. Press backspace. Or try any of bullets above.

I guess that this is connected, but transform.moveNodeByKey has this issue as well. It behaves correctly until I try to move node into doubleNestedBlock.

When I removeNodeByKey and subsequently insertNodeByKey, everything works just fine. I guess that moveNodeByKey could internaly be just combination of these, although it isn't.

When I was testing behavior of my PR #563, I came across strange behavior in nested blocks:

https://jsfiddle.net/3mherqht/2/

Place cursor at beginning of DoubleNested1 and press backspace. Puff... And DoubleNested2 is gone. More exactly everything that is inside that block will be removed except first block that is moved two levels up.

Also, running into this. Deleting a void node within a non-void node is leading to these errors for me. This prevents from the node being deleted 馃槙

@Mangatt did you come across any solution for this?

@Mangatt agreed, there appears to be a class of issues related to deeply nested block handling.

I opened #917 to address one aspect of it. Here's what I found (again, just one aspect of a greater issue):

  • Insert a fragment containing two sibling leaf blocks nested more than one layer deep.
  • Example: A list with a single list item, and that list item contains two paragraphs with text.
  • Transform.insertFragmentAtRange checks the first and last leaf blocks, and if they are not the same, it removes the parent of the first leaf block pending a potential node split. The intention is to add the first leaf node parent back in after the split.
  • In this case, the first and last leaf node share the same parent, so both nodes are inadvertently removed. I believe this is due to this line, where the iterator for finding lonelyParent simply requires the parent to contain only one node. This doesn't account for that one node having multiple children.
  • We then attempt to move the last leaf node into the document, but it's been removed from the fragment, hence the error associated with moveNodeByKey.

I've minimally reproduced this in a Fiddle: https://jsfiddle.net/erquhart/L05uz8qp/2/

My PR addresses this particular case, but there seem to be many more. For example, if you paste into a selection, which requires a split, you get new errors associated with marks.

The ideal approach is to find the underlying shared assumptions that cause deeply nested fragments to be mishandled.

@ianstormtaylor we'll likely need your help getting this going. Do you recall running into any issues like this?

This issue hasn't had a response in a while, and it's unclear if it's still present in the latest versions of Slate. To keep the issues easier to manage, since there are so many being opened every day, I'm going to close this old one out.

But if you notice that the bug is still happening, please feel free to comment and it can be revisited. It might be an edge case that not many people run into, in which case the fastest way to get it fixed would probably be to write a pull request for it.

Thanks for understanding!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

YurkaninRyan picture YurkaninRyan  路  3Comments

varoot picture varoot  路  3Comments

chrpeter picture chrpeter  路  3Comments

JSH3R0 picture JSH3R0  路  3Comments

vdms picture vdms  路  3Comments