Title: [153815] trunk
- Revision
- 153815
- Author
- [email protected]
- Date
- 2013-08-08 00:16:51 -0700 (Thu, 08 Aug 2013)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (153814 => 153815)
--- trunk/LayoutTests/ChangeLog 2013-08-08 06:28:02 UTC (rev 153814)
+++ trunk/LayoutTests/ChangeLog 2013-08-08 07:16:51 UTC (rev 153815)
@@ -1,3 +1,14 @@
+2013-08-08 Andreas Kling <[email protected]>
+
+ 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 <[email protected]>
[CSS Regions] Propagate overflow from the flow thread to the first and last region
Added: trunk/LayoutTests/fast/repaint/caret-jump-between-nodes-expected.txt (0 => 153815)
--- trunk/LayoutTests/fast/repaint/caret-jump-between-nodes-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/repaint/caret-jump-between-nodes-expected.txt 2013-08-08 07:16:51 UTC (rev 153815)
@@ -0,0 +1,45 @@
+This test verifies that moving the caret between two distinct nodes doesn't leave a ghost behind when the node-relative coordinate offset is identical.
+
+
+(repaint rects
+ (rect 8 44 100 100)
+ (rect 8 44 100 100)
+ (rect 3 39 110 110)
+ (rect 8 44 1 18)
+ (rect 8 44 1 18)
+ (rect 3 39 110 110)
+ (rect -5 -5 5 610)
+ (rect 800 -5 5 610)
+ (rect -5 -5 810 5)
+ (rect -5 600 810 5)
+ (rect -5 -5 5 610)
+ (rect 800 -5 5 610)
+ (rect -5 -5 810 5)
+ (rect -5 600 810 5)
+ (rect 8 44 1 18)
+ (rect 16 44 1 18)
+ (rect 3 39 110 110)
+ (rect 3 39 110 28)
+ (rect 3 57 110 10)
+ (rect 3 57 110 10)
+ (rect 3 57 110 28)
+ (rect 16 44 1 18)
+ (rect 8 62 1 18)
+ (rect 8 62 1 18)
+ (rect 8 44 1 18)
+ (rect 3 39 110 28)
+ (rect 3 49 110 18)
+ (rect 8 44 100 5)
+ (rect 3 57 110 28)
+ (rect 3 39 110 28)
+ (rect 8 44 1 18)
+ (rect 3 39 110 10)
+ (rect 3 39 110 28)
+ (rect 3 39 110 28)
+ (rect 3 57 110 28)
+ (rect 8 44 1 18)
+ (rect 8 44 1 18)
+ (rect 8 44 1 18)
+ (rect 8 62 1 18)
+)
+
Added: trunk/LayoutTests/fast/repaint/caret-jump-between-nodes.html (0 => 153815)
--- trunk/LayoutTests/fast/repaint/caret-jump-between-nodes.html (rev 0)
+++ trunk/LayoutTests/fast/repaint/caret-jump-between-nodes.html 2013-08-08 07:16:51 UTC (rev 153815)
@@ -0,0 +1,25 @@
+<script src="" type="text/_javascript_"></script>
+<script type="text/_javascript_">
+function repaintTest() {
+ var div = document.getElementById("div");
+ window.getSelection().setPosition(div, 0);
+ document.execCommand("InsertText", false, "x");
+ document.execCommand("InsertParagraph");
+ window.getSelection().setPosition(div, 0);
+ document.execCommand("ForwardDelete");
+ window.getSelection().setPosition(div, 1);
+}
+</script>
+<html>
+<style>
+#div {
+ font-family: 12px Ahem;
+ width: 100px;
+ height: 100px;
+}
+</style>
+<body _onload_="runRepaintTest();">
+<div>This test verifies that moving the caret between two distinct nodes doesn't leave a ghost behind when the node-relative coordinate offset is identical.</div>
+<div id="div" contenteditable="true"></div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (153814 => 153815)
--- trunk/Source/WebCore/ChangeLog 2013-08-08 06:28:02 UTC (rev 153814)
+++ trunk/Source/WebCore/ChangeLog 2013-08-08 07:16:51 UTC (rev 153815)
@@ -1,3 +1,25 @@
+2013-08-08 Andreas Kling <[email protected]>
+
+ 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 <[email protected]>
[CSS Regions] Propagate overflow from the flow thread to the first and last region
Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (153814 => 153815)
--- trunk/Source/WebCore/editing/FrameSelection.cpp 2013-08-08 06:28:02 UTC (rev 153814)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp 2013-08-08 07:16:51 UTC (rev 153815)
@@ -1380,27 +1380,29 @@
if (!v)
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;
#if ENABLE(TEXT_CARET)
if (RenderView* view = m_frame->document()->renderView()) {
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);
}
}
#endif
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
