2011-06-16 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Jun 2011 19:11:32 +0000 (19:11 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Jun 2011 19:11:32 +0000 (19:11 +0000)
        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  <rniwa@webkit.org>

        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 <msanchez@igalia.com>.

        Test: editing/selection/move-vertically-with-paddings-borders.html

        * editing/visible_units.cpp:
        (WebCore::previousLinePosition):
        (WebCore::nextLinePosition):
        * rendering/RootInlineBox.h:
        (WebCore::RootInlineBox::blockDirectionPointInLine):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@89056 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/selection/move-vertically-with-paddings-borders-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/move-vertically-with-paddings-borders.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/visible_units.cpp
Source/WebCore/rendering/RootInlineBox.h

index d134b26..bcac3b0 100644 (file)
@@ -1,3 +1,16 @@
+2011-06-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        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  <keunsoon.lee@samsung.com>
 
         Reviewed by Martin Robinson.
diff --git a/LayoutTests/editing/selection/move-vertically-with-paddings-borders-expected.txt b/LayoutTests/editing/selection/move-vertically-with-paddings-borders-expected.txt
new file mode 100644 (file)
index 0000000..ac7a4d4
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/editing/selection/move-vertically-with-paddings-borders.html b/LayoutTests/editing/selection/move-vertically-with-paddings-borders.html
new file mode 100644 (file)
index 0000000..59edfa7
--- /dev/null
@@ -0,0 +1,75 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<style>
+
+#tests p {
+    font-size: 20px;
+    width: 12ex;
+    word-wrap: normal;
+}
+
+</style>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</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="">right1 left2</a> right2</p></li>
+<li><p contenteditable>left1 <a style="border: solid 5px blue;" href="">right1 left2</a> right2</p></li>
+<li><p contenteditable>left1 <a style="padding: 5px;" href="">right1 left2</a> right2</p></li>
+<li><p contenteditable>left1 <a style="padding: 5px;" href="">right1 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="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 8c3be1e..32978eb 100644 (file)
@@ -1,3 +1,23 @@
+2011-06-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        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 <msanchez@igalia.com>.
+
+        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  <keunsoon.lee@samsung.com>
 
         Reviewed by Martin Robinson.
index 8a71ea9..4f52298 100644 (file)
@@ -573,7 +573,7 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
         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 @@ VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int x)
         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.
index d1dde43..d0585f7 100644 (file)
@@ -57,6 +57,8 @@ public:
     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);