2011-01-12 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Jan 2011 04:51:16 +0000 (04:51 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Jan 2011 04:51:16 +0000 (04:51 +0000)
        Reviewed by Eric Seidel.

        REGRESSION(r69831): focus() in onkeypress discards input (affects chaseonline.chase.com)
        https://bugs.webkit.org/show_bug.cgi?id=52241

        The bug was caused by RenderTextControl::selection's creating a Range with m_insertText
        which is a shadow DOM div as both start and end containers. Fixed the bug by traversing
        through the descendents of m_innerText and using the right start and end containers to
        create a Range.

        Test: fast/forms/focus-change-on-keypress.html

        * rendering/RenderTextControl.cpp:
        (WebCore::setContainerAndOffsetForRange): Added; a helper function.
        (WebCore::RenderTextControl::selection): See above.
2011-01-12  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Eric Seidel.

        REGRESSION(r69831): focus() in onkeypress discards input (affects chaseonline.chase.com)
        https://bugs.webkit.org/show_bug.cgi?id=52241

        Added a test to ensure WebKit inserts the character typed by a user when focus's node
        is changed by a presskey event handler.

        * fast/forms/focus-change-on-keypress-expected.txt: Added.
        * fast/forms/focus-change-on-keypress.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/focus-change-on-keypress-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/focus-change-on-keypress.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTextControl.cpp

index b906ac6..cbda0b8 100644 (file)
@@ -1,3 +1,16 @@
+2011-01-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        REGRESSION(r69831): focus() in onkeypress discards input (affects chaseonline.chase.com)
+        https://bugs.webkit.org/show_bug.cgi?id=52241
+
+        Added a test to ensure WebKit inserts the character typed by a user when focus's node
+        is changed by a presskey event handler.
+
+        * fast/forms/focus-change-on-keypress-expected.txt: Added.
+        * fast/forms/focus-change-on-keypress.html: Added.
+
 2011-01-12  Kenichi Ishibashi  <bashi@google.com>
 
         Reviewed by Kent Tamura.
diff --git a/LayoutTests/fast/forms/focus-change-on-keypress-expected.txt b/LayoutTests/fast/forms/focus-change-on-keypress-expected.txt
new file mode 100644 (file)
index 0000000..af5410c
--- /dev/null
@@ -0,0 +1,16 @@
+This tests changing the focus on keypress event. The character typed by user should be inserted into input. This test only runs in DRT.
+
+input without selectionchange
+PASS
+
+input with selectionchange
+PASS
+
+textarea without selectionchange
+PASS
+
+textarea with selectionchange
+PASS
+
+DONE
+
diff --git a/LayoutTests/fast/forms/focus-change-on-keypress.html b/LayoutTests/fast/forms/focus-change-on-keypress.html
new file mode 100644 (file)
index 0000000..cfcd33e
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests changing the focus on keypress event. The character typed by user should be inserted into input. This test only runs in DRT.</p>
+<pre><script>
+
+function runTest(name, shouldChangeSelection) {
+    var dummy = document.createElement('input');
+    document.body.appendChild(dummy);
+
+    var element = document.createElement(name);
+    document.body.appendChild(element);
+    element.addEventListener('keypress', function() {
+        if (element.value.length >= 2)
+            dummy.focus();
+    })
+
+    document.writeln(name + (shouldChangeSelection ? ' with selectionchange' : ' without selectionchange'));
+
+    element.focus();
+    eventSender.keyDown('a');
+    eventSender.leapForward(100);
+    eventSender.keyDown(name == 'textarea' ? '\n' : 'b');
+    eventSender.leapForward(100);
+    if (shouldChangeSelection) {
+        element.selectionStart = 1;
+        element.selectionEnd = 1;
+    }
+    eventSender.keyDown('c');
+    eventSender.leapForward(100);
+
+    var expected = null;
+    if (name == 'textarea')
+        expected = shouldChangeSelection ? 'ac\n' : 'a\nc';
+    else
+        expected = shouldChangeSelection ? 'acb' : 'abc';
+
+    if (element.value == expected)
+        document.writeln('PASS');
+    else
+        document.writeln('FAIL: value was "' + element.value + '" but expected "' + expected + '"');
+    document.writeln();
+}
+
+if (window.layoutTestController && window.eventSender) {
+    layoutTestController.dumpAsText();
+
+    runTest('input', false);
+    runTest('input', true);
+    runTest('textarea', false);
+    runTest('textarea', true);
+
+    document.writeln('DONE');
+}
+
+</script></pre>
+</body>
+</html>
index 9abfd5c..c915ac7 100644 (file)
@@ -1,3 +1,21 @@
+2011-01-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        REGRESSION(r69831): focus() in onkeypress discards input (affects chaseonline.chase.com)
+        https://bugs.webkit.org/show_bug.cgi?id=52241
+
+        The bug was caused by RenderTextControl::selection's creating a Range with m_insertText
+        which is a shadow DOM div as both start and end containers. Fixed the bug by traversing
+        through the descendents of m_innerText and using the right start and end containers to
+        create a Range.
+
+        Test: fast/forms/focus-change-on-keypress.html
+
+        * rendering/RenderTextControl.cpp:
+        (WebCore::setContainerAndOffsetForRange): Added; a helper function.
+        (WebCore::RenderTextControl::selection): See above.
+
 2011-01-12  Kenichi Ishibashi  <bashi@google.com>
 
         Reviewed by Kent Tamura.
index b2db9e8..40aa4c2 100644 (file)
@@ -33,6 +33,7 @@
 #include "HTMLInputElement.h"
 #include "HTMLNames.h"
 #include "HitTestResult.h"
+#include "Position.h"
 #include "RenderLayer.h"
 #include "RenderText.h"
 #include "ScrollbarTheme.h"
@@ -271,12 +272,49 @@ bool RenderTextControl::isSelectableElement(Node* node) const
         || (shadowAncestor->hasTagName(inputTag) && static_cast<HTMLInputElement*>(shadowAncestor)->isTextField()));
 }
 
+static inline void setContainerAndOffsetForRange(Node* node, int offset, Node*& containerNode, int& offsetInContainer)
+{
+    if (node->isTextNode()) {
+        containerNode = node;
+        offsetInContainer = offset;
+    } else {
+        containerNode = node->parentNode();
+        offsetInContainer = node->nodeIndex() + offset;
+    }
+}
+
 PassRefPtr<Range> RenderTextControl::selection(int start, int end) const
 {
+    ASSERT(start <= end);
     if (!m_innerText)
         return 0;
 
-    return Range::create(document(), m_innerText, start, m_innerText, end);
+    if (!m_innerText->firstChild())
+        return Range::create(document(), m_innerText, 0, m_innerText, 0);
+
+    int offset = 0;
+    Node* startNode = 0;
+    Node* endNode = 0;
+    for (Node* node = m_innerText->firstChild(); node; node = node->traverseNextNode(m_innerText.get())) {
+        ASSERT(!node->firstChild());
+        ASSERT(node->isTextNode() || node->hasTagName(brTag));
+        int length = node->isTextNode() ? lastOffsetInNode(node) : 1;
+    
+        if (offset <= start && start <= offset + length)
+            setContainerAndOffsetForRange(node, start - offset, startNode, start);
+
+        if (offset <= end && end <= offset + length) {
+            setContainerAndOffsetForRange(node, end - offset, endNode, end);
+            break;
+        }
+
+        offset += length;
+    }
+    
+    if (!startNode || !endNode)
+        return 0;
+
+    return Range::create(document(), startNode, start, endNode, end);
 }
 
 VisiblePosition RenderTextControl::visiblePositionForIndex(int index) const