LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Oct 2006 19:47:55 +0000 (19:47 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Oct 2006 19:47:55 +0000 (19:47 +0000)
        Reviewed by sullivan

        <rdar://problem/4776765>
        REGRESSION: Caret's ghost left behind after inserting a paragraph separator (11237)

        * fast/repaint/4776765-expected.checksum: Added.
        * fast/repaint/4776765-expected.png: Added.
        * fast/repaint/4776765-expected.txt: Added.
        * fast/repaint/4776765.html: Added.

WebCore:

        Reviewed by sullivan

        <rdar://problem/4776765>
        REGRESSION: Caret's ghost left behind after inserting a paragraph separator (11237)

        We set m_needsLayout to false and call caretRect() in the hopes that it will give us
        the old caret rect.  It in fact corrects the caret rect for an offset that it
        believes is due to scrolling but which is actually due to a change in selection
        without an accompanying layout.  So it returns the new caret rect regardless of
        what m_needsLayout is set to.

        * editing/SelectionController.cpp:
        (WebCore::repaintRectForCaret): Moved the code from caretRepaintRect that
        adds a one pixel slop to this new function.
        (WebCore::SelectionController::caretRepaintRect): Moved this code to
        repaintRectForCaret.
        (WebCore::SelectionController::recomputeCaretRect): Compare the old
        caret rect to the new one that's computed with a fresh layout.  If
        they are different, invalidate both repaint rects.

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

LayoutTests/ChangeLog
LayoutTests/fast/repaint/4776765-expected.checksum [new file with mode: 0644]
LayoutTests/fast/repaint/4776765-expected.png [new file with mode: 0644]
LayoutTests/fast/repaint/4776765-expected.txt [new file with mode: 0644]
LayoutTests/fast/repaint/4776765.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/SelectionController.cpp

index d6dd5e6c318e45f23854ffacff216fc8c69d82ee..2fe9a4b74f0c9881d5ce5054c5fb2d33e94ee58e 100644 (file)
@@ -1,8 +1,23 @@
+2006-10-17  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by sullivan
+        
+        <rdar://problem/4776765>
+        REGRESSION: Caret's ghost left behind after inserting a paragraph separator (11237)
+
+        * fast/repaint/4776765-expected.checksum: Added.
+        * fast/repaint/4776765-expected.png: Added.
+        * fast/repaint/4776765-expected.txt: Added.
+        * fast/repaint/4776765.html: Added.
+
 2006-10-17  Alice Liu  <alice.liu@apple.com>
 
         Reviewed by Tim Hatcher.
 
-        When nodes are removed, selections are cleared, and when http://bugs.webkit.org/show_bug.cgi?id=6498 was fixed, we started sending didChangeSelection notifications.  Updating the test results fixes some of the tests mentioned in http://bugs.webkit.org/show_bug.cgi?id=10924
+        When nodes are removed, selections are cleared, and when 
+        http://bugs.webkit.org/show_bug.cgi?id=6498 was fixed, we started 
+        sending didChangeSelection notifications.  Updating the test results 
+        fixes some of the tests mentioned in http://bugs.webkit.org/show_bug.cgi?id=10924
 
         * fast/dynamic/move-node-with-selection-expected.txt:
         * fast/events/dblclick-addEventListener-expected.txt:
diff --git a/LayoutTests/fast/repaint/4776765-expected.checksum b/LayoutTests/fast/repaint/4776765-expected.checksum
new file mode 100644 (file)
index 0000000..fd15f09
--- /dev/null
@@ -0,0 +1 @@
+a5f367ab983b2114cc7626bdf53d1393
\ No newline at end of file
diff --git a/LayoutTests/fast/repaint/4776765-expected.png b/LayoutTests/fast/repaint/4776765-expected.png
new file mode 100644 (file)
index 0000000..0e6d6ad
Binary files /dev/null and b/LayoutTests/fast/repaint/4776765-expected.png differ
diff --git a/LayoutTests/fast/repaint/4776765-expected.txt b/LayoutTests/fast/repaint/4776765-expected.txt
new file mode 100644 (file)
index 0000000..ef006a5
--- /dev/null
@@ -0,0 +1,23 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 5 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+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 784x18
+        RenderText {#text} at (0,0) size 699x18
+          text run at (0,0) width 175: "This tests for a repaint bug. "
+          text run at (175,0) width 524: "The old caret position should be invalidated when a paragraph separator is inserted."
+      RenderBlock {DIV} at (0,34) size 784x54
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderBR {BR} at (0,0) size 0x18
+        RenderBlock {DIV} at (0,18) size 784x18
+          RenderBR {BR} at (0,0) size 0x18
+        RenderBlock {DIV} at (0,36) size 784x18
+          RenderBR {BR} at (0,0) size 0x18
+caret: position 0 of child 0 {BR} of child 4 {DIV} of child 3 {DIV} of child 2 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/fast/repaint/4776765.html b/LayoutTests/fast/repaint/4776765.html
new file mode 100644 (file)
index 0000000..adac887
--- /dev/null
@@ -0,0 +1,19 @@
+<script src="repaint.js" type="text/javascript"></script>
+<script type="text/javascript">
+function repaintTest() {
+    document.execCommand("InsertParagraph");
+}
+</script>
+<body onload="runRepaintTest();">
+<p>This tests for a repaint bug.  The old caret position should be invalidated when a paragraph separator is inserted.</p>
+<div contenteditable="true">
+<div><br></div>
+<div id="div"><br></div>
+</div>
+<script>
+var div = document.getElementById("div");
+var sel = window.getSelection();
+
+sel.setPosition(div, 0);
+</script>
+</body>
index 7d354e2f153b45d47ddd1f83dec84f665a04f1b3..0a9f9c96973bb07c5b704e295e21197f97f43109 100644 (file)
@@ -1,3 +1,25 @@
+2006-10-16  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by sullivan
+        
+        <rdar://problem/4776765>
+        REGRESSION: Caret's ghost left behind after inserting a paragraph separator (11237)
+
+        We set m_needsLayout to false and call caretRect() in the hopes that it will give us 
+        the old caret rect.  It in fact corrects the caret rect for an offset that it 
+        believes is due to scrolling but which is actually due to a change in selection
+        without an accompanying layout.  So it returns the new caret rect regardless of
+        what m_needsLayout is set to.
+        
+        * editing/SelectionController.cpp:
+        (WebCore::repaintRectForCaret): Moved the code from caretRepaintRect that
+        adds a one pixel slop to this new function.
+        (WebCore::SelectionController::caretRepaintRect): Moved this code to
+        repaintRectForCaret.
+        (WebCore::SelectionController::recomputeCaretRect): Compare the old
+        caret rect to the new one that's computed with a fresh layout.  If
+        they are different, invalidate both repaint rects.
+
 2006-10-17  David Harrison  <harrison@apple.com>
 
         Reviewed by Adele.
index 6f23c1e5051e92f6c52144dd97fe4783058871f4..781c48d8db299113e59fafa93d7ac027eb4b2071 100644 (file)
@@ -776,13 +776,17 @@ IntRect SelectionController::caretRect() const
     return caret;
 }
 
-IntRect SelectionController::caretRepaintRect() const
+static IntRect repaintRectForCaret(IntRect caret)
 {
-    // FIXME: Add one pixel of slop on each side to make sure we don't leave behind artifacts.
-    IntRect r = caretRect();
-    if (r.isEmpty())
+    if (caret.isEmpty())
         return IntRect();
-    return IntRect(r.x() - 1, r.y() - 1, r.width() + 2, r.height() + 2);
+    caret.inflate(1);
+    return caret;
+}
+
+IntRect SelectionController::caretRepaintRect() const
+{
+    return repaintRectForCaret(caretRect());
 }
 
 bool SelectionController::recomputeCaretRect()
@@ -797,20 +801,14 @@ bool SelectionController::recomputeCaretRect()
     if (!m_needsLayout)
         return false;
 
-    // Set m_needsLayout to false to get the "old" repaint rect,
-    // since caretRepaintRect currently recomputes the rect if
-    // m_needsLayout is true. It's a problem if that ever happens
-    // outside this function, so we need to change that design in
-    // the future.
-    m_needsLayout = false;
-    IntRect oldRect = caretRepaintRect();
+    IntRect oldRect = m_caretRect;
     m_needsLayout = true;
-    IntRect newRect = caretRepaintRect();
+    IntRect newRect = caretRect();
     if (oldRect == newRect)
         return false;
 
-    v->updateContents(oldRect, false);
-    v->updateContents(newRect, false);
+    v->updateContents(repaintRectForCaret(oldRect), false);
+    v->updateContents(repaintRectForCaret(newRect), false);
     return true;
 }