Changeset 143313 in webkit

Timestamp:

Feb 19, 2013 2:14:07 AM (11 years ago)

Author:

commit-queue@webkit.org

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.bah@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 <​arpitabahuguna@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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/selection/caret-in-div-containing-empty-block-expected.txt (added)
• LayoutTests/editing/selection/caret-in-div-containing-empty-block.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-02-19  Arpita Bahuguna  <arpitabahuguna@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  <mihnea@adobe.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-19  Arpita Bahuguna  <a.bah@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  <mihnea@adobe.com>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -3820 +3820 @@
     // 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 +3850 @@
     // 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())