Title: [88110] trunk
Revision
88110
Author
a...@apple.com
Date
2011-06-04 04:23:09 -0700 (Sat, 04 Jun 2011)

Log Message

2011-06-04  Alexey Proskuryakov  <a...@apple.com>

        Reviewed by Darin Adler.

        Input value sanitization for text fields is incorrect
        https://bugs.webkit.org/show_bug.cgi?id=62061
        <rdar://problem/9553273>

        * fast/forms/input-value-sanitization-expected.txt:
        * fast/forms/input-value-sanitization.html:
        * fast/forms/paste-multiline-text-input.html:
        * fast/forms/script-tests/input-value-sanitization.js: Removed.
2011-06-04  Alexey Proskuryakov  <a...@apple.com>

        Reviewed by Darin Adler.

        Input value sanitization for text fields is incorrect
        https://bugs.webkit.org/show_bug.cgi?id=62061
        <rdar://problem/9553273>

        Newline characters should be removed according to HTML5, not replaced with spaces.
        This also matches Safari 5 behavior.

        * html/TextFieldInputType.cpp:
        (WebCore::isASCIILineBreak): A functor for removeCharacters().
        (WebCore::limitLength): Do one thing at once.
        (WebCore::TextFieldInputType::sanitizeValue): Sanitization removes newlines.
        (WebCore::TextFieldInputType::handleBeforeTextInsertedEvent): Moved (somewhat surprising)
        code that replaces newlines with spaces here.

Modified Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (88109 => 88110)


--- trunk/LayoutTests/ChangeLog	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/LayoutTests/ChangeLog	2011-06-04 11:23:09 UTC (rev 88110)
@@ -1,3 +1,16 @@
+2011-06-04  Alexey Proskuryakov  <a...@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Input value sanitization for text fields is incorrect
+        https://bugs.webkit.org/show_bug.cgi?id=62061
+        <rdar://problem/9553273>
+
+        * fast/forms/input-value-sanitization-expected.txt:
+        * fast/forms/input-value-sanitization.html:
+        * fast/forms/paste-multiline-text-input.html:
+        * fast/forms/script-tests/input-value-sanitization.js: Removed.
+
 2011-06-04  Jeffrey Pfau  <jp...@apple.com>
 
         Reviewed by Beth Dakin.

Modified: trunk/LayoutTests/fast/forms/input-value-sanitization-expected.txt (88109 => 88110)


--- trunk/LayoutTests/fast/forms/input-value-sanitization-expected.txt	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/LayoutTests/fast/forms/input-value-sanitization-expected.txt	2011-06-04 11:23:09 UTC (rev 88110)
@@ -1,9 +1,6 @@
 Tests for value sanitization algorithm.
 
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
-
-
 Number:
 PASS input.value is "65536"
 PASS input.value = "256"; input.value is "256"
@@ -13,8 +10,8 @@
 PASS input.value is "50"
 
 Text:
-PASS input.value is "   foo bar   "
-PASS document.getSelection().toString() is "   foo bar   "
+PASS input.value is " foo bar "
+PASS document.getSelection().toString() is " foo bar "
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/forms/input-value-sanitization.html (88109 => 88110)


--- trunk/LayoutTests/fast/forms/input-value-sanitization.html	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/LayoutTests/fast/forms/input-value-sanitization.html	2011-06-04 11:23:09 UTC (rev 88110)
@@ -5,9 +5,47 @@
 <script src=""
 </head>
 <body>
-<p id="description"></p>
+<p>Tests for value sanitization algorithm.</p>
 <div id="console"></div>
-<script src=""
+<script>
+var input;
+
+debug('');
+debug('Number:');
+input = document.createElement('input');
+input.setAttribute('value', '65536');
+input.type = 'number';
+shouldBe('input.value', '"65536"');
+shouldBe('input.value = "256"; input.value', '"256"');
+shouldBe('input.value = ""; input.value', '""');
+
+
+debug('');
+debug('Range:');
+input = document.createElement('input');
+input.type = 'text';
+input.value = ':)';
+input.type = 'range';
+shouldBe('input.value', '"50"');
+
+debug('');
+debug('Text:');
+var container = document.createElement('div');
+document.body.appendChild(container);
+container.innerHTML = '<input type="text" id="text" value="\n\r foo bar \n\r\n">';
+input = document.getElementById('text');
+shouldBe('input.value', '" foo bar "');
+input.focus();
+document.execCommand('SelectAll');
+shouldBe('document.getSelection().toString()', '" foo bar "');
+
+// FIXME: Add more sanitization tests.
+// https://bugs.webkit.org/show_bug.cgi?id=37024
+
+container.innerHTML = '';
+var successfullyParsed = true;
+
+</script>
 <script src=""
 </body>
 </html>

Modified: trunk/LayoutTests/fast/forms/paste-multiline-text-input.html (88109 => 88110)


--- trunk/LayoutTests/fast/forms/paste-multiline-text-input.html	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/LayoutTests/fast/forms/paste-multiline-text-input.html	2011-06-04 11:23:09 UTC (rev 88110)
@@ -10,8 +10,10 @@
         }
 
         var DEFAULT_LINE_1 = "line\t(1 of 2)\r\nline\t(2 of 2)";
