Avoid jumpscroll when entering new text in a multi-line editor.

https://bugs.webkit.org/show_bug.cgi?id=82875

Reviewed by Ryosuke Niwa

Scroll caret to the edge of the viewport in case if a line break or a paragraph
separator is inserted at the end of a multi-line editor.  This avoids
undesirable jumpscroll in cases when there is content under the editor.

Tests: editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html
       editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html
       editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html

* editing/Editor.cpp:
(WebCore::Editor::insertLineBreak):
(WebCore::Editor::insertParagraphSeparator):
(WebCore::Editor::revealSelectionAfterEditingOperation):
* editing/Editor.h:
(Editor):

* editing/input/resources/reveal-utilities.js:
(performJumpAtTheEdgeTest):
* editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable-expected.txt: Added.
* editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html: Added.
* editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea-expected.txt: Added.
* editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html: Added.
* editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable-expected.txt: Added.
* editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 280a0a1..8282e59 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,23 @@
+2012-05-16  Mikhail Naganov  <mnaganov@chromium.org>
+
+        Avoid jumpscroll when entering new text in a multi-line editor.
+        https://bugs.webkit.org/show_bug.cgi?id=82875
+
+        Reviewed by Ryosuke Niwa.
+
+        Scroll caret to the edge of the viewport in case if a line break or a paragraph
+        separator is inserted at the end of a multi-line editor.  This avoids
+        undesirable jumpscroll in cases when there is content under the editor.
+
+        * editing/input/resources/reveal-utilities.js:
+        (performJumpAtTheEdgeTest):
+        * editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable-expected.txt: Added.
+        * editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html: Added.
+        * editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea-expected.txt: Added.
+        * editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html: Added.
+        * editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable-expected.txt: Added.
+        * editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html: Added.
+
 2012-05-16  Abhishek Arya  <inferno@chromium.org>
 
         Missing RenderApplet cast check in HTMLAppletElement::renderWidgetForJSBindings.

diff --git a/LayoutTests/editing/input/resources/reveal-utilities.js b/LayoutTests/editing/input/resources/reveal-utilities.js
index bc0dce0..a2ef366 100644 (file)
--- a/LayoutTests/editing/input/resources/reveal-utilities.js
+++ b/LayoutTests/editing/input/resources/reveal-utilities.js
@@ -64,3 +64,27 @@ function performVerticalScrollingPasteTest()
         document.execCommand("paste");
     }
 }
+
+function performJumpAtTheEdgeTest(useCtrlKeyModifier)
+{
+    var textArea = document.getElementById("input");
+    textArea.focus();
+    if (window.eventSender) {
+        var previousScrollTop = 0, currentScrollTop = 0;
+        var jumpDetected = false;
+        for (var i = 0; i < 120; ++i) {
+            previousScrollTop = document.body.scrollTop;
+            eventSender.keyDown("\r", useCtrlKeyModifier ? ["ctrlKey"] : []);
+            currentScrollTop = document.body.scrollTop;
+            // Smooth scrolls are allowed.
+            if (Math.abs(previousScrollTop - currentScrollTop) > 24) {
+                jumpDetected = true;
+                break;
+            }
+        }
+        if (!jumpDetected)
+            document.write("PASS");
+        else
+            document.write("FAIL<br>Jump scroll from " + previousScrollTop + " to " + currentScrollTop);
+    }
+}

diff --git a/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable-expected.txt b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable-expected.txt
new file mode 100644 (file)
index 0000000..ed3bca9
--- /dev/null
+++ b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable-expected.txt
@@ -0,0 +1,123 @@
+When the caret is scrolled out and resides at the end of the contenteditable, on pressing "Ctrl+Return" it must be scrolled to the bottom of the view, not to the center to avoid undesirable content view jumping.
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+PASS

