[webkit-reviews] review requested: [Bug 8333] Blank lines are not properly created when <pre> is used : [Attachment 7696] Patch for review.

[bugzilla-request-daemon at opendarwin.org]

Dave Hyatt <hyatt at apple.com> has asked	for review:
Bug 8333: Blank lines are not properly created when <pre> is used
http://bugzilla.opendarwin.org/show_bug.cgi?id=8333

Attachment 7696: Patch for review.
http://bugzilla.opendarwin.org/attachment.cgi?id=7696&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
Here is a patch for review.  This patch also turns off all of the layout test
hacks (and so it changes every single layout test in the tree).

I should make a general comment about the merge-whitespace-pre.html test. 
After spending about 5 hours on the problem, I have decided to go ahead and
check in updated results for the test.	The current rendering is already buggy
(the layout test is demonstrating an unfixed bug).  My patch does make the
rendering worse, but the reason it does is that much of the editing code simply
doesn't know how to deal properly with preserved newlines yet.

The deletion issue has to do with the fact that a placeholder doesn't get made
once newlines are rendered positions and so you hit code that already has a
FIXME...

// FIXME: Deletion has bugs and it doesn't always add a placeholder.  If it
fails, still do pruning.
    else
	prune(placeholder);

The layout test in question is already badly broken.  If you hit enter in
newline-preserving content (like white-space: pre) we are doubling the spacing
between lines.	I think the code is so confused that it is splitting the <pre>
block (which has margins on it by default).

Deletion that crosses white-space boundaries also doesn't handle the fact that
line breaks are different (<br>s in some cases and preserved newlines in
others).  This has to be dealt with when merging blocks.  Many of the places in
the editing code that deal with <br>s simply don't know what to make of
preserved newlines.  This was true both before and after my patch.

The upshot is that editing of preformatted text is in a sorry state. :(