Bug 9505 – moveParagraphContentsToNewBlockIfNecessary creates a new block outside the current position

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Graham Dennis]

When pasting certain HTML from Safari, moveParagraphContents creates an invalid DIV element as the first child of a TABLE element. e.g.
<table> <div> <tbody> ... </tbody> </div> </table>

Steps to reproduce:
Do a select-all on the website http://nowataballpark.org/contact.html then paste the result into any contentEditable div. The result will include a <div> element within the table.

Cause: By the time the fixupNodeStyles() phase of pasting begins, the HTML just before the table element looks like:
...
    <p style="color: rgb(255, 0, 0); text-align: -khtml-center; "/>
    <table border="1" bgcolor="#FFFFFF">
      <tbody style="border-spacing: 2px 2px; color: rgb(255, 0, 0); ">
...
When applyStyle() is called on the empty paragraph element, this eventually calls through to moveParagraphContentsToNewBlockIfNecessary with a position of [p, 0) (where p is the paragraph element). When the VisiblePositions are created, because the paragraph element has no visible position, the vp's are all within the table element, and a div element is created inside the table element. I will attach a patch.

This patch doesn't come with any layout tests as I haven't managed to get one working. I've tried modifying editing/pasteboard/paste-4039777-fix without any luck. However the above-mentioned steps do replicate the bug.

[Graham Dennis]

Created attachment 8920 [details]
patch

[Graham Dennis]

Created attachment 8931 [details]
testcase

While messing around with trying to make a new testcase for bug 9507, I accidentally made a testcase that replicates this bug. :-)

[Justin Garcia]

Comment on attachment 8920 [details]
patch

+	// Perform some sanity checks. If the position pos doesn't have any visible positions
+	// then paragraphStart will be outside the paragraph
+	if (Range::compareBoundaryPoints(pos, paragraphStart) < 0)
+		return;
+	  

Looks good.  I think you mean "If there are no VisiblePositions in the same block as pos".
I can't reproduce the bug you describe with the testcase you attached.  I run it and the table doesn't have an invalid div child.

[Graham Dennis]

Created attachment 8978 [details]
patch 2

Modified patch including testcase and modified the comments.

Note that because this testcase uses pasteCommand(), it will only work in Debug builds of WebKit or in DumpRenderTree. I've tested the testcase again for myself and it seems to work.

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> Created an attachment (id=8978) [edit]
> patch 2

Graham, in the future, it would be helpful to include test results (*-expected.txt, *-expected.checksum and *-expected.png) with the tests when you create a patch.  Thanks!

[David Kilzer (:ddkilzer)]

Committed revision 15103.

Also, please don't include tabs in the source code.  :)

[Graham Dennis]

(In reply to comment #6)
> Committed revision 15103.
> 
> Also, please don't include tabs in the source code.  :)
> 
Oops. I have included results of tests in some of my other patches, but I forgot to do that for this one...
And I'll change SEE over to use spaces instead of tabs.
Sorry about that...