Title: [95574] trunk
Revision
95574
Author
rn...@webkit.org
Date
2011-09-20 15:33:48 -0700 (Tue, 20 Sep 2011)

Log Message

Hit testing on margins of body and head elements doesn't recur
https://bugs.webkit.org/show_bug.cgi?id=40753

Reviewed by Darin Adler.

Source/WebCore: 

The bug was caused by positionForPointRespectingEditingBoundaries's comparing the editability
of head/body and html elements when hit testing was done inside margins of head and body elements.

Fixed the bug by special-casing html element since margins of head and body elements are special.

Tests: editing/selection/click-on-body-margin.html
       editing/selection/click-on-head-margin.html

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

LayoutTests: 

Add tests to click on margins of head and body elements. WebKit should not
(attempt to) place the caret after or before head and body elements.

* editing/selection/click-on-body-margin-expected.txt: Added.
* editing/selection/click-on-body-margin.html: Added.
* editing/selection/click-on-head-margin-expected.txt: Added.
* editing/selection/click-on-head-margin.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (95573 => 95574)


--- trunk/LayoutTests/ChangeLog	2011-09-20 22:23:41 UTC (rev 95573)
+++ trunk/LayoutTests/ChangeLog	2011-09-20 22:33:48 UTC (rev 95574)
@@ -1,3 +1,18 @@
+2011-09-20  Ryosuke Niwa  <rn...@webkit.org>
+
+        Hit testing on margins of body and head elements doesn't recur
+        https://bugs.webkit.org/show_bug.cgi?id=40753
+
+        Reviewed by Darin Adler.
+
+        Add tests to click on margins of head and body elements. WebKit should not
+        (attempt to) place the caret after or before head and body elements.
+
+        * editing/selection/click-on-body-margin-expected.txt: Added.
+        * editing/selection/click-on-body-margin.html: Added.
+        * editing/selection/click-on-head-margin-expected.txt: Added.
+        * editing/selection/click-on-head-margin.html: Added.
+
 2011-09-20  Alan Stearns  <stea...@adobe.com>
 
         [CSSRegions] Need test case for 66198 (sliced line box rendering)

Added: trunk/LayoutTests/editing/selection/click-on-body-margin-expected.txt (0 => 95574)


--- trunk/LayoutTests/editing/selection/click-on-body-margin-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/click-on-body-margin-expected.txt	2011-09-20 22:33:48 UTC (rev 95574)
@@ -0,0 +1,4 @@
+Click on the right of this line outside the black box.
+The caret should be placed on the right of the first line, NOT on the right of this line.
+PASS
+

Added: trunk/LayoutTests/editing/selection/click-on-body-margin.html (0 => 95574)


--- trunk/LayoutTests/editing/selection/click-on-body-margin.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/click-on-body-margin.html	2011-09-20 22:33:48 UTC (rev 95574)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<body style="white-space: nowrap; margin:100px; border: solid 1px black;" contenteditable>
+<span id="firstLine">Click on the right of this line outside the black box.</span><br>
+<span id="longLine">The caret should be placed on the right of the first line, NOT on the right of this line.
+<span></span></span>
+<pre><script>
+
+var longLine = document.getElementById('longLine');
+while (longLine.offsetWidth < document.body.offsetWidth + 200)
+    longLine.lastChild.textContent += ' some text';
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+
+    var firstLine = document.getElementById('firstLine');
+    eventSender.mouseMoveTo(firstLine.offsetLeft + document.body.offsetWidth + 10,
+        firstLine.offsetTop + firstLine.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    if (!getSelection().isCollapsed)
+        document.writeln('FAIL - selection was not collapsed');
+    else if (getSelection().baseNode != firstLine.firstChild)
+        document.writeln('FAIL - caret was not in the first line');
+    else if (getSelection().baseOffset != firstLine.textContent.length)
+        document.writeln('FAIL - caret was not on the right edge');
+    else
+        document.writeln('PASS');
+
+    longLine.lastChild.style.display = 'none';
+}
+
+</script></pre>
+</body>
+</html>

Added: trunk/LayoutTests/editing/selection/click-on-head-margin-expected.txt (0 => 95574)


--- trunk/LayoutTests/editing/selection/click-on-head-margin-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/click-on-head-margin-expected.txt	2011-09-20 22:33:48 UTC (rev 95574)
@@ -0,0 +1,4 @@
+Click on the right of this line outside the black box.
+The caret should be placed on the right of the first line, NOT on the right of this line.
+ PASS
+

Added: trunk/LayoutTests/editing/selection/click-on-head-margin.html (0 => 95574)


--- trunk/LayoutTests/editing/selection/click-on-head-margin.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/click-on-head-margin.html	2011-09-20 22:33:48 UTC (rev 95574)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head contenteditable style="display: block; white-space: nowrap; margin:100px; border: solid 1px black;"></head>
+<body>
+<div id="firstLine">Click on the right of this line outside the black box.</div>
+<span id="longLine">The caret should be placed on the right of the first line, NOT on the right of this line.<span></span></span>
+<pre><script>
+
+var head = document.getElementsByTagName('head')[0];
+var longLine = document.getElementById('longLine');
+var firstLine = document.getElementById('firstLine');
+
+// Work-around HTML5 parser.
+head.appendChild(firstLine);
+head.appendChild(longLine);
+
+while (longLine.offsetWidth < head.offsetWidth + 200)
+    longLine.lastChild.textContent += ' some text';
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+
+    eventSender.mouseMoveTo(firstLine.offsetLeft + head.offsetWidth + 10,
+        100 + firstLine.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    if (!getSelection().isCollapsed)
+        document.writeln('FAIL - selection was not collapsed');
+    else if (getSelection().baseNode != firstLine.firstChild)
+        document.writeln('FAIL - caret was not in the first line');
+    else if (getSelection().baseOffset != firstLine.textContent.length)
+        document.writeln('FAIL - caret was not on the right edge');
+    else
+        document.writeln('PASS');
+
+    longLine.lastChild.style.display = 'none';
+}
+
+</script></pre>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (95573 => 95574)


--- trunk/Source/WebCore/ChangeLog	2011-09-20 22:23:41 UTC (rev 95573)
+++ trunk/Source/WebCore/ChangeLog	2011-09-20 22:33:48 UTC (rev 95574)
@@ -1,3 +1,21 @@
+2011-09-20  Ryosuke Niwa  <rn...@webkit.org>
+
+        Hit testing on margins of body and head elements doesn't recur
+        https://bugs.webkit.org/show_bug.cgi?id=40753
+
+        Reviewed by Darin Adler.
+
+        The bug was caused by positionForPointRespectingEditingBoundaries's comparing the editability
+        of head/body and html elements when hit testing was done inside margins of head and body elements.
+
+        Fixed the bug by special-casing html element since margins of head and body elements are special.
+
+        Tests: editing/selection/click-on-body-margin.html
+               editing/selection/click-on-head-margin.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::positionForPointRespectingEditingBoundaries):
+
 2011-09-20  David Hyatt  <hy...@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=68480

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (95573 => 95574)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2011-09-20 22:23:41 UTC (rev 95573)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2011-09-20 22:33:48 UTC (rev 95574)
@@ -4209,6 +4209,14 @@
     return createLegacyEditingPosition(box->renderer()->node(), start ? textBox->start() : textBox->start() + textBox->len());
 }
 
+static inline bool isEditingBoundary(RenderObject* ancestor, RenderObject* child)
+{
+    ASSERT(!ancestor || ancestor->node());
+    ASSERT(child && child->node());
+    return !ancestor || !ancestor->parent() || (ancestor->hasLayer() && ancestor->parent()->isRenderView())
+        || ancestor->node()->rendererIsEditable() == child->node()->rendererIsEditable();
+}
+
 // FIXME: This function should go on RenderObject as an instance method. Then
 // all cases in which positionForPoint recurs could call this instead to
 // prevent crossing editable boundaries. This would require many tests.
@@ -4232,7 +4240,7 @@
         ancestor = ancestor->parent();
 
     // If we can't find an ancestor to check editability on, or editability is unchanged, we recur like normal
-    if (!ancestor || ancestor->node()->rendererIsEditable() == childNode->rendererIsEditable())
+    if (isEditingBoundary(ancestor, child))
         return child->positionForPoint(pointInChildCoordinates);
 
     // Otherwise return before or after the child, depending on if the click was to the logical left or logical right of the child
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to