Slate: operations need to normalize all ancestors

[ianstormtaylor]

Do you want to request a _feature_ or report a _bug_?

Bug.

What's the current behavior?

Right now when an operation is normalized, we determine which paths need to be normalized based on which type of operation it is.

https://github.com/ianstormtaylor/slate/blob/ea6c4dcb08734a2480f4c8faaad1986bbaa0e7da/packages/slate/src/controllers/change.js#L319-L379

However, we are currently missing out on the ancestors of the target nodes in many situations. This leads to marks validation not working properly, and things like first/last not working properly in certain scenarios.

What's the expected behavior?

We need to always include all of the ancestor paths of the dirty paths (instead of just the target node, or the target and its parent), so that the normalization can continue up the document tree.

[Slapbox]

@ianstormtaylor do you have a fix in mind for this/any rough estimate of when we might see a fix if so?

I updated to 0.42 and I'd like to stay on it and try to help discover/clean up loose ends, but this bug is pretty brutal and I'm torn between staying the course and rolling back my commits to when I was using 0.41

[ianstormtaylor]

@Slapbox I'm working on a few other things right this moment, but I'd get to this at some point soon-ish otherwise. The fix is somewhat straightforward. As an example, in the case of an insert_node right now at [0,0,1]...

https://github.com/ianstormtaylor/slate/blob/ea6c4dcb08734a2480f4c8faaad1986bbaa0e7da/packages/slate/src/controllers/change.js#L332-L337

We'd end up marking a few paths as dirty:

• The parent's path: [0,0]
• The node's path: [0,0,1]
• And all of the node's children's paths: [0,0,1,...]

But to be fully correct, we need to make sure the dirty paths include all of the node's ancestors, and not just its direct parent, including:

• The grandparent's path: [0]
• The document's path: []

To do so, we can actually simplify some of the existing logic, by introducing a PathUtils.getAncestors (we can find a better name later) utility:

PathUtils.getAncestors(path).toArray()
// [
//   [],
//   [0],
//   [0,0],
// ]

Which we can then use to easily add all of the higher-up paths to the dirty paths for each of the different operations:

return [...ancestors, path, ...children]

[Slapbox]

@ianstormtaylor thanks a lot! On the surface that makes sense, we'll see if I still feel that way once I start digging.