Bug 7955 - REGRESSION: Content with an interchange newline lost when pasted at the end of the document
Summary: REGRESSION: Content with an interchange newline lost when pasted at the end o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Justin Garcia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-24 00:18 PST by Justin Garcia
Modified: 2006-03-24 19:26 PST (History)
0 users

See Also:


Attachments
testcase (1.09 KB, text/html)
2006-03-24 01:14 PST, Justin Garcia
no flags Details
patch (20.28 KB, patch)
2006-03-24 01:58 PST, Justin Garcia
justin.garcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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