diff --git a/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html
new file mode 100644 (file)
index 0000000..746587b
--- /dev/null
+++ b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<head>
+<script type="text/javascript" src="resources/reveal-utilities.js"></script>
+</head>
+<body>
+<div>When the caret is scrolled out and resides at the end of the contenteditable,
+on pressing &quot;Ctrl+Return&quot; it must be scrolled to the bottom of the view,
+not to the center to avoid undesirable content view jumping.</div>
+<div style="height:1000px;">
+  <div style="overflow:visible; height:100px;" contenteditable="true" id="input"></div>
+</div>
+<script>
+if (window.layoutTestController)
+   layoutTestController.dumpAsText();
+
+performJumpAtTheEdgeTest(true);
+
+</script>
+</body>

diff --git a/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea-expected.txt b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea-expected.txt
new file mode 100644 (file)
index 0000000..4f5015b
--- /dev/null
+++ b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea-expected.txt
@@ -0,0 +1,3 @@
+When the caret is scrolled out and resides at the end of the textarea, on pressing "Return" it must be scrolled to the bottom of the view, not to the center to avoid undesirable content view jumping.
+
+PASS

diff --git a/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html
new file mode 100644 (file)
index 0000000..d056cfa
--- /dev/null
+++ b/LayoutTests/editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<head>
+<script type="text/javascript" src="resources/reveal-utilities.js"></script>
+</head>
+<body>
+<div>When the caret is scrolled out and resides at the end of the textarea,
+on pressing &quot;Return&quot; it must be scrolled to the bottom of the view,
+not to the center to avoid undesirable content view jumping.</div>
+<div style="overflow:auto;height:1000px;">
+  <textarea rows="50" cols="20" id="input"></textarea>
+</div>
+<script>
+if (window.layoutTestController)
+   layoutTestController.dumpAsText();
+
+performJumpAtTheEdgeTest(false);
+
+</script>
+</body>

diff --git a/LayoutTests/editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable-expected.txt b/LayoutTests/editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable-expected.txt
new file mode 100644 (file)
index 0000000..a922c34
--- /dev/null
+++ b/LayoutTests/editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable-expected.txt
@@ -0,0 +1,123 @@
+When the caret is scrolled out and resides at the end of the contenteditable, on pressing "Return" it must be scrolled to the bottom of the view, not to the center to avoid undesirable content view jumping.
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+PASS

diff --git a/LayoutTests/editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html b/LayoutTests/editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html
new file mode 100644 (file)
index 0000000..d3e243e
--- /dev/null
+++ b/LayoutTests/editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<head>
+<script type="text/javascript" src="resources/reveal-utilities.js"></script>
+</head>
+<body>
+<div>When the caret is scrolled out and resides at the end of the contenteditable,
+on pressing &quot;Return&quot; it must be scrolled to the bottom of the view,
+not to the center to avoid undesirable content view jumping.</div>
+<div style="height:1000px;">
+  <div style="overflow:visible; height:100px;" contenteditable="true" id="input"></div>
+</div>
+<script>
+if (window.layoutTestController)
+   layoutTestController.dumpAsText();
+
+performJumpAtTheEdgeTest(false);
+
+</script>
+</body>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 2ca73cc..f19ef28 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2012-05-16  Mikhail Naganov  <mnaganov@chromium.org>
+
+        Avoid jumpscroll when entering new text in a multi-line editor.
+        https://bugs.webkit.org/show_bug.cgi?id=82875
+
+        Reviewed by Ryosuke Niwa.
+
+        Scroll caret to the edge of the viewport in case if a line break or a paragraph
+        separator is inserted at the end of a multi-line editor.  This avoids
+        undesirable jumpscroll in cases when there is content under the editor.
+
+        Tests: editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html
+               editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html
+               editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::insertLineBreak):
+        (WebCore::Editor::insertParagraphSeparator):
+        (WebCore::Editor::revealSelectionAfterEditingOperation):
+        * editing/Editor.h:
+        (Editor):
+
 2012-05-16  Vsevolod Vlasov  <vsevik@chromium.org>
 
         Web Inspector: Pressing esc after requesting snippet creation should remove snippet.

diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
index bf72828..f57f360 100644 (file)
--- a/Source/WebCore/editing/Editor.cpp
+++ b/Source/WebCore/editing/Editor.cpp
@@ -945,9 +945,11 @@ bool Editor::insertLineBreak()
     if (!shouldInsertText("\n", m_frame->selection()->toNormalizedRange().get(), EditorInsertActionTyped))
         return true;
 
+    VisiblePosition caret = m_frame->selection()->selection().visibleStart();
+    bool alignToEdge = isEndOfDocument(caret);
     bool autocorrectionIsApplied = m_alternativeTextController->applyAutocorrectionBeforeTypingIfAppropriate();
     TypingCommand::insertLineBreak(m_frame->document(), autocorrectionIsApplied ? TypingCommand::RetainAutocorrectionIndicator : 0);
-    revealSelectionAfterEditingOperation();
+    revealSelectionAfterEditingOperation(alignToEdge ? ScrollAlignment::alignToEdgeIfNeeded : ScrollAlignment::alignCenterIfNeeded);
 
     return true;
 }
@@ -963,9 +965,11 @@ bool Editor::insertParagraphSeparator()
     if (!shouldInsertText("\n", m_frame->selection()->toNormalizedRange().get(), EditorInsertActionTyped))
         return true;
 
+    VisiblePosition caret = m_frame->selection()->selection().visibleStart();
+    bool alignToEdge = isEndOfDocument(caret);
     bool autocorrectionIsApplied = m_alternativeTextController->applyAutocorrectionBeforeTypingIfAppropriate();
     TypingCommand::insertParagraphSeparator(m_frame->document(), autocorrectionIsApplied ? TypingCommand::RetainAutocorrectionIndicator : 0);
-    revealSelectionAfterEditingOperation();
+    revealSelectionAfterEditingOperation(alignToEdge ? ScrollAlignment::alignToEdgeIfNeeded : ScrollAlignment::alignCenterIfNeeded);
 
     return true;
 }
@@ -2283,12 +2287,12 @@ PassRefPtr<Range> Editor::rangeForPoint(const IntPoint& windowPoint)
     return avoidIntersectionWithNode(selection.toNormalizedRange().get(), m_deleteButtonController->containerElement());
 }
 
-void Editor::revealSelectionAfterEditingOperation()
+void Editor::revealSelectionAfterEditingOperation(const ScrollAlignment& alignment)
 {
     if (m_ignoreCompositionSelectionChange)
         return;
 
-    m_frame->selection()->revealSelection(ScrollAlignment::alignCenterIfNeeded);
+    m_frame->selection()->revealSelection(alignment);
 }
 
 void Editor::setIgnoreCompositionSelectionChange(bool ignore)

diff --git a/Source/WebCore/editing/Editor.h b/Source/WebCore/editing/Editor.h
index 7c875e8..aa073f2 100644 (file)
--- a/Source/WebCore/editing/Editor.h
+++ b/Source/WebCore/editing/Editor.h
@@ -421,7 +421,7 @@ private:
     PassRefPtr<Clipboard> newGeneralClipboard(ClipboardAccessPolicy, Frame*);
     void pasteAsPlainTextWithPasteboard(Pasteboard*);
     void pasteWithPasteboard(Pasteboard*, bool allowPlainText);
-    void revealSelectionAfterEditingOperation();
+    void revealSelectionAfterEditingOperation(const ScrollAlignment& = ScrollAlignment::alignCenterIfNeeded);
     void markMisspellingsOrBadGrammar(const VisibleSelection&, bool checkSpelling, RefPtr<Range>& firstMisspellingRange);
     TextCheckingTypeMask resolveTextCheckingTypeMask(TextCheckingTypeMask);