WebCore:
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Apr 2009 21:22:30 +0000 (21:22 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Apr 2009 21:22:30 +0000 (21:22 +0000)
        Reviewed by Justin Garcia.

        - fix <rdar://problem/6081309> Mail crash when pressing down arrow in
          some messages in WebCore::canHaveChildrenForEditing

        Test: editing/selection/extend-by-line-anonymous-content-crash.html

        * editing/visible_units.cpp:
        (WebCore::previousLinePosition): Null-check node. If p is not an
        editable position, then closestLeafChildForXPos() may have returned a
        non-editable box, and in particular one belonging to anonymous content.
        If node is 0, fall back on RenderObject::positionForPoint, which
        finds the closest position in non-anonymous content.
        (WebCore::nextLinePosition): Ditto.
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::createVisiblePosition): Fixed a typo.

LayoutTests:

        Reviewed by Justin Garcia.

        - test for <rdar://problem/6081309> Mail crash when pressing down arrow
          in some messages in WebCore::canHaveChildrenForEditing

        * editing/selection/extend-by-line-anonymous-content-crash-expected.txt: Added.
        * editing/selection/extend-by-line-anonymous-content-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/extend-by-line-anonymous-content-crash-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/extend-by-line-anonymous-content-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/visible_units.cpp
WebCore/rendering/RenderObject.cpp

index 4977940..7a05a80 100644 (file)
@@ -1,3 +1,13 @@
+2009-04-23  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Justin Garcia.
+
+        - test for <rdar://problem/6081309> Mail crash when pressing down arrow
+          in some messages in WebCore::canHaveChildrenForEditing
+
+        * editing/selection/extend-by-line-anonymous-content-crash-expected.txt: Added.
+        * editing/selection/extend-by-line-anonymous-content-crash.html: Added.
+
 2009-04-23  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Adele Peterson.
 2009-04-23  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Adele Peterson.
diff --git a/LayoutTests/editing/selection/extend-by-line-anonymous-content-crash-expected.txt b/LayoutTests/editing/selection/extend-by-line-anonymous-content-crash-expected.txt
new file mode 100644 (file)
index 0000000..e40a89c
--- /dev/null
@@ -0,0 +1,4 @@
+Test for rdar://problem/6081309, a crash when moving or extending the selection with line granularity.
+
+foo
+baz
diff --git a/LayoutTests/editing/selection/extend-by-line-anonymous-content-crash.html b/LayoutTests/editing/selection/extend-by-line-anonymous-content-crash.html
new file mode 100644 (file)
index 0000000..7a8f12d
--- /dev/null
@@ -0,0 +1,17 @@
+<style>
+    #b:before { content: "bar"; }
+</style>
+<p>
+    Test for <a href="rdar://problem/6081309">rdar://problem/6081309</a>, a crash when moving or extending the selection with line granularity.
+</p>
+<div id="target">foo</div>
+<div id="b">baz</div>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    var sel = getSelection();
+    var start = document.getElementById("target").firstChild;
+    sel.setBaseAndExtent(start, 0, start, 1);
+    sel.modify("extend", "forward", "line");
+</script>
index cb559c6..974be06 100644 (file)
@@ -1,3 +1,22 @@
+2009-04-23  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Justin Garcia.
+
+        - fix <rdar://problem/6081309> Mail crash when pressing down arrow in
+          some messages in WebCore::canHaveChildrenForEditing
+
+        Test: editing/selection/extend-by-line-anonymous-content-crash.html
+
+        * editing/visible_units.cpp:
+        (WebCore::previousLinePosition): Null-check node. If p is not an
+        editable position, then closestLeafChildForXPos() may have returned a
+        non-editable box, and in particular one belonging to anonymous content.
+        If node is 0, fall back on RenderObject::positionForPoint, which
+        finds the closest position in non-anonymous content.
+        (WebCore::nextLinePosition): Ditto.
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::createVisiblePosition): Fixed a typo.
+
 2009-04-23  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Adele Peterson.
 2009-04-23  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Adele Peterson.
index 21818b0..00f33c3 100644 (file)
@@ -600,15 +600,15 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
             absPos -= containingBlock->layer()->scrolledContentOffset();
         RenderObject* renderer = root->closestLeafChildForXPos(x - absPos.x(), isEditablePosition(p))->renderer();
         Node* node = renderer->node();
             absPos -= containingBlock->layer()->scrolledContentOffset();
         RenderObject* renderer = root->closestLeafChildForXPos(x - absPos.x(), isEditablePosition(p))->renderer();
         Node* node = renderer->node();
-        if (editingIgnoresContent(node))
+        if (node && editingIgnoresContent(node))
             return Position(node->parent(), node->nodeIndex());
             return Position(node->parent(), node->nodeIndex());
-        return renderer->positionForCoordinates(x - absPos.x(), root->topOverflow());
+        return renderer->positionForPoint(IntPoint(x - absPos.x(), root->topOverflow()));
     }
     
     // Could not find a previous line. This means we must already be on the first line.
     // Move to the start of the content in this block, which effectively moves us
     // to the start of the line we're on.
     }
     
     // Could not find a previous line. This means we must already be on the first line.
     // Move to the start of the content in this block, which effectively moves us
     // to the start of the line we're on.
-    Node* rootElement = node->isContentEditable() ? node->rootEditableElement() : node->document()->documentElement();
+    Element* rootElement = node->isContentEditable() ? node->rootEditableElement() : node->document()->documentElement();
     return VisiblePosition(rootElement, 0, DOWNSTREAM);
 }
 
     return VisiblePosition(rootElement, 0, DOWNSTREAM);
 }
 
@@ -701,9 +701,9 @@ VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int x)
             absPos -= containingBlock->layer()->scrolledContentOffset();
         RenderObject* renderer = root->closestLeafChildForXPos(x - absPos.x(), isEditablePosition(p))->renderer();
         Node* node = renderer->node();
             absPos -= containingBlock->layer()->scrolledContentOffset();
         RenderObject* renderer = root->closestLeafChildForXPos(x - absPos.x(), isEditablePosition(p))->renderer();
         Node* node = renderer->node();
-        if (editingIgnoresContent(node))
+        if (node && editingIgnoresContent(node))
             return Position(node->parent(), node->nodeIndex());
             return Position(node->parent(), node->nodeIndex());
-        return renderer->positionForCoordinates(x - absPos.x(), root->topOverflow());
+        return renderer->positionForPoint(IntPoint(x - absPos.x(), root->topOverflow()));
     }    
 
     // Could not find a next line. This means we must already be on the last line.
     }    
 
     // Could not find a next line. This means we must already be on the last line.
index 2fe55dc..8050f6f 100644 (file)
@@ -2303,7 +2303,7 @@ RenderBoxModelObject* RenderObject::offsetParent() const
 
 VisiblePosition RenderObject::createVisiblePosition(int offset, EAffinity affinity)
 {
 
 VisiblePosition RenderObject::createVisiblePosition(int offset, EAffinity affinity)
 {
-    // If is is a non-anonymous renderer, then it's simple.
+    // If this is a non-anonymous renderer, then it's simple.
     if (Node* node = this->node())
         return VisiblePosition(node, offset, affinity);
 
     if (Node* node = this->node())
         return VisiblePosition(node, offset, affinity);