WebCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Dec 2008 23:02:11 +0000 (23:02 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Dec 2008 23:02:11 +0000 (23:02 +0000)
2008-12-13  Darin Adler  <darin@apple.com>

        Reviewed by Dan Bernstein.

        - fix https://bugs.webkit.org/show_bug.cgi?id=18734
          REGRESSION (r31081): Focus problems in Gmail 2/Plain text message text
          <rdar://problem/5892415>

        Test: fast/forms/textarea-selection-preservation.html

        The regression reported was caused by the fact that the renderer code had
        a bug where it would constantly think the newline at the end of text was
        missing, and so it would replace all the text even though it wasn't changing,
        which would destroy the selection.

        When writing the regression test I discovered another problem: The value
        property in HTMLTextAreaElement was intentionally changing the selection
        to the end of the textarea, but doing that even when the value wasn't changing.

        This patch fixes both and the test checks both.

        * html/HTMLTextAreaElement.cpp:
        (WebCore::HTMLTextAreaElement::setValue): Exit early if the value is
        not changing.

        * rendering/RenderTextControl.cpp:
        (WebCore::RenderTextControl::text): Add a newline character for each <br>
        element encountered in the control

LayoutTests:

2008-12-13  Darin Adler  <darin@apple.com>

        Reviewed by Dan Bernstein.

        - test for https://bugs.webkit.org/show_bug.cgi?id=18734
          REGRESSION (r31081): Focus problems in Gmail 2/Plain text message text
          <rdar://problem/5892415>

        * fast/forms/textarea-selection-preservation-expected.txt: Added.
        * fast/forms/textarea-selection-preservation.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/textarea-selection-preservation-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/textarea-selection-preservation.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/html/HTMLTextAreaElement.cpp
WebCore/rendering/RenderTextControl.cpp

index 3acf74a..571547b 100644 (file)
@@ -1,3 +1,14 @@
+2008-12-13  Darin Adler  <darin@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        - test for https://bugs.webkit.org/show_bug.cgi?id=18734
+          REGRESSION (r31081): Focus problems in Gmail 2/Plain text message text
+          <rdar://problem/5892415>
+
+        * fast/forms/textarea-selection-preservation-expected.txt: Added.
+        * fast/forms/textarea-selection-preservation.html: Added.
+
 2008-12-13  Holger Hans Peter Freyther  <zecke@selfish.org>
 
         Reviewed by Dan Bernstein.
diff --git a/LayoutTests/fast/forms/textarea-selection-preservation-expected.txt b/LayoutTests/fast/forms/textarea-selection-preservation-expected.txt
new file mode 100644 (file)
index 0000000..6f1d82e
--- /dev/null
@@ -0,0 +1,26 @@
+This tests for problems where we'd lose the selection in a textarea when making style and value changes.
+
+PASS ta.selectionStart is 3
+PASS ta.selectionEnd is 4
+- add background style
+PASS ta.selectionStart is 3
+PASS ta.selectionEnd is 4
+- set value to same value
+PASS ta.selectionStart is 3
+PASS ta.selectionEnd is 4
+- set value to a different value
+PASS ta.selectionStart is 6
+PASS ta.selectionEnd is 6
+- set selection so we can test again without a trailing newline
+PASS ta.selectionStart is 3
+PASS ta.selectionEnd is 4
+- add background style
+PASS ta.selectionStart is 3
+PASS ta.selectionEnd is 4
+- set value to same value
+PASS ta.selectionStart is 3
+PASS ta.selectionEnd is 4
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/textarea-selection-preservation.html b/LayoutTests/fast/forms/textarea-selection-preservation.html
new file mode 100644 (file)
index 0000000..0d6d73a
--- /dev/null
@@ -0,0 +1,47 @@
+<html>
+<head>
+<link rel="stylesheet" type="text/css" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p>This tests for problems where we'd lose the selection in a textarea when making style and value changes.</p>
+<div id="console"></div>
+<p><textarea id="ta">abc123
+</textarea></p>
+<script type="text/javascript">
+    var ta = document.getElementById('ta');
+    ta.selectionStart = 3;
+    ta.selectionEnd = 4;
+    shouldBe('ta.selectionStart', '3');
+    shouldBe('ta.selectionEnd', '4');
+    debug("- add background style");
+    ta.setAttribute("style", "background-color: yellow");
+    shouldBe('ta.selectionStart', '3');
+    shouldBe('ta.selectionEnd', '4');
+    debug("- set value to same value");
+    ta.value = "abc123\n";
+    shouldBe('ta.selectionStart', '3');
+    shouldBe('ta.selectionEnd', '4');
+    debug("- set value to a different value");
+    ta.value = "abc123";
+    shouldBe('ta.selectionStart', '6');
+    shouldBe('ta.selectionEnd', '6');
+    debug("- set selection so we can test again without a trailing newline");
+    ta.selectionStart = 3;
+    ta.selectionEnd = 4;
+    ta.removeAttribute("style");
+    shouldBe('ta.selectionStart', '3');
+    shouldBe('ta.selectionEnd', '4');
+    debug("- add background style");
+    ta.setAttribute("style", "background-color: yellow");
+    shouldBe('ta.selectionStart', '3');
+    shouldBe('ta.selectionEnd', '4');
+    debug("- set value to same value");
+    ta.value = "abc123";
+    shouldBe('ta.selectionStart', '3');
+    shouldBe('ta.selectionEnd', '4');
+    var successfullyParsed = true;
+</script>
+<script type="text/javascript" src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 0f156de..5bd4e9c 100644 (file)
@@ -1,5 +1,34 @@
 2008-12-13  Darin Adler  <darin@apple.com>
 
+        Reviewed by Dan Bernstein.
+
+        - fix https://bugs.webkit.org/show_bug.cgi?id=18734
+          REGRESSION (r31081): Focus problems in Gmail 2/Plain text message text
+          <rdar://problem/5892415>
+
+        Test: fast/forms/textarea-selection-preservation.html
+
+        The regression reported was caused by the fact that the renderer code had
+        a bug where it would constantly think the newline at the end of text was
+        missing, and so it would replace all the text even though it wasn't changing,
+        which would destroy the selection.
+
+        When writing the regression test I discovered another problem: The value
+        property in HTMLTextAreaElement was intentionally changing the selection
+        to the end of the textarea, but doing that even when the value wasn't changing.
+
+        This patch fixes both and the test checks both.
+
+        * html/HTMLTextAreaElement.cpp:
+        (WebCore::HTMLTextAreaElement::setValue): Exit early if the value is
+        not changing.
+
+        * rendering/RenderTextControl.cpp:
+        (WebCore::RenderTextControl::text): Add a newline character for each <br>
+        element encountered in the control
+
+2008-12-13  Darin Adler  <darin@apple.com>
+
         - file deletion part of https://bugs.webkit.org/show_bug.cgi?id=17497
           eliminate DeprecatedValueList
 
index f0fca6c..def348a 100644 (file)
@@ -275,11 +275,17 @@ String HTMLTextAreaElement::value() const
 void HTMLTextAreaElement::setValue(const String& value)
 {
     // Code elsewhere normalizes line endings added by the user via the keyboard or pasting.
-    // We must normalize line endings coming from JS.
-    m_value = value;
-    m_value.replace("\r\n", "\n");
-    m_value.replace('\r', '\n');
+    // We normalize line endings coming from JavaScript here.
+    String normalizedValue = value.isNull() ? "" : value;
+    normalizedValue.replace("\r\n", "\n");
+    normalizedValue.replace('\r', '\n');
+
+    // Return early because we don't want to move the caret or trigger other side effects
+    // when the value isn't changing. This matches Firefox behavior, at least.
+    if (normalizedValue == this->value())
+        return;
 
+    m_value = normalizedValue;
     setValueMatchesRenderer();
     if (inDocument())
         document()->updateRendering();
index 6378d46..a6ed49f 100644 (file)
@@ -521,7 +521,9 @@ String RenderTextControl::text()
     Vector<UChar> result;
 
     for (Node* n = m_innerText.get(); n; n = n->traverseNextNode(m_innerText.get())) {
-        if (n->isTextNode()) {
+        if (n->hasTagName(brTag))
+            result.append(&newlineCharacter, 1);
+        else if (n->isTextNode()) {
             Text* text = static_cast<Text*>(n);
             String data = text->data();
             unsigned length = data.length();