- Revision
- 139166
- Author
- [email protected]
- Date
- 2013-01-08 22:55:58 -0800 (Tue, 08 Jan 2013)
Log Message
Caret is incorrectly painted for a contenteditable <div> containing a <br> in vertical writing mode
https://bugs.webkit.org/show_bug.cgi?id=103621
Patch by Arpita Bahuguna <[email protected]> on 2013-01-08
Reviewed by Ryosuke Niwa.
Source/WebCore:
While computing the caret rect for the given specific scenario, we
canonicalize our position. For the upstream (or downstream) block flow
candidates a check to ignore the nodes having renderers with zero height
is carried out. This is where we fail our check in the vertical writing mode.
In the vertical writing mode, instead of verifying the height of the descendants
of the candidate nodes, a check for their width should be carried out.
For our case, i.e. the <br> element contained inside the div, the bounding box
in the vertical writing mode would have a width greater than zero and height
equal to zero (as is to be expected in the vertical mode).
Thus, we need to make a check against the logical height. For the vertical
writing mode, the logical height should return the width of the computed
bounding box.
Test: editing/selection/caret-in-div-containing-br-in-vertical-mode.html
* dom/Position.cpp:
(WebCore::Position::hasRenderedNonAnonymousDescendantsWithHeight):
Have modified the code to verify against the logical height, instead
of simply the height, both in case of RenderText and RenderBox.
The logical height of the computed rects returns a value according
to the writing mode.
For the RenderText a new function, linesLogicalBoundingBox() is called
which returns the values depending on the writing mode.
Similarly, on the RenderBox, pixelSnappedLogicalHeight() is called which
too takes care of the writing mode internally. (borderBoundingBox()
internally calls on the pixelSnappedRect).
* rendering/RenderText.cpp:
(WebCore::RenderText::linesLogicalBoundingBox):
(WebCore):
* rendering/RenderText.h:
(RenderText):
New function is added to return the logical linesBoundingBox, i.e.,
the width and height of the linesBoundingBox are set according to
the writing mode.
LayoutTests:
* editing/selection/caret-in-div-containing-br-in-vertical-mode-expected.txt: Added.
* editing/selection/caret-in-div-containing-br-in-vertical-mode.html: Added.
Testcase added for verifying the caret display at the start of an editable
div containing only a <br> element in vertical writing mode.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (139165 => 139166)
--- trunk/LayoutTests/ChangeLog 2013-01-09 06:42:55 UTC (rev 139165)
+++ trunk/LayoutTests/ChangeLog 2013-01-09 06:55:58 UTC (rev 139166)
@@ -1,3 +1,15 @@
+2013-01-08 Arpita Bahuguna <[email protected]>
+
+ Caret is incorrectly painted for a contenteditable <div> containing a <br> in vertical writing mode
+ https://bugs.webkit.org/show_bug.cgi?id=103621
+
+ Reviewed by Ryosuke Niwa.
+
+ * editing/selection/caret-in-div-containing-br-in-vertical-mode-expected.txt: Added.
+ * editing/selection/caret-in-div-containing-br-in-vertical-mode.html: Added.
+ Testcase added for verifying the caret display at the start of an editable
+ div containing only a <br> element in vertical writing mode.
+
2013-01-08 Csaba Osztrogonác <[email protected]>
[Qt] Unreviewed gardening, skip new failing tests.
Added: trunk/LayoutTests/editing/selection/caret-in-div-containing-br-in-vertical-mode-expected.txt (0 => 139166)
--- trunk/LayoutTests/editing/selection/caret-in-div-containing-br-in-vertical-mode-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/caret-in-div-containing-br-in-vertical-mode-expected.txt 2013-01-09 06:55:58 UTC (rev 139166)
@@ -0,0 +1,11 @@
+Testcase for bug 103621: Caret is incorrectly painted for a contenteditable <div> containing a <br> in vertical writing mode. For the test to pass, carets in both the boxes should be exactly similar. To manually verify the issue, try clicking on the second box. A caret should be visible at the start of that container.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS withTextCaretRect.top is withoutTextCaretRect.top
+PASS withTextCaretRect.width is withoutTextCaretRect.width
+PASS withTextCaretRect.height is withoutTextCaretRect.height
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/selection/caret-in-div-containing-br-in-vertical-mode.html (0 => 139166)
--- trunk/LayoutTests/editing/selection/caret-in-div-containing-br-in-vertical-mode.html (rev 0)
+++ trunk/LayoutTests/editing/selection/caret-in-div-containing-br-in-vertical-mode.html 2013-01-09 06:55:58 UTC (rev 139166)
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.editableDiv {
+ height: 200px;
+ width: 200px;
+ border: 1px solid black;
+ -webkit-writing-mode: vertical-rl;
+}
+</style>
+<script src=""
+<script>
+function runTest() {
+ description('Testcase for bug <a href="" Caret is incorrectly painted for a contenteditable <div> containing a <br> in vertical writing mode.\nFor the test to pass, carets in both the boxes should be exactly similar.\nTo manually verify the issue, try clicking on the second box. A caret should be visible at the start of that container.');
+
+ document.getElementById('textDiv').focus();
+ if (window.internals)
+ withTextCaretRect = internals.absoluteCaretBounds(document);
+
+ document.getElementById('noTextDiv').focus();
+ if (window.internals)
+ withoutTextCaretRect = internals.absoluteCaretBounds(document);
+
+ if (window.internals) {
+ shouldBe("withTextCaretRect.top", "withoutTextCaretRect.top");
+ shouldBe("withTextCaretRect.width", "withoutTextCaretRect.width");
+ shouldBe("withTextCaretRect.height", "withoutTextCaretRect.height");
+
+ document.getElementById('textDiv').style.display = 'none';
+ document.getElementById('noTextDiv').style.display = 'none';
+ }
+
+ isSuccessfullyParsed();
+}
+</script>
+</head>
+<body _onload_="runTest();">
+<div contenteditable="true" style="float: left;" class="editableDiv" id="textDiv"><br>some text</div>
+<div contenteditable="true" class="editableDiv" id="noTextDiv"><br></div>
+<div id="description"></div>
+<div id="console"></div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (139165 => 139166)
--- trunk/Source/WebCore/ChangeLog 2013-01-09 06:42:55 UTC (rev 139165)
+++ trunk/Source/WebCore/ChangeLog 2013-01-09 06:55:58 UTC (rev 139166)
@@ -1,3 +1,50 @@
+2013-01-08 Arpita Bahuguna <[email protected]>
+
+ Caret is incorrectly painted for a contenteditable <div> containing a <br> in vertical writing mode
+ https://bugs.webkit.org/show_bug.cgi?id=103621
+
+ Reviewed by Ryosuke Niwa.
+
+ While computing the caret rect for the given specific scenario, we
+ canonicalize our position. For the upstream (or downstream) block flow
+ candidates a check to ignore the nodes having renderers with zero height
+ is carried out. This is where we fail our check in the vertical writing mode.
+
+ In the vertical writing mode, instead of verifying the height of the descendants
+ of the candidate nodes, a check for their width should be carried out.
+
+ For our case, i.e. the <br> element contained inside the div, the bounding box
+ in the vertical writing mode would have a width greater than zero and height
+ equal to zero (as is to be expected in the vertical mode).
+
+ Thus, we need to make a check against the logical height. For the vertical
+ writing mode, the logical height should return the width of the computed
+ bounding box.
+
+ Test: editing/selection/caret-in-div-containing-br-in-vertical-mode.html
+
+ * dom/Position.cpp:
+ (WebCore::Position::hasRenderedNonAnonymousDescendantsWithHeight):
+ Have modified the code to verify against the logical height, instead
+ of simply the height, both in case of RenderText and RenderBox.
+ The logical height of the computed rects returns a value according
+ to the writing mode.
+
+ For the RenderText a new function, linesLogicalBoundingBox() is called
+ which returns the values depending on the writing mode.
+ Similarly, on the RenderBox, pixelSnappedLogicalHeight() is called which
+ too takes care of the writing mode internally. (borderBoundingBox()
+ internally calls on the pixelSnappedRect).
+
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::linesLogicalBoundingBox):
+ (WebCore):
+ * rendering/RenderText.h:
+ (RenderText):
+ New function is added to return the logical linesBoundingBox, i.e.,
+ the width and height of the linesBoundingBox are set according to
+ the writing mode.
+
2013-01-08 Tony Gentilcore <[email protected]>
Remove a few unused includes from HTMLTreeBuilder
Modified: trunk/Source/WebCore/dom/Position.cpp (139165 => 139166)
--- trunk/Source/WebCore/dom/Position.cpp 2013-01-09 06:42:55 UTC (rev 139165)
+++ trunk/Source/WebCore/dom/Position.cpp 2013-01-09 06:55:58 UTC (rev 139166)
@@ -846,8 +846,8 @@
RenderObject* stop = renderer->nextInPreOrderAfterChildren();
for (RenderObject *o = renderer->firstChild(); o && o != stop; o = o->nextInPreOrder())
if (o->nonPseudoNode()) {
- if ((o->isText() && toRenderText(o)->linesBoundingBox().height()) ||
- (o->isBox() && toRenderBox(o)->borderBoundingBox().height()))
+ if ((o->isText() && toRenderText(o)->linesLogicalBoundingBox().height())
+ || (o->isBox() && toRenderBox(o)->pixelSnappedLogicalHeight()))
return true;
}
return false;
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (139165 => 139166)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2013-01-09 06:42:55 UTC (rev 139165)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2013-01-09 06:55:58 UTC (rev 139166)
@@ -1610,6 +1610,13 @@
return result;
}
+IntRect RenderText::linesLogicalBoundingBox() const
+{
+ IntRect rect = linesBoundingBox();
+
+ return style()->isHorizontalWritingMode() ? rect : IntRect(rect.x(), rect.y(), rect.height(), rect.width());
+}
+
LayoutRect RenderText::linesVisualOverflowBoundingBox() const
{
if (!firstTextBox())
Modified: trunk/Source/WebCore/rendering/RenderText.h (139165 => 139166)
--- trunk/Source/WebCore/rendering/RenderText.h 2013-01-09 06:42:55 UTC (rev 139165)
+++ trunk/Source/WebCore/rendering/RenderText.h 2013-01-09 06:55:58 UTC (rev 139166)
@@ -96,6 +96,7 @@
virtual IntRect linesBoundingBox() const;
LayoutRect linesVisualOverflowBoundingBox() const;
+ IntRect linesLogicalBoundingBox() const;
FloatPoint firstRunOrigin() const;
float firstRunX() const;
