2011-02-24 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Feb 2011 11:09:14 +0000 (11:09 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Feb 2011 11:09:14 +0000 (11:09 +0000)
        Reviewed by Ojan Vafai.

        Crash when deleting inside a blockquote with a large offset
        https://bugs.webkit.org/show_bug.cgi?id=55098

        The bug was caused by inconsistency in lineBreakExistsAtPosition and breakOutOfEmptyMailBlockquotedParagraph.
        While lineBreakExistsAtPosition was checking that a line break exists at the downstream of the given position,
        breakOutOfEmptyMailBlockquotedParagraph wasn't using the downstream for caretPos. Fixed the bug by using
        the downstream position to instantiate caretPos.

        Co-author: Abhishek Arya <inferno@chromium.org>.

        Test: editing/deleting/delete-blockquote-large-offsets.html

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph):
2011-02-24  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Ojan Vafai.

        Crash when deleting inside a blockquote with a large offset
        https://bugs.webkit.org/show_bug.cgi?id=55098

        Added a test to ensure WebKit doesn't crash when deleting line inside a blockquote
        and the caret is programmatically set to have a large offset.

        Co-author: Abhishek Arya <inferno@chromium.org>.

        * editing/deleting/delete-blockquote-large-offsets-expected.txt: Added.
        * editing/deleting/delete-blockquote-large-offsets.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/deleting/delete-blockquote-large-offsets-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-blockquote-large-offsets.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/CompositeEditCommand.cpp

index 3e7410d..2869292 100644 (file)
@@ -1,3 +1,18 @@
+2011-02-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Ojan Vafai.
+
+        Crash when deleting inside a blockquote with a large offset
+        https://bugs.webkit.org/show_bug.cgi?id=55098
+
+        Added a test to ensure WebKit doesn't crash when deleting line inside a blockquote
+        and the caret is programmatically set to have a large offset.
+
+        Co-author: Abhishek Arya <inferno@chromium.org>.
+
+        * editing/deleting/delete-blockquote-large-offsets-expected.txt: Added.
+        * editing/deleting/delete-blockquote-large-offsets.html: Added.
+
 2011-02-24  MORITA Hajime  <morrita@google.com>
 
         Unreviewed test_expectations.txt update
diff --git a/LayoutTests/editing/deleting/delete-blockquote-large-offsets-expected.txt b/LayoutTests/editing/deleting/delete-blockquote-large-offsets-expected.txt
new file mode 100644 (file)
index 0000000..2ff9f6a
--- /dev/null
@@ -0,0 +1,2 @@
+This tests setting caret inside a blockquote with a large offset and running execCommand('Delete'). WebKit should not crash and you should see PASS below:
+PASS
diff --git a/LayoutTests/editing/deleting/delete-blockquote-large-offsets.html b/LayoutTests/editing/deleting/delete-blockquote-large-offsets.html
new file mode 100644 (file)
index 0000000..d08d724
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+This tests setting caret inside a blockquote with a large offset and running execCommand('Delete').  WebKit should not crash and you should see PASS below:
+<div id="test" contentEditable="true">
+<blockquote type="cite" id="blockquote" style="font-size: 0px; -webkit-min-logical-height: 4px;"><br></blockquote>
+</div>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+window.getSelection().setPosition(document.getElementById('blockquote'), 2000000000);
+document.execCommand("Delete");
+
+var test = document.getElementById('test');
+if (test.innerHTML == '' || test.innerHTML == '<br>') // Allow a placeholder
+    document.writeln('PASS');
+else
+    document.writeln('FAIL: expected empty line but got ' + test.innerHTML.replace('<', '&lt;'));
+
+</script>
+</body>
+</html>
index d64a99d..b76a57e 100644 (file)
@@ -1,3 +1,22 @@
+2011-02-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Ojan Vafai.
+
+        Crash when deleting inside a blockquote with a large offset
+        https://bugs.webkit.org/show_bug.cgi?id=55098
+
+        The bug was caused by inconsistency in lineBreakExistsAtPosition and breakOutOfEmptyMailBlockquotedParagraph.
+        While lineBreakExistsAtPosition was checking that a line break exists at the downstream of the given position,
+        breakOutOfEmptyMailBlockquotedParagraph wasn't using the downstream for caretPos. Fixed the bug by using
+        the downstream position to instantiate caretPos.
+
+        Co-author: Abhishek Arya <inferno@chromium.org>.
+
+        Test: editing/deleting/delete-blockquote-large-offsets.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph):
+
 2011-02-24  Robert Kroeger  <rjkroege@chromium.org>
 
         Reviewed by Darin Fisher.
index 5710f3a..e96f11a 100644 (file)
@@ -1128,8 +1128,8 @@ bool CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph()
     // If this is an empty paragraph there must be a line break here.
     if (!lineBreakExistsAtVisiblePosition(caret))
         return false;
-    
-    Position caretPos(caret.deepEquivalent());
+
+    Position caretPos(caret.deepEquivalent().downstream());
     // A line break is either a br or a preserved newline.
     ASSERT(caretPos.deprecatedNode()->hasTagName(brTag) || (caretPos.deprecatedNode()->isTextNode() && caretPos.deprecatedNode()->renderer()->style()->preserveNewline()));
     
@@ -1137,7 +1137,7 @@ bool CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph()
         Position beforeBR(positionInParentBeforeNode(caretPos.deprecatedNode()));
         removeNode(caretPos.deprecatedNode());
         prune(beforeBR.deprecatedNode());
-    } else {
+    } else if (caretPos.deprecatedNode()->isTextNode()) {
         ASSERT(caretPos.deprecatedEditingOffset() == 0);
         Text* textNode = static_cast<Text*>(caretPos.deprecatedNode());
         ContainerNode* parentNode = textNode->parentNode();
@@ -1146,7 +1146,7 @@ bool CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph()
         deleteTextFromNode(textNode, 0, 1);
         prune(parentNode);
     }
-    
+
     return true;
 }