-        var EXPECTED_LINE_1 = "line\t(1 of 2) line\t(2 of 2)";
+        var EXPECTED_LINE_1 = "line\t(1 of 2)line\t(2 of 2)";
 
+        // FIXME: Is this really expected behavior to truncate the string at a null byte?
+        // It doesn't match Firefox 4 and common sense.
         var DEFAULT_LINE_2 = "null\0char";
         var EXPECTED_LINE_2 = "null";
 

Deleted: trunk/LayoutTests/fast/forms/script-tests/input-value-sanitization.js (88109 => 88110)


--- trunk/LayoutTests/fast/forms/script-tests/input-value-sanitization.js	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/LayoutTests/fast/forms/script-tests/input-value-sanitization.js	2011-06-04 11:23:09 UTC (rev 88110)
@@ -1,38 +0,0 @@
-description('Tests for value sanitization algorithm.');
-
-var input;
-
-debug('');
-debug('Number:');
-input = document.createElement('input');
-input.setAttribute('value', '65536');
-input.type = 'number';
-shouldBe('input.value', '"65536"');
-shouldBe('input.value = "256"; input.value', '"256"');
-shouldBe('input.value = ""; input.value', '""');
-
-
-debug('');
-debug('Range:');
-input = document.createElement('input');
-input.type = 'text';
-input.value = ':)';
-input.type = 'range';
-shouldBe('input.value', '"50"');
-
-debug('');
-debug('Text:');
-var container = document.createElement('div');
-document.body.appendChild(container);
-container.innerHTML = '<input type="text" id="text" value="\n\r foo bar \n\r\n">';
-input = document.getElementById('text');
-shouldBe('input.value', '"   foo bar   "');
-input.focus();
-document.execCommand('SelectAll');
-shouldBe('document.getSelection().toString()', '"   foo bar   "');
-
-// FIXME: Add more sanitization tests.
-// https://bugs.webkit.org/show_bug.cgi?id=37024
-
-container.innerHTML = '';
-var successfullyParsed = true;

Modified: trunk/Source/WebCore/ChangeLog (88109 => 88110)


--- trunk/Source/WebCore/ChangeLog	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/Source/WebCore/ChangeLog	2011-06-04 11:23:09 UTC (rev 88110)
@@ -1,3 +1,21 @@
+2011-06-04  Alexey Proskuryakov  <a...@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Input value sanitization for text fields is incorrect
+        https://bugs.webkit.org/show_bug.cgi?id=62061
+        <rdar://problem/9553273>
+
+        Newline characters should be removed according to HTML5, not replaced with spaces.
+        This also matches Safari 5 behavior.
+
+        * html/TextFieldInputType.cpp:
+        (WebCore::isASCIILineBreak): A functor for removeCharacters().
+        (WebCore::limitLength): Do one thing at once.
+        (WebCore::TextFieldInputType::sanitizeValue): Sanitization removes newlines.
+        (WebCore::TextFieldInputType::handleBeforeTextInsertedEvent): Moved (somewhat surprising)
+        code that replaces newlines with spaces here.
+
 2011-06-04  Jeffrey Pfau  <jp...@apple.com>
 
         Reviewed by Beth Dakin.

Modified: trunk/Source/WebCore/html/TextFieldInputType.cpp (88109 => 88110)


--- trunk/Source/WebCore/html/TextFieldInputType.cpp	2011-06-04 11:14:49 UTC (rev 88109)
+++ trunk/Source/WebCore/html/TextFieldInputType.cpp	2011-06-04 11:23:09 UTC (rev 88110)
@@ -208,13 +208,13 @@
     return true;
 }
 
-static String replaceEOLAndLimitLength(const String& proposedValue, int maxLength)
+static bool isASCIILineBreak(UChar c)
 {
-    String string = proposedValue;
-    string.replace("\r\n", " ");
-    string.replace('\r', ' ');
-    string.replace('\n', ' ');
+    return c == '\r' || c == '\n';
+}
 
+static String limitLength(const String& string, int maxLength)
+{
     unsigned newLength = numCharactersInGraphemeClusters(string, maxLength);
     for (unsigned i = 0; i < newLength; ++i) {
         const UChar current = string[i];
@@ -235,7 +235,7 @@
         return String();
     }
 #endif
-    return replaceEOLAndLimitLength(proposedValue, HTMLInputElement::maximumLength);
+    return limitLength(proposedValue.removeCharacters(isASCIILineBreak), HTMLInputElement::maximumLength);
 }
 
 void TextFieldInputType::handleBeforeTextInsertedEvent(BeforeTextInsertedEvent* event)
@@ -273,7 +273,13 @@
         return;
     }
 #endif
-    event->setText(replaceEOLAndLimitLength(event->text(), appendableLength));
+
+    String eventText = event->text();
+    eventText.replace("\r\n", " ");
+    eventText.replace('\r', ' ');
+    eventText.replace('\n', ' ');
+
+    event->setText(limitLength(eventText, appendableLength));
 }
 
 bool TextFieldInputType::shouldRespectListAttribute()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to