- Revision
- 89056
- Author
- [email protected]
- Date
- 2011-06-16 12:11:32 -0700 (Thu, 16 Jun 2011)
Log Message
2011-06-16 Ryosuke Niwa <[email protected]>
Reviewed by Eric Seidel.
Consider padding and border when looking for the next/previous line position
https://bugs.webkit.org/show_bug.cgi?id=55481
Added a test to ensure WebKit can allow vertical caret movements even when
inline elements that span multiple lines have paddings, borders, or both.
* editing/selection/move-vertically-with-paddings-borders-expected.txt: Added.
* editing/selection/move-vertically-with-paddings-borders.html: Added.
2011-06-16 Ryosuke Niwa <[email protected]>
Reviewed by Eric Seidel.
Consider padding and border when looking for the next/previous line position
https://bugs.webkit.org/show_bug.cgi?id=55481
The bug was caused by previousLinePosition and nextLinePosition passing y coordinate
above the line in some cases. Fixed the bug by passing the larger of selectionTop and logicalTop.
This patch is based on a patch originally written by Mario Sanchez Prada <[email protected]>.
Test: editing/selection/move-vertically-with-paddings-borders.html
* editing/visible_units.cpp:
(WebCore::previousLinePosition):
(WebCore::nextLinePosition):
* rendering/RootInlineBox.h:
(WebCore::RootInlineBox::blockDirectionPointInLine):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (89055 => 89056)
--- trunk/LayoutTests/ChangeLog 2011-06-16 18:54:09 UTC (rev 89055)
+++ trunk/LayoutTests/ChangeLog 2011-06-16 19:11:32 UTC (rev 89056)
@@ -1,3 +1,16 @@
+2011-06-16 Ryosuke Niwa <[email protected]>
+
+ Reviewed by Eric Seidel.
+
+ Consider padding and border when looking for the next/previous line position
+ https://bugs.webkit.org/show_bug.cgi?id=55481
+
+ Added a test to ensure WebKit can allow vertical caret movements even when
+ inline elements that span multiple lines have paddings, borders, or both.
+
+ * editing/selection/move-vertically-with-paddings-borders-expected.txt: Added.
+ * editing/selection/move-vertically-with-paddings-borders.html: Added.
+
2011-06-16 Keunsoon Lee <[email protected]>
Reviewed by Martin Robinson.
Added: trunk/LayoutTests/editing/selection/move-vertically-with-paddings-borders-expected.txt (0 => 89056)
--- trunk/LayoutTests/editing/selection/move-vertically-with-paddings-borders-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/move-vertically-with-paddings-borders-expected.txt 2011-06-16 19:11:32 UTC (rev 89056)
@@ -0,0 +1,34 @@
+This test ensures WebKit takes paddings and borders into account when moving vertically.
+
+test 1
+PASS selectWord() is "left2"
+PASS selectWord() is "right2"
+PASS selectWord() is "left1"
+PASS selectWord() is "right1"
+
+test 2
+PASS selectWord() is "left2"
+PASS selectWord() is "right2"
+PASS selectWord() is "left1"
+PASS selectWord() is "right1"
+
+test 3
+PASS selectWord() is "left2"
+PASS selectWord() is "right2"
+PASS selectWord() is "left1"
+PASS selectWord() is "right1"
+
+test 4
+PASS selectWord() is "left2"
+PASS selectWord() is "right2"
+PASS selectWord() is "left1"
+PASS selectWord() is "right1"
+PASS selectWord() is "left3"
+PASS selectWord() is "right3"
+PASS selectWord() is "left2"
+PASS selectWord() is "right2"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/selection/move-vertically-with-paddings-borders.html (0 => 89056)
--- trunk/LayoutTests/editing/selection/move-vertically-with-paddings-borders.html (rev 0)
+++ trunk/LayoutTests/editing/selection/move-vertically-with-paddings-borders.html 2011-06-16 19:11:32 UTC (rev 89056)
@@ -0,0 +1,75 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href=""
+<style>
+
+#tests p {
+ font-size: 20px;
+ width: 12ex;
+ word-wrap: normal;
+}
+
+</style>
+<script src=""
+</head>
+<body>
+<p>This test ensures WebKit takes paddings and borders into account when moving vertically.</p>
+<ol id="tests">
+<li><p contenteditable>left1 <a href="" left2</a> right2</p></li>
+<li><p contenteditable>left1 <a style="border: solid 5px blue;" href="" left2</a> right2</p></li>
+<li><p contenteditable>left1 <a style="padding: 5px;" href="" left2</a> right2</p></li>
+<li><p contenteditable>left1 <a style="padding: 5px;" href="" left2 right2 left3</a> right3</p></li>
+</ol>
+<div id="console"></div>
+<script>
+
+function moveToMiddleOfWord(node, word) {
+ window.getSelection().setPosition(node, 0);
+ if (!window.find(word))
+ return false;
+ window.getSelection().modify('move', 'backward', 'character');
+ window.getSelection().modify('move', 'forward', 'character');
+ window.getSelection().modify('move', 'forward', 'character');
+ return true;
+}
+
+function selectWord() {
+ window.getSelection().modify('move', 'backward', 'word');
+ window.getSelection().modify('extend', 'forward', 'word');
+ return window.getSelection().toString();
+}
+
+function moveVerticallyAndVerify(node, direction, from, to) {
+ if (node.innerText.indexOf(from) === -1 || node.innerText.indexOf(to) === -1)
+ return;
+ if (!moveToMiddleOfWord(node, from))
+ return;
+ window.getSelection().modify('move', direction, 'line');
+ shouldBe('selectWord()', '"' + to + '"');
+}
+
+var tests = document.getElementById('tests').getElementsByTagName('p');
+for (var i = 0; i < tests.length; i++) {
+ var node = tests[i];
+
+ debug('test ' + (i + 1));
+
+ node.focus();
+ for (var j = 1; j <= 2; j++) {
+ moveVerticallyAndVerify(node, 'forward', 'left' + j, 'left' + (j + 1));
+ moveVerticallyAndVerify(node, 'forward', 'right' + j, 'right' + (j + 1));
+ moveVerticallyAndVerify(node, 'backward', 'left' + (j + 1), 'left' + j);
+ moveVerticallyAndVerify(node, 'backward', 'right' + (j + 1), 'right' + j);
+ }
+
+ debug('');
+}
+
+document.getElementById('tests').style.display = 'none';
+var successfullyParsed = true;
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (89055 => 89056)
--- trunk/Source/WebCore/ChangeLog 2011-06-16 18:54:09 UTC (rev 89055)
+++ trunk/Source/WebCore/ChangeLog 2011-06-16 19:11:32 UTC (rev 89056)
@@ -1,3 +1,23 @@
+2011-06-16 Ryosuke Niwa <[email protected]>
+
+ Reviewed by Eric Seidel.
+
+ Consider padding and border when looking for the next/previous line position
+ https://bugs.webkit.org/show_bug.cgi?id=55481
+
+ The bug was caused by previousLinePosition and nextLinePosition passing y coordinate
+ above the line in some cases. Fixed the bug by passing the larger of selectionTop and logicalTop.
+
+ This patch is based on a patch originally written by Mario Sanchez Prada <[email protected]>.
+
+ Test: editing/selection/move-vertically-with-paddings-borders.html
+
+ * editing/visible_units.cpp:
+ (WebCore::previousLinePosition):
+ (WebCore::nextLinePosition):
+ * rendering/RootInlineBox.h:
+ (WebCore::RootInlineBox::blockDirectionPointInLine):
+
2011-06-16 Keunsoon Lee <[email protected]>
Reviewed by Martin Robinson.
Modified: trunk/Source/WebCore/editing/visible_units.cpp (89055 => 89056)
--- trunk/Source/WebCore/editing/visible_units.cpp 2011-06-16 18:54:09 UTC (rev 89055)
+++ trunk/Source/WebCore/editing/visible_units.cpp 2011-06-16 19:11:32 UTC (rev 89056)
@@ -573,7 +573,7 @@
Node* node = renderer->node();
if (node && editingIgnoresContent(node))
return positionInParentBeforeNode(node);
- return renderer->positionForPoint(IntPoint(x - absPos.x(), root->lineTop()));
+ return renderer->positionForPoint(IntPoint(x - absPos.x(), root->blockDirectionPointInLine()));
}
// Could not find a previous line. This means we must already be on the first line.
@@ -680,7 +680,7 @@
Node* node = renderer->node();
if (node && editingIgnoresContent(node))
return positionInParentBeforeNode(node);
- return renderer->positionForPoint(IntPoint(x - absPos.x(), root->lineTop()));
+ return renderer->positionForPoint(IntPoint(x - absPos.x(), root->blockDirectionPointInLine()));
}
// Could not find a next line. This means we must already be on the last line.
Modified: trunk/Source/WebCore/rendering/RootInlineBox.h (89055 => 89056)
--- trunk/Source/WebCore/rendering/RootInlineBox.h 2011-06-16 18:54:09 UTC (rev 89055)
+++ trunk/Source/WebCore/rendering/RootInlineBox.h 2011-06-16 19:11:32 UTC (rev 89056)
@@ -57,6 +57,8 @@
int selectionBottom() const;
int selectionHeight() const { return max(0, selectionBottom() - selectionTop()); }
+ int blockDirectionPointInLine() const { return max(lineTop(), selectionTop()); }
+
int alignBoxesInBlockDirection(int heightOfBlock, GlyphOverflowAndFallbackFontsMap&, VerticalPositionCache&);
void setLineTopBottomPositions(int top, int bottom);
