LayoutTests:
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Jul 2006 19:41:49 +0000 (19:41 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Jul 2006 19:41:49 +0000 (19:41 +0000)
        Reviewed by John Sullivan.

        - test for http://bugzilla.opendarwin.org/show_bug.cgi?id=8952
          <rdar://problem/4575185>
          REGRESSION: crash on drag of highlighted Google custom home page modules

        * fast/dynamic/move-node-with-selection-expected.checksum: Added.
        * fast/dynamic/move-node-with-selection-expected.png: Added.
        * fast/dynamic/move-node-with-selection-expected.txt: Added.
        * fast/dynamic/move-node-with-selection.html: Added.

WebCore:

        Reviewed by John Sullivan.

        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=8952
          <rdar://problem/4575185>
          REGRESSION: crash on drag of highlighted Google custom home page modules

        Test: fast/dynamic/move-node-with-selection.html

        * editing/SelectionController.cpp: (WebCore::SelectionController::nodeWillBeRemoved):
        Call updateRendering before calling clearSelection(), since it's important to do any
        work beforehand, and there are calls inside clearSelection that will indirectly do an
        updateRendering. Also change code to make fewer assumptions about object lifetime.

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

LayoutTests/ChangeLog
LayoutTests/fast/dynamic/8952-reduction-expected.png [new file with mode: 0644]
LayoutTests/fast/dynamic/move-node-with-selection-expected.checksum [new file with mode: 0644]
LayoutTests/fast/dynamic/move-node-with-selection-expected.txt [new file with mode: 0644]
LayoutTests/fast/dynamic/move-node-with-selection.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/SelectionController.cpp

index 3c4782a85b99a3e1da7b13f58cbfb7de2613f51b..848a0c52b8b2b61a9dc3cd77d652a1783106c758 100644 (file)
@@ -1,3 +1,16 @@
+2006-07-15  Darin Adler  <darin@apple.com>
+
+        Reviewed by John Sullivan.
+
+        - test for http://bugzilla.opendarwin.org/show_bug.cgi?id=8952
+          <rdar://problem/4575185>
+          REGRESSION: crash on drag of highlighted Google custom home page modules
+
+        * fast/dynamic/move-node-with-selection-expected.checksum: Added.
+        * fast/dynamic/move-node-with-selection-expected.png: Added.
+        * fast/dynamic/move-node-with-selection-expected.txt: Added.
+        * fast/dynamic/move-node-with-selection.html: Added.
+
 2006-07-15  Darin Adler  <darin@apple.com>
 
         Reviewed by John Sullivan.
diff --git a/LayoutTests/fast/dynamic/8952-reduction-expected.png b/LayoutTests/fast/dynamic/8952-reduction-expected.png
new file mode 100644 (file)
index 0000000..25d63a3
Binary files /dev/null and b/LayoutTests/fast/dynamic/8952-reduction-expected.png differ
diff --git a/LayoutTests/fast/dynamic/move-node-with-selection-expected.checksum b/LayoutTests/fast/dynamic/move-node-with-selection-expected.checksum
new file mode 100644 (file)
index 0000000..3563631
--- /dev/null
@@ -0,0 +1,2 @@
+19fe633c3140d68fc1a7b227d8962007
+\ No newline at end of file
diff --git a/LayoutTests/fast/dynamic/move-node-with-selection-expected.txt b/LayoutTests/fast/dynamic/move-node-with-selection-expected.txt
new file mode 100644 (file)
index 0000000..0026340
--- /dev/null
@@ -0,0 +1,11 @@
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+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 {DIV} at (0,0) size 784x18
+        RenderInline {DIV} at (0,0) size 84x18
+          RenderText {#text} at (0,0) size 84x18
+            text run at (0,0) width 84: "Lorem ipsum"
+      RenderBlock (anonymous) at (0,18) size 784x0
diff --git a/LayoutTests/fast/dynamic/move-node-with-selection.html b/LayoutTests/fast/dynamic/move-node-with-selection.html
new file mode 100644 (file)
index 0000000..1f5f67c
--- /dev/null
@@ -0,0 +1,12 @@
+<div id="dest"></div>
+<div id="t">
+Lorem ipsum
+</div>
+<script type="text/javascript">
+    var t = document.getElementById('t');
+    var dest = document.getElementById('dest');
+
+    window.getSelection().setBaseAndExtent(t.childNodes[0], 4, t.childNodes[0], 8);
+    t.style.display="inline";
+    dest.appendChild(t);
+</script>
index 63464fa2b22932fc485507c8785a74b4d9c54769..4897bead701c6b73d699d5437a67ba3b959a75ce 100644 (file)
@@ -1,3 +1,18 @@
+2006-07-15  Darin Adler  <darin@apple.com>
+
+        Reviewed by John Sullivan.
+
+        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=8952
+          <rdar://problem/4575185>
+          REGRESSION: crash on drag of highlighted Google custom home page modules
+
+        Test: fast/dynamic/move-node-with-selection.html
+
+        * editing/SelectionController.cpp: (WebCore::SelectionController::nodeWillBeRemoved):
+        Call updateRendering before calling clearSelection(), since it's important to do any
+        work beforehand, and there are calls inside clearSelection that will indirectly do an
+        updateRendering. Also change code to make fewer assumptions about object lifetime.
+
 2006-07-15  Darin Adler  <darin@apple.com>
 
         Reviewed by John Sullivan.
index 4b0bcf45df99d288bd8e76534304cada5218b570..a6787a7a0abbaa9f4b50104621b0c344c52bd07e 100644 (file)
@@ -177,20 +177,23 @@ void SelectionController::nodeWillBeRemoved(Node *node)
     Node* start = m_sel.start().node();
     Node* end = m_sel.end().node();
     
-    bool baseRemoved = node == base || base->isAncestor(node);
-    bool extentRemoved = node == extent || extent->isAncestor(node);
-    bool startRemoved = node == start || start->isAncestor(node);
-    bool endRemoved = node == end || end->isAncestor(node);
+    bool baseRemoved = node == base || (base && base->isAncestor(node));
+    bool extentRemoved = node == extent || (extent && extent->isAncestor(node));
+    bool startRemoved = node == start || (start && start->isAncestor(node));
+    bool endRemoved = node == end || (end && end->isAncestor(node));
     
+    bool clearSelection = false;
     if (startRemoved || endRemoved) {
-    
-        // FIXME (6498): This doesn't notify the editing delegate of a selection change.
-
         // FIXME: When endpoints are removed, we should just alter the selection, instead of blowing it away.
-        
-        static_cast<RenderView*>(start->document()->renderer())->clearSelection();
-        setSelection(Selection());
-        
+        clearSelection = true;
+    // FIXME: This could be more efficient if we had an isNodeInRange function on Ranges.
+    } else if (Range::compareBoundaryPoints(m_sel.start(), Position(node, 0)) == -1 &&
+               Range::compareBoundaryPoints(m_sel.end(), Position(node, 0)) == 1) {
+        // If we did nothing here, when this node's renderer was destroyed, the rect that it 
+        // occupied would be invalidated, but, selection gaps that change as a result of 
+        // the removal wouldn't be invalidated.
+        // FIXME: Don't do so much unnecessary invalidation.
+        clearSelection = true;
     } else if (baseRemoved || extentRemoved) {
         if (m_sel.isBaseFirst()) {
             m_sel.setBase(m_sel.start());
@@ -199,14 +202,15 @@ void SelectionController::nodeWillBeRemoved(Node *node)
             m_sel.setBase(m_sel.start());
             m_sel.setExtent(m_sel.end());
         }
-    // FIXME: This could be more efficient if we had an isNodeInRange function on Ranges.
-    } else if (Range::compareBoundaryPoints(m_sel.start(), Position(node, 0)) == -1 &&
-               Range::compareBoundaryPoints(m_sel.end(), Position(node, 0)) == 1) {
-        // If we did nothing here, when this node's renderer was destroyed, the rect that it 
-        // occupied would be invalidated, but, selection gaps that change as a result of 
-        // the removal wouldn't be invalidated.
-        // FIXME: Don't do so much unnecessary invalidation.
-        static_cast<RenderView*>(start->document()->renderer())->clearSelection();
+    }
+
+    if (clearSelection) {
+        // FIXME (6498): This doesn't notify the editing delegate of a selection change.
+        RefPtr<Document> document = start->document();
+        document->updateRendering();
+        if (RenderView* view = static_cast<RenderView*>(document->renderer()))
+            view->clearSelection();
+        setSelection(Selection());
     }
 }