WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2007 05:32:35 +0000 (05:32 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2007 05:32:35 +0000 (05:32 +0000)
        Reviewed by Oliver Hunt.

        <rdar://problem/5607069> In Mail, a crash occurs at WebCore::AppendNodeCommand() after dragging image into a <FORM> element

        * editing/InsertLineBreakCommand.cpp:
        (WebCore::InsertLineBreakCommand::shouldUseBreakElement): Equip this function to
        handle editing positions, like [input, 0];
        * editing/InsertParagraphSeparatorCommand.cpp:
        (WebCore::InsertParagraphSeparatorCommand::doApply): Pass enclosingBlock a node peeled
        off of a non-editing position, to fix a bug where the enclosing block of [input, 0] was
        the input element itself.
        Insert a <br> when a <form> element is the enclosing block instead of splitting/cloning or
        nesting a <div>.

LayoutTests:

        Reviewed by Oliver Hunt.

        <rdar://problem/5607069> In Mail, a crash occurs at WebCore::AppendNodeCommand() after dragging image into a <FORM> element

        * editing/inserting/5607069-1-expected.checksum: Added.
        * editing/inserting/5607069-1-expected.png: Added.
        * editing/inserting/5607069-1-expected.txt: Added.
        * editing/inserting/5607069-1.html: Added.
        * editing/inserting/5607069-2.html: Added.
        * editing/inserting/5607069-3.html: Added.
        * platform/mac/editing/inserting/5607069-2-expected.checksum: Added.
        * platform/mac/editing/inserting/5607069-2-expected.png: Added.
        * platform/mac/editing/inserting/5607069-2-expected.txt: Added.
        * platform/mac/editing/inserting/5607069-3-expected.checksum: Added.
        * platform/mac/editing/inserting/5607069-3-expected.png: Added.
        * platform/mac/editing/inserting/5607069-3-expected.txt: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@28710 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/inserting/5607069-1-expected.checksum [new file with mode: 0644]
LayoutTests/editing/inserting/5607069-1-expected.png [new file with mode: 0644]
LayoutTests/editing/inserting/5607069-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/5607069-1.html [new file with mode: 0644]
LayoutTests/editing/inserting/5607069-2.html [new file with mode: 0644]
LayoutTests/editing/inserting/5607069-3.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/inserting/5607069-2-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/editing/inserting/5607069-2-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/editing/inserting/5607069-2-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/editing/inserting/5607069-3-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/editing/inserting/5607069-3-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/editing/inserting/5607069-3-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/InsertLineBreakCommand.cpp
WebCore/editing/InsertParagraphSeparatorCommand.cpp

index 2f89c6e..4032546 100644 (file)
@@ -1,3 +1,22 @@
+2007-12-13  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        <rdar://problem/5607069> In Mail, a crash occurs at WebCore::AppendNodeCommand() after dragging image into a <FORM> element
+
+        * editing/inserting/5607069-1-expected.checksum: Added.
+        * editing/inserting/5607069-1-expected.png: Added.
+        * editing/inserting/5607069-1-expected.txt: Added.
+        * editing/inserting/5607069-1.html: Added.
+        * editing/inserting/5607069-2.html: Added.
+        * editing/inserting/5607069-3.html: Added.
+        * platform/mac/editing/inserting/5607069-2-expected.checksum: Added.
+        * platform/mac/editing/inserting/5607069-2-expected.png: Added.
+        * platform/mac/editing/inserting/5607069-2-expected.txt: Added.
+        * platform/mac/editing/inserting/5607069-3-expected.checksum: Added.
+        * platform/mac/editing/inserting/5607069-3-expected.png: Added.
+        * platform/mac/editing/inserting/5607069-3-expected.txt: Added.
+
 2007-12-13  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Dave Hyatt.
diff --git a/LayoutTests/editing/inserting/5607069-1-expected.checksum b/LayoutTests/editing/inserting/5607069-1-expected.checksum
new file mode 100644 (file)
index 0000000..2f45b0e
--- /dev/null
@@ -0,0 +1 @@
+a5cf7ee538fad3dddb7865ada5c4a0b1
\ No newline at end of file
diff --git a/LayoutTests/editing/inserting/5607069-1-expected.png b/LayoutTests/editing/inserting/5607069-1-expected.png
new file mode 100644 (file)
index 0000000..b87b38d
Binary files /dev/null and b/LayoutTests/editing/inserting/5607069-1-expected.png differ
diff --git a/LayoutTests/editing/inserting/5607069-1-expected.txt b/LayoutTests/editing/inserting/5607069-1-expected.txt
new file mode 100644 (file)
index 0000000..b76d2c1
--- /dev/null
@@ -0,0 +1,4 @@
+This tests for a crash in InsertParagraph when inserting next to an input field or form control. You should see two input fields below in separate paragraphs.
+
+
+
diff --git a/LayoutTests/editing/inserting/5607069-1.html b/LayoutTests/editing/inserting/5607069-1.html
new file mode 100644 (file)
index 0000000..f505c11
--- /dev/null
@@ -0,0 +1,10 @@
+<p>This tests for a crash in InsertParagraph when inserting next to an input field or form control. You should see two input fields below in separate paragraphs.</p>
+<div id="div" contenteditable="true"><input type="text"><input type="text"></div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+div = document.getElementById("div");
+window.getSelection().setPosition(div, 1);
+document.execCommand("InsertParagraph");
+</script>
diff --git a/LayoutTests/editing/inserting/5607069-2.html b/LayoutTests/editing/inserting/5607069-2.html
new file mode 100644 (file)
index 0000000..f914074
--- /dev/null
@@ -0,0 +1,8 @@
+<p>This tests for a bug where InsertLineBreak would insert a '\n' instead of a &lt;br&gt; if the caret was set just before an input field. You should see two input fields each in its own paragraph below.</p>
+<div id="div" contenteditable="true"><input type="text"><input type="text"></div>
+
+<script>
+div = document.getElementById("div");
+window.getSelection().setPosition(div, 1);
+document.execCommand("InsertLineBreak");
+</script>
diff --git a/LayoutTests/editing/inserting/5607069-3.html b/LayoutTests/editing/inserting/5607069-3.html
new file mode 100644 (file)
index 0000000..ca0c218
--- /dev/null
@@ -0,0 +1,10 @@
+<p>This tests for a bug in InsertParagraph where it would split and clone a &lt;form&gt; element in the same way it would a &lt;p&gt; or a &lt;div&gt;.  You should see two paragraphs below both inside the form (blue bordered) element.</p>
+<div id="div" contenteditable="true"><form style="border: 1px solid blue;"><input type="text">xx<input type="text"></form></div>
+
+<script>
+div = document.getElementById("div");
+form = div.childNodes[0];
+text = form.childNodes[1];
+window.getSelection().setPosition(text, 1);
+document.execCommand("InsertParagraph");
+</script>
diff --git a/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.checksum b/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.checksum
new file mode 100644 (file)
index 0000000..e082ff5
--- /dev/null
@@ -0,0 +1 @@
+54bfaae93ceee59549beaf1e931e2d43
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.png b/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.png
new file mode 100644 (file)
index 0000000..b59e3de
Binary files /dev/null and b/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.txt b/LayoutTests/platform/mac/editing/inserting/5607069-2-expected.txt
new file mode 100644 (file)
index 0000000..60cdf7d
--- /dev/null
@@ -0,0 +1,18 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 776x36
+          text run at (0,0) width 776: "This tests for a bug where InsertLineBreak would insert a '\\n' instead of a <br> if the caret was set just before an input field."
+          text run at (0,18) width 413: "You should see two input fields each in its own paragraph below."
+      RenderBlock {DIV} at (0,52) size 784x46
+        RenderTextControl {INPUT} at (2,2) size 148x19 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
+        RenderBR {BR} at (152,16) size 0x0
+        RenderTextControl {INPUT} at (2,25) size 148x19 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
+layer at (13,65) size 142x13
+  RenderBlock {DIV} at (3,3) size 142x13
+layer at (13,88) size 142x13
+  RenderBlock {DIV} at (3,3) size 142x13
+caret: position 0 of child 2 {INPUT} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.checksum b/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.checksum
new file mode 100644 (file)
index 0000000..b34d55a
--- /dev/null
@@ -0,0 +1 @@
+0b5d200dc74b662b948739df4b1aa6a3
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.png b/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.png
new file mode 100644 (file)
index 0000000..6ae6e8c
Binary files /dev/null and b/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.txt b/LayoutTests/platform/mac/editing/inserting/5607069-3-expected.txt
new file mode 100644 (file)
index 0000000..300b594
--- /dev/null
@@ -0,0 +1,24 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x576
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 780x36
+          text run at (0,0) width 780: "This tests for a bug in InsertParagraph where it would split and clone a <form> element in the same way it would a <p> or a"
+          text run at (0,18) width 46: "<div>. "
+          text run at (46,18) width 530: "You should see two paragraphs below both inside the form (blue bordered) element."
+      RenderBlock {DIV} at (0,52) size 784x48
+        RenderBlock {FORM} at (0,0) size 784x48 [border: (1px solid #0000FF)]
+          RenderTextControl {INPUT} at (3,3) size 148x19 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
+          RenderText {#text} at (153,3) size 8x18
+            text run at (153,3) width 8: "x"
+          RenderBR {BR} at (161,17) size 0x0
+          RenderText {#text} at (1,26) size 8x18
+            text run at (1,26) width 8: "x"
+          RenderTextControl {INPUT} at (11,26) size 148x19 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
+layer at (14,66) size 142x13
+  RenderBlock {DIV} at (3,3) size 142x13
+layer at (22,89) size 142x13
+  RenderBlock {DIV} at (3,3) size 142x13
+caret: position 0 of child 3 {#text} of child 0 {FORM} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index d418289..f00f404 100644 (file)
@@ -1,3 +1,19 @@
+2007-12-13  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        <rdar://problem/5607069> In Mail, a crash occurs at WebCore::AppendNodeCommand() after dragging image into a <FORM> element
+
+        * editing/InsertLineBreakCommand.cpp:
+        (WebCore::InsertLineBreakCommand::shouldUseBreakElement): Equip this function to
+        handle editing positions, like [input, 0];
+        * editing/InsertParagraphSeparatorCommand.cpp:
+        (WebCore::InsertParagraphSeparatorCommand::doApply): Pass enclosingBlock a node peeled
+        off of a non-editing position, to fix a bug where the enclosing block of [input, 0] was
+        the input element itself.
+        Insert a <br> when a <form> element is the enclosing block instead of splitting/cloning or
+        nesting a <div>.
+
 2007-12-13  Alp Toker  <alp@atoker.com>
 
         Reviewed by Oliver Hunt.
index cd1067b..1da16d5 100644 (file)
@@ -80,7 +80,11 @@ void InsertLineBreakCommand::insertNodeBeforePosition(Node *node, const Position
 // Whether we should insert a break element or a '\n'.
 bool InsertLineBreakCommand::shouldUseBreakElement(const Position& insertionPos)
 {
-    return insertionPos.node()->renderer() && !insertionPos.node()->renderer()->style()->preserveNewline();
+    // An editing position like [input, 0] actually refers to the position before
+    // the input element, and in that case we need to check the input element's
+    // parent's renderer.
+    Position p(rangeCompliantEquivalent(insertionPos));
+    return p.node()->renderer() && !p.node()->renderer()->style()->preserveNewline();
 }
 
 void InsertLineBreakCommand::doApply()
index 4c5033b..9cd6a92 100644 (file)
@@ -96,12 +96,12 @@ void InsertParagraphSeparatorCommand::doApply()
         affinity = endingSelection().affinity();
     }
     
-    // FIXME: Turn into an InsertLineBreak in other cases where we don't want to do the splitting/cloning that
-    // InsertParagraphSeparator does.
-    Node* block = enclosingBlock(pos.node());
+    // FIXME: The rangeCompliantEquivalent conversion needs to be moved into enclosingBlock.
+    Node* startBlock = enclosingBlock(rangeCompliantEquivalent(pos).node());
     Position canonicalPos = VisiblePosition(pos).deepEquivalent();
-    if (!block || !block->parentNode() || 
-        isTableCell(block) ||
+    if (!startBlock || !startBlock->parentNode() || 
+        isTableCell(startBlock) ||
+        startBlock->hasTagName(formTag) || 
         canonicalPos.node()->renderer() && canonicalPos.node()->renderer()->isTable() ||
         canonicalPos.node()->hasTagName(hrTag)) {
         applyCommandToComposite(new InsertLineBreakCommand(document()));
@@ -125,8 +125,10 @@ void InsertParagraphSeparatorCommand::doApply()
 
     //---------------------------------------------------------------------
     // Prepare for more general cases.
+    // FIXME: We shouldn't peel off the node here because then we lose track of
+    // the fact that it's the node that belongs to an editing position and
+    // not a rangeCompliantEquivalent.
     Node *startNode = pos.node();
-    Node *startBlock = startNode->enclosingBlockFlowElement();
 
     bool isFirstInBlock = isStartOfBlock(visiblePos);
     bool isLastInBlock = isEndOfBlock(visiblePos);