Changeset 153815 in webkit

Timestamp:

Aug 8, 2013 12:16:51 AM (11 years ago)

Author:

akling@apple.com

Message:

REGRESSION(r139282): Old caret sometimes gets "stuck" (not repainted) in contenteditable elements.
<​http://webkit.org/b/119520>
<rdar://problem/14658929>

Reviewed by Simon Fraser.

Source/WebCore:

When computing a new caret rect, don't forget to repaint the old rect if the caret jumped
from one node to another.
We were being a little too clever, assuming that oldRect==newRect meant that no repaint was
necessary, but moving from (0,0) in one node to (0,0) in another would cause a ghost caret
in the old node.

Test: fast/repaint/caret-jump-between-nodes.html

• editing/FrameSelection.cpp:

(WebCore::FrameSelection::recomputeCaretRect):

Fall through all the way down to caret repaints if the selection's new anchor node is not
the same as m_previousCaretNode.

LayoutTests:

• fast/repaint/caret-jump-between-nodes-expected.txt: Added.
• fast/repaint/caret-jump-between-nodes.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/repaint/caret-jump-between-nodes-expected.txt (added)
• LayoutTests/fast/repaint/caret-jump-between-nodes.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/FrameSelection.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-08-08  Andreas Kling  <akling@apple.com>
+
+        REGRESSION(r139282): Old caret sometimes gets "stuck" (not repainted) in contenteditable elements.
+        <http://webkit.org/b/119520>
+        <rdar://problem/14658929>
+
+        Reviewed by Simon Fraser.
+
+        * fast/repaint/caret-jump-between-nodes-expected.txt: Added.
+        * fast/repaint/caret-jump-between-nodes.html: Added.
+
 2013-08-07  Andrei Bucur  <abucur@adobe.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-08-08  Andreas Kling  <akling@apple.com>
+
+        REGRESSION(r139282): Old caret sometimes gets "stuck" (not repainted) in contenteditable elements.
+        <http://webkit.org/b/119520>
+        <rdar://problem/14658929>
+
+        Reviewed by Simon Fraser.
+
+        When computing a new caret rect, don't forget to repaint the old rect if the caret jumped
+        from one node to another.
+        We were being a little too clever, assuming that oldRect==newRect meant that no repaint was
+        necessary, but moving from (0,0) in one node to (0,0) in another would cause a ghost caret
+        in the old node.
+
+        Test: fast/repaint/caret-jump-between-nodes.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::recomputeCaretRect):
+
+            Fall through all the way down to caret repaints if the selection's new anchor node is not
+            the same as m_previousCaretNode.
+
 2013-08-07  Andrei Bucur  <abucur@adobe.com>
 

trunk/Source/WebCore/editing/FrameSelection.cpp

@@ -1381 +1381 @@
         return false;
 
+    Node* caretNode = m_selection.start().deprecatedNode();
+
     LayoutRect oldRect = localCaretRectWithoutUpdate();
     LayoutRect newRect = localCaretRect();
-    if (oldRect == newRect && !m_absCaretBoundsDirty)
+
+    if (caretNode == m_previousCaretNode && oldRect == newRect && !m_absCaretBoundsDirty)
         return false;
 
     IntRect oldAbsCaretBounds = m_absCaretBounds;
-    m_absCaretBounds = absoluteBoundsForLocalRect(m_selection.start().deprecatedNode(), localCaretRectWithoutUpdate());
+    m_absCaretBounds = absoluteBoundsForLocalRect(caretNode, localCaretRectWithoutUpdate());
     m_absCaretBoundsDirty = false;
-    
-    if (oldAbsCaretBounds == m_absCaretBounds)
+
+    if (caretNode == m_previousCaretNode && oldAbsCaretBounds == m_absCaretBounds)
         return false;
 
@@ -1397 +1400 @@
         bool previousOrNewCaretNodeIsContentEditable = isContentEditable() || (m_previousCaretNode && m_previousCaretNode->isContentEditable());
         if (shouldRepaintCaret(view, previousOrNewCaretNodeIsContentEditable)) {
-            Node* node = m_selection.start().deprecatedNode();
             if (m_previousCaretNode)
                 repaintCaretForLocalRect(m_previousCaretNode.get(), oldRect);
-            m_previousCaretNode = node;
-            repaintCaretForLocalRect(node, newRect);
+            m_previousCaretNode = caretNode;
+            repaintCaretForLocalRect(caretNode, newRect);
         }
     }