Changeset 17089 in webkit

Timestamp:

Oct 17, 2006 12:47:55 PM (18 years ago)

Author:

justing

Message:

LayoutTests:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/repaint/4776765-expected.checksum (added)
• LayoutTests/fast/repaint/4776765-expected.png (added)
• LayoutTests/fast/repaint/4776765-expected.txt (added)
• LayoutTests/fast/repaint/4776765.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/editing/SelectionController.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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:

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/editing/SelectionController.cpp

@@ -777 +777 @@
 }
 
+static IntRect repaintRectForCaret(IntRect caret)
+{
+    if (caret.isEmpty())
+        return IntRect();
+    caret.inflate(1);
+    return caret;
+}
+
 IntRect SelectionController::caretRepaintRect() const
 {
-    // FIXME: Add one pixel of slop on each side to make sure we don't leave behind artifacts.
-    IntRect r = caretRect();
-    if (r.isEmpty())
-        return IntRect();
-    return IntRect(r.x() - 1, r.y() - 1, r.width() + 2, r.height() + 2);
+    return repaintRectForCaret(caretRect());
 }
 
@@ -798 +802 @@
         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;
 }