[webkit-reviews] review granted: [Bug 215406] Text manipulation should not manipulate nodes out of paragraph range : [Attachment 406445] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Sihui Liu
<sihui_liu at apple.com>'s request for review:
Bug 215406: Text manipulation should not manipulate nodes out of paragraph
range
https://bugs.webkit.org/show_bug.cgi?id=215406

Attachment 406445: Patch

https://bugs.webkit.org/attachment.cgi?id=406445&action=review




--- Comment #2 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 406445
  --> https://bugs.webkit.org/attachment.cgi?id=406445
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406445&action=review

> Source/WebCore/ChangeLog:3
> +	   Text manipulationshould not manipulate nodes out of paragraph range

Nit - space before “should”

> Source/WebCore/ChangeLog:9
> +	   the nodes on the start path but out ouf range correctly, and may
change position of those nodes by mistake. For 

Nit - “out of”

> Source/WebCore/editing/TextManipulationController.cpp:840
> +    while (!startTopDownPath.isEmpty()) {
> +	   auto lastNode = startTopDownPath.last();
> +	   ASSERT(is<ContainerNode>(lastNode.get()));
> +	   if (!downcast<ContainerNode>(lastNode.get()).hasOneChild())
> +	       break;
> +	   nodesToRemove.add(startTopDownPath.takeLast());
> +    }

Perhaps getPath() should just return `Vector<Ref<ContainerNode>>` instead, and
change this into something along the lines of:
```
while (!startTopDownPath.isEmpty() && startTopDownPath.last()->hasOneChild())
    nodesToRemove.add(startTopDownPath.takeLast());
```