- Revision
- 87936
- Author
- [email protected]
- Date
- 2011-06-02 12:05:52 -0700 (Thu, 02 Jun 2011)
Log Message
2011-06-01 Ryosuke Niwa <[email protected]>
Reviewed by Simon Fraser.
REGRESSION: Text selection broken for text with line-height applied
https://bugs.webkit.org/show_bug.cgi?id=54929
The bug was caused by RenderText::positionForPoint's not considering the case where a point is
above selectionTop and below lineTop of the first root inline box. Fixed the bug by considering
any point between selectionTop and lineTop to be inside a root inline box. This condition is
consistent with the condition we use to determine the bottom of a line.
Test: editing/selection/hit-test-on-text-with-line-height.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::positionForPointWithInlineChildren): Fixed a condition to determine whether
or not a point is above the first root line box. We need to check both selectionTop and logicalTop
for the same reason explained above.
* rendering/RenderText.cpp:
(WebCore::RenderText::positionForPoint): See above.
2011-06-01 Ryosuke Niwa <[email protected]>
Reviewed by Simon Fraser.
REGRESSION: Text selection broken for text with line-height applied
https://bugs.webkit.org/show_bug.cgi?id=54929
Added a test to ensure WebKit can place caret in text with a line-height smaller than
the height of the text.
* editing/selection/hit-test-on-text-with-line-height-expected.txt: Added.
* editing/selection/hit-test-on-text-with-line-height.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (87935 => 87936)
--- trunk/LayoutTests/ChangeLog 2011-06-02 18:57:50 UTC (rev 87935)
+++ trunk/LayoutTests/ChangeLog 2011-06-02 19:05:52 UTC (rev 87936)
@@ -1,3 +1,16 @@
+2011-06-01 Ryosuke Niwa <[email protected]>
+
+ Reviewed by Simon Fraser.
+
+ REGRESSION: Text selection broken for text with line-height applied
+ https://bugs.webkit.org/show_bug.cgi?id=54929
+
+ Added a test to ensure WebKit can place caret in text with a line-height smaller than
+ the height of the text.
+
+ * editing/selection/hit-test-on-text-with-line-height-expected.txt: Added.
+ * editing/selection/hit-test-on-text-with-line-height.html: Added.
+
2011-06-02 Adam Barth <[email protected]>
This test actually fails as IMAGE not as IMAGE+TEXT.
Added: trunk/LayoutTests/editing/selection/hit-test-on-text-with-line-height-expected.txt (0 => 87936)
--- trunk/LayoutTests/editing/selection/hit-test-on-text-with-line-height-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/hit-test-on-text-with-line-height-expected.txt 2011-06-02 19:05:52 UTC (rev 87936)
@@ -0,0 +1,24 @@
+This test ensures WebKit can place caret after the line even when the line-height is smaller than a line. To manually test, click inside the black box outside the red box. The caret should be placed at the end of "hello".
+
+Also test that when you click in the red box above or below the black box, caret is placed at where you clicked.
+
+PASS Click after hello
+PASS Click after hello (top)
+PASS Click after hello (bottom)
+
+Click above black box
+PASS before h
+PASS before e
+PASS before l
+PASS before l
+PASS before o
+PASS after "hello"
+
+Click below black box
+PASS before h
+PASS before e
+PASS before l
+PASS before l
+PASS before o
+PASS after "hello"
+
Added: trunk/LayoutTests/editing/selection/hit-test-on-text-with-line-height.html (0 => 87936)
--- trunk/LayoutTests/editing/selection/hit-test-on-text-with-line-height.html (rev 0)
+++ trunk/LayoutTests/editing/selection/hit-test-on-text-with-line-height.html 2011-06-02 19:05:52 UTC (rev 87936)
@@ -0,0 +1,78 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href=""
+<script src=""
+<style>
+#test { width: 300px; font-size: 50px; line-height: 10px; border: solid 1px black; padding: 5px; }
+#test span { border: solid 1px red; }
+</style>
+</head>
+<body>
+<p>This test ensures WebKit can place caret after the line even when the line-height is smaller than a line.
+To manually test, click inside the black box outside the red box.
+The caret should be placed at the end of "hello".</p>
+<p>Also test that when you click in the red box above or below the black box, caret is placed at where you clicked.</p>
+<div style="padding: 50px;">
+<div id="test" contenteditable><span>hello</span></div>
+</div>
+<div id="console"></div>
+<script>
+
+var test = document.getElementById('test');
+var span = test.firstChild;
+var textNode = span.firstChild;
+
+function clickAndVerify(title, x, y, expectedOffset) {
+ eventSender.mouseMoveTo(x, y);
+ eventSender.mouseDown();
+ eventSender.leapForward(500);
+ eventSender.mouseUp();
+
+ var selection = window.getSelection();
+ if (!selection.isCollapsed)
+ testFailed(title + ' - selection was not collapsed');
+ else if (selection.baseNode != textNode)
+ testFailed(title + ' - baseNode was not "' + textNode.textContent + '"');
+ else if (selection.baseOffset != expectedOffset)
+ testFailed(title + ' - caret was at ' + selection.baseOffset + ' but expected to be at ' + expectedOffset);
+ else
+ testPassed(title);
+}
+
+function clickBetweenEachLetterAndVerify(y) {
+ for (var i = 0; i <= textNode.textContent.length; i++) {
+ x = span.offsetLeft + span.offsetWidth * i / 5;
+ x = Math.max(span.offsetLeft + 1, Math.min(span.offsetLeft + span.offsetWidth - 1, x));
+ if (i == textNode.textContent.length)
+ title = 'after "' + textNode.textContent + '"';
+ else
+ title = 'before ' + textNode.textContent.charAt(i);
+ clickAndVerify(title, x, y, i);
+ }
+}
+
+if (window.layoutTestController && !window.eventSender)
+ testFailed('This test requires eventSender');
+else if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ clickAndVerify('Click after hello', test.offsetLeft + test.offsetWidth - 5, test.offsetTop + test.offsetHeight / 2, 5);
+ clickAndVerify('Click after hello (top)', test.offsetLeft + test.offsetWidth - 5, test.offsetTop + 1, 5);
+ clickAndVerify('Click after hello (bottom)', test.offsetLeft + test.offsetWidth - 5, test.offsetTop + test.offsetHeight - 1, 5);
+
+ debug('');
+ debug('Click above black box');
+ clickBetweenEachLetterAndVerify(span.offsetTop + 1);
+
+ debug('');
+ debug('Click below black box');
+ clickBetweenEachLetterAndVerify(span.offsetTop + span.offsetHeight - 1);
+
+ test.style.display = 'none';
+}
+
+var successfullyParsed = true;
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (87935 => 87936)
--- trunk/Source/WebCore/ChangeLog 2011-06-02 18:57:50 UTC (rev 87935)
+++ trunk/Source/WebCore/ChangeLog 2011-06-02 19:05:52 UTC (rev 87936)
@@ -1,3 +1,24 @@
+2011-06-01 Ryosuke Niwa <[email protected]>
+
+ Reviewed by Simon Fraser.
+
+ REGRESSION: Text selection broken for text with line-height applied
+ https://bugs.webkit.org/show_bug.cgi?id=54929
+
+ The bug was caused by RenderText::positionForPoint's not considering the case where a point is
+ above selectionTop and below lineTop of the first root inline box. Fixed the bug by considering
+ any point between selectionTop and lineTop to be inside a root inline box. This condition is
+ consistent with the condition we use to determine the bottom of a line.
+
+ Test: editing/selection/hit-test-on-text-with-line-height.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::positionForPointWithInlineChildren): Fixed a condition to determine whether
+ or not a point is above the first root line box. We need to check both selectionTop and logicalTop
+ for the same reason explained above.
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::positionForPoint): See above.
+
2011-06-02 Andreas Kling <[email protected]>
Reviewed by James Robinson.
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (87935 => 87936)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2011-06-02 18:57:50 UTC (rev 87935)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2011-06-02 19:05:52 UTC (rev 87936)
@@ -4138,7 +4138,8 @@
}
if (closestBox) {
- if (moveCaretToBoundary && pointInLogicalContents.y() < firstRootBoxWithChildren->selectionTop()) {
+ if (moveCaretToBoundary && pointInLogicalContents.y() < firstRootBoxWithChildren->selectionTop()
+ && pointInLogicalContents.y() < firstRootBoxWithChildren->logicalTop()) {
// y coordinate is above first root line box, so return the start of the first
return VisiblePosition(positionForBox(firstRootBoxWithChildren->firstLeafChild(), true), DOWNSTREAM);
}
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (87935 => 87936)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2011-06-02 18:57:50 UTC (rev 87935)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2011-06-02 19:05:52 UTC (rev 87936)
@@ -496,7 +496,7 @@
InlineTextBox* lastBoxAbove = 0;
for (InlineTextBox* box = firstTextBox(); box; box = box->nextTextBox()) {
RootInlineBox* rootBox = box->root();
- if (pointBlockDirection >= rootBox->selectionTop()) {
+ if (pointBlockDirection >= rootBox->selectionTop() || pointBlockDirection >= rootBox->lineTop()) {
int bottom = rootBox->selectionBottom();
if (rootBox->nextRootBox())
bottom = min(bottom, rootBox->nextRootBox()->lineTop());
