Bug 7955

Summary: REGRESSION: Content with an interchange newline lost when pasted at the end of the document
Product: WebKit Reporter: Justin Garcia <justin.garcia>
Component: HTML EditingAssignee: Justin Garcia <justin.garcia>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
testcase
none
patch justin.garcia: review+

Description Justin Garcia 2006-03-24 00:18:04 PST
ReplaceSelectionCommand sometimes does an InsertParagraphSeparator, which sometimes folds everything from the current insertion point to the end of the document into a div.  This will move the ReplacementFragment's holder, something I didn't foresee when I checked in my patch yesterday.  Also the holder can cause isEndOfBlock and isEndOfDocument checks inside ReplaceSelectionCommand to fail erroneously.
Comment 1 Justin Garcia 2006-03-24 01:14:40 PST
Created attachment 7274 [details]
testcase
Comment 2 Justin Garcia 2006-03-24 01:58:29 PST
Created attachment 7276 [details]
patch

Instead of getting rid of isProbablyBlock by keeping the replacement fragment in the document during the paste operation, I keep a map from nodes to RenderingInfos - a class that contains the style that a node had during the test insertion, and a bool for whether or not its renderer was blockFlow during the test insertion.

When I restore styles, I must do so in depth first order, so that restoring the style on a node doesn't negate restoration I've done on its children.  To do this, I traverse the fragment in depth first order during the test insertion, putting nodes into a Vector in addition to putting them into the map.  Then I restore styles to nodes in the order that they occur in the Vector.  At first I tried to avoid using the Vector by passing fixupNodeStyles a range to operate over, and traversing that range in depth first order.  But since style restoration does an ApplyStyleCommand, which moves nodes around, wraps them in new blocks, and splits and merges nodes, I wasn't completely confident that my code would visit all of the nodes that were pasted.

I could have also left the code as-is and just added a HashMap<Node*, bool> for the isBlockFlow checks, but I thought that paste might eventually need to save even more information about the test insertion, like isInline.

The attached diff is between my tree and a revision before my patch was checked-in.
Comment 3 Justin Garcia 2006-03-24 05:59:48 PST
> When I restore styles, I must do so in depth first order, so that restoring the
> style on a node doesn't negate restoration I've done on its children.  

I meant pre-order traversal.

> restoration does an ApplyStyleCommand, which moves nodes around, wraps them in
> new blocks, and splits and merges nodes, I wasn't completely confident that my
> code would visit all of the nodes that were pasted.

This isn't exactly true, I think I just thought the code looked confusing.
Comment 4 Justin Garcia 2006-03-24 17:53:43 PST
Comment on attachment 7276 [details]
patch

dave reviewed this