Title: [143313] trunk
Revision
143313
Author
commit-qu...@webkit.org
Date
2013-02-19 02:14:07 -0800 (Tue, 19 Feb 2013)

Log Message

Caret is not displayed when trying to focus inside a contenteditable element containing an empty block.
https://bugs.webkit.org/show_bug.cgi?id=108053

Source/WebCore:

Patch by Arpita Bahuguna <a....@samsung.com> on 2013-02-19
Reviewed by Ryosuke Niwa.

Test: editing/selection/caret-in-div-containing-empty-block.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::localCaretRect):
When trying to compute the caret rect for the contenteditable div, the
border and the padding were not considered. Because of this, for the
given test case, which had a border defined on the containing div, the
caret was being painted just atop the border, thereby masking it.

Have modified the code to ensure that the computed caret rect takes
into account the border and padding (if any) specified on the box, but only
if the node doesn't have content that shall be skipped for editing.

We do not add border and padding while computing the caret rect for any
element that either has no content or has content that shall be skipped
for editing purposes. This holds true for table elements as well.

This helps avoid the caret displacement previsouly observed before/after
any controls placed within the contenteditable box, when considering
border and padding in computation of the caret rect.

LayoutTests:

Patch by Arpita Bahuguna <arpitabahug...@gmail.com> on 2013-02-19
Reviewed by Ryosuke Niwa.

* editing/selection/caret-in-div-containing-empty-block-expected.txt: Added.
* editing/selection/caret-in-div-containing-empty-block.html: Added.
Layout test added for verifying that a caret is displayed within a
contenteditable div having a border, both for the horizontal
as well as the vertical writing modes.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (143312 => 143313)


--- trunk/LayoutTests/ChangeLog	2013-02-19 09:56:15 UTC (rev 143312)
+++ trunk/LayoutTests/ChangeLog	2013-02-19 10:14:07 UTC (rev 143313)
@@ -1,3 +1,16 @@
+2013-02-19  Arpita Bahuguna  <arpitabahug...@gmail.com>
+
+        Caret is not displayed when trying to focus inside a contenteditable element containing an empty block.
+        https://bugs.webkit.org/show_bug.cgi?id=108053
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/selection/caret-in-div-containing-empty-block-expected.txt: Added.
+        * editing/selection/caret-in-div-containing-empty-block.html: Added.
+        Layout test added for verifying that a caret is displayed within a
+        contenteditable div having a border, both for the horizontal
+        as well as the vertical writing modes.
+
 2013-02-19  Mihnea Ovidenie  <mih...@adobe.com>
 
         CSSRegions: crash positioned object with inline containing block in flow thread

Added: trunk/LayoutTests/editing/selection/caret-in-div-containing-empty-block-expected.txt (0 => 143313)


--- trunk/LayoutTests/editing/selection/caret-in-div-containing-empty-block-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/caret-in-div-containing-empty-block-expected.txt	2013-02-19 10:14:07 UTC (rev 143313)
@@ -0,0 +1,14 @@
+Testcase for bug 108053: Caret is not displayed when trying to focus inside a contenteditable element containing an empty block. To manually verify the issue, click inside the three boxes. A caret should be displayed for each of them.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS horizontalCaretRect.top is textCaretRect.top
+PASS horizontalCaretRect.width is textCaretRect.width
+PASS horizontalCaretRect.height is textCaretRect.height
+PASS verticalCaretRect.top is textCaretRect.left
+PASS verticalCaretRect.width is textCaretRect.height
+PASS verticalCaretRect.height is textCaretRect.width
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/editing/selection/caret-in-div-containing-empty-block.html (0 => 143313)


--- trunk/LayoutTests/editing/selection/caret-in-div-containing-empty-block.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/caret-in-div-containing-empty-block.html	2013-02-19 10:14:07 UTC (rev 143313)
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#textDiv, #horizontalDiv, #verticalDiv{
+    height: 200px;
+    width: 200px;
+    border: 1px solid black;
+}
+#verticalDiv {
+    -webkit-writing-mode: vertical-rl;
+}
+</style>
+<script src=""
+<script>
+function runTest() {
+    description('Testcase for bug <a href="" Caret is not displayed when trying to focus inside a contenteditable element containing an empty block.\nTo manually verify the issue, click inside the three boxes. A caret should be displayed for each of them.');
+
+    if (window.internals) {
+        var textDiv = document.getElementById('textDiv');
+        textDiv.focus();
+        textCaretRect = internals.absoluteCaretBounds(document);
+
+        var horizontalDiv = document.getElementById('horizontalDiv');
+        horizontalDiv.focus();
+        horizontalCaretRect = internals.absoluteCaretBounds(document);
+
+        var verticalDiv = document.getElementById('verticalDiv');
+        verticalDiv.focus();
+        verticalCaretRect = internals.absoluteCaretBounds(document);
+        
+        shouldBe("horizontalCaretRect.top", "textCaretRect.top");
+        shouldBe("horizontalCaretRect.width", "textCaretRect.width");
+        shouldBe("horizontalCaretRect.height", "textCaretRect.height");
+        shouldBe("verticalCaretRect.top", "textCaretRect.left");
+        shouldBe("verticalCaretRect.width", "textCaretRect.height");
+        shouldBe("verticalCaretRect.height", "textCaretRect.width");
+
+        isSuccessfullyParsed();
+        
+        textDiv.style.display = 'none';
+        horizontalDiv.style.display = 'none';
+        verticalDiv.style.display = 'none';
+    }
+}
+</script>
+</head>
+<body _onload_="runTest();">
+<div id="textDiv" contenteditable="true" style="float: left;">Some text.<p></p></div>
+<div id="horizontalDiv" contenteditable="true" style="float: left;"><p></p></div>
+<div id="verticalDiv" contenteditable="true"><p></p></div>
+<div id="description"></div>
+<div id="console"></div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (143312 => 143313)


--- trunk/Source/WebCore/ChangeLog	2013-02-19 09:56:15 UTC (rev 143312)
+++ trunk/Source/WebCore/ChangeLog	2013-02-19 10:14:07 UTC (rev 143313)
@@ -1,3 +1,31 @@
+2013-02-19  Arpita Bahuguna  <a....@samsung.com>
+
+        Caret is not displayed when trying to focus inside a contenteditable element containing an empty block.
+        https://bugs.webkit.org/show_bug.cgi?id=108053
+
+        Reviewed by Ryosuke Niwa.
+
+        Test: editing/selection/caret-in-div-containing-empty-block.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::localCaretRect):
+        When trying to compute the caret rect for the contenteditable div, the
+        border and the padding were not considered. Because of this, for the
+        given test case, which had a border defined on the containing div, the
+        caret was being painted just atop the border, thereby masking it.
+
+        Have modified the code to ensure that the computed caret rect takes
+        into account the border and padding (if any) specified on the box, but only
+        if the node doesn't have content that shall be skipped for editing.
+
+        We do not add border and padding while computing the caret rect for any
+        element that either has no content or has content that shall be skipped
+        for editing purposes. This holds true for table elements as well.
+
+        This helps avoid the caret displacement previsouly observed before/after
+        any controls placed within the contenteditable box, when considering
+        border and padding in computation of the caret rect.
+
 2013-02-19  Mihnea Ovidenie  <mih...@adobe.com>
 
         CSSRegions: crash positioned object with inline containing block in flow thread

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (143312 => 143313)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2013-02-19 09:56:15 UTC (rev 143312)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2013-02-19 10:14:07 UTC (rev 143313)
@@ -3819,7 +3819,6 @@
     // They never refer to children.
     // FIXME: Paint the carets inside empty blocks differently than the carets before/after elements.
 
-    // FIXME: What about border and padding?
     LayoutRect rect(location(), LayoutSize(caretWidth, height()));
     bool ltr = box ? box->isLeftToRightDirection() : style()->isLeftToRightDirection();
 
@@ -3851,6 +3850,14 @@
     // Move to local coords
     rect.moveBy(-location());
 
+    // FIXME: Border/padding should be added for all elements but this workaround
+    // is needed because we use offsets inside an "atomic" element to represent
+    // positions before and after the element in deprecated editing offsets.
+    if (!(editingIgnoresContent(node()) || isTableElement(node()))) {
+        rect.setX(rect.x() + borderLeft() + paddingLeft());
+        rect.setY(rect.y() + paddingTop() + borderTop());
+    }
+
     if (!isHorizontalWritingMode())
         return rect.transposedRect();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to