Incorrect selection with absolutely positioned div
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2011 21:51:44 +0000 (21:51 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2011 21:51:44 +0000 (21:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=39503

Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

The bug was caused by a false assumption in RenderBlock::positionForPoint. Because the last child box
can be positioned, floated, invisible, etc..., we can't always trust last child's logicalTop to tell us
whether a given point is inside or below the last child box.

Fixed the bug by using the last hit-test candidate instead.

Test: editing/selection/block-with-positioned-lastchild.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::positionForPoint):

LayoutTests:

Added a regression test for placing the caret inside a block with multiple logical lines
with an absolutely positioned last child. WebKit should place the caret on the left of the first line
(instead of after the last line) when the user clicks on the left of the first line.

* editing/selection/block-with-positioned-lastchild-expected.txt: Added.
* editing/selection/block-with-positioned-lastchild.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/block-with-positioned-lastchild-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/block-with-positioned-lastchild.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index 23f420e..90a429f 100644 (file)
@@ -1,3 +1,17 @@
+2011-09-19  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Incorrect selection with absolutely positioned div
+        https://bugs.webkit.org/show_bug.cgi?id=39503
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Added a regression test for placing the caret inside a block with multiple logical lines
+        with an absolutely positioned last child. WebKit should place the caret on the left of the first line
+        (instead of after the last line) when the user clicks on the left of the first line.
+
+        * editing/selection/block-with-positioned-lastchild-expected.txt: Added.
+        * editing/selection/block-with-positioned-lastchild.html: Added.
+
 2011-09-19  Abhishek Arya  <inferno@chromium.org>
 
         Unreviewed. Chromium Rebaselines for r95461.
diff --git a/LayoutTests/editing/selection/block-with-positioned-lastchild-expected.txt b/LayoutTests/editing/selection/block-with-positioned-lastchild-expected.txt
new file mode 100644 (file)
index 0000000..7f4585a
--- /dev/null
@@ -0,0 +1,4 @@
+Click on the left of this line.
+Caret should NOT be placed in this line,
+PASS
+
diff --git a/LayoutTests/editing/selection/block-with-positioned-lastchild.html b/LayoutTests/editing/selection/block-with-positioned-lastchild.html
new file mode 100644 (file)
index 0000000..939503d
--- /dev/null
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div style="padding: 20px;" contenteditable>Click on the left of this line.
+<div>Caret should NOT be placed in this line,</div>
+<div style="position:absolute; top:0px; right:0px;"></div>
+</div>
+<pre><script>
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+
+    var container = document.body.children[0];
+    eventSender.mouseMoveTo(container.offsetLeft + 5, container.offsetTop + 5);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    if (!getSelection().isCollapsed)
+        document.writeln('FAIL - selection was not collapsed');
+    else if (getSelection().baseNode != container.firstChild)
+        document.writeln('FAIL - caret was not in the first line');
+    else if (getSelection().baseOffset)
+        document.writeln('FAIL - caret was not on the left edge');
+    else
+        document.writeln('PASS');
+}
+
+</script></pre>
+</body>
+</html>
index 87bae2a..3ce1e22 100644 (file)
@@ -1,3 +1,21 @@
+2011-09-19  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Incorrect selection with absolutely positioned div
+        https://bugs.webkit.org/show_bug.cgi?id=39503
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        The bug was caused by a false assumption in RenderBlock::positionForPoint. Because the last child box
+        can be positioned, floated, invisible, etc..., we can't always trust last child's logicalTop to tell us
+        whether a given point is inside or below the last child box.
+
+        Fixed the bug by using the last hit-test candidate instead.
+
+        Test: editing/selection/block-with-positioned-lastchild.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::positionForPoint):
+
 2011-09-19  Dmitry Titov  <dimich@chromium.org>
 
         [Chromium] Crash after magic iframe transfer for Pepper/NaCl plugins.
index 3aa675a..206dbab 100644 (file)
@@ -4336,12 +4336,14 @@ VisiblePosition RenderBlock::positionForPoint(const LayoutPoint& point)
     if (childrenInline())
         return positionForPointWithInlineChildren(pointInLogicalContents);
 
-    if (lastChildBox() && pointInContents.y() > lastChildBox()->logicalTop()) {
-        for (RenderBox* childBox = lastChildBox(); childBox; childBox = childBox->previousSiblingBox()) {
-            if (isChildHitTestCandidate(childBox))
-                return positionForPointRespectingEditingBoundaries(this, childBox, pointInContents);
-        }
-    } else {
+    RenderBox* lastCandidateBox = lastChildBox();
+    while (lastCandidateBox && !isChildHitTestCandidate(lastCandidateBox))
+        lastCandidateBox = lastCandidateBox->previousSiblingBox();
+
+    if (lastCandidateBox) {
+        if (pointInContents.y() > lastCandidateBox->logicalTop())
+            return positionForPointRespectingEditingBoundaries(this, lastCandidateBox, pointInContents);
+
         for (RenderBox* childBox = firstChildBox(); childBox; childBox = childBox->nextSiblingBox()) {
             // We hit child if our click is above the bottom of its padding box (like IE6/7 and FF3).
             if (isChildHitTestCandidate(childBox) && pointInContents.y() < childBox->logicalBottom())