Hit testing on margins of body and head elements doesn't recur
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2011 22:33:48 +0000 (22:33 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2011 22:33:48 +0000 (22:33 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/click-on-body-margin-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/click-on-body-margin.html [new file with mode: 0644]
LayoutTests/editing/selection/click-on-head-margin-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/click-on-head-margin.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index 7ea4dd7..2e7ea99 100644 (file)
@@ -1,3 +1,18 @@
+2011-09-20  Ryosuke Niwa  <rniwa@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  <stearns@adobe.com>
 
         [CSSRegions] Need test case for 66198 (sliced line box rendering)
diff --git a/LayoutTests/editing/selection/click-on-body-margin-expected.txt b/LayoutTests/editing/selection/click-on-body-margin-expected.txt
new file mode 100644 (file)
index 0000000..a1aeb55
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/editing/selection/click-on-body-margin.html b/LayoutTests/editing/selection/click-on-body-margin.html
new file mode 100644 (file)
index 0000000..64f5a75
--- /dev/null
@@ -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>
diff --git a/LayoutTests/editing/selection/click-on-head-margin-expected.txt b/LayoutTests/editing/selection/click-on-head-margin-expected.txt
new file mode 100644 (file)
index 0000000..92e2f9b
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/editing/selection/click-on-head-margin.html b/LayoutTests/editing/selection/click-on-head-margin.html
new file mode 100644 (file)
index 0000000..be8c5db
--- /dev/null
@@ -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>
index 74a7ddd..39692c1 100644 (file)
@@ -1,3 +1,21 @@
+2011-09-20  Ryosuke Niwa  <rniwa@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  <hyatt@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=68480
index 3510b20..1552dda 100644 (file)
@@ -4209,6 +4209,14 @@ Position RenderBlock::positionForBox(InlineBox *box, bool start) const
     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 @@ static VisiblePosition positionForPointRespectingEditingBoundaries(RenderBlock*
         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