2008-08-31 Cameron Zwarich <cwzwarich@uwaterloo.ca>
authorcwzwarich@webkit.org <cwzwarich@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 31 Aug 2008 21:57:32 +0000 (21:57 +0000)
committercwzwarich@webkit.org <cwzwarich@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 31 Aug 2008 21:57:32 +0000 (21:57 +0000)
        Reviewed by Maciej Stachowiak.

        Bug 20577: REGRESSION (r35006): Gmail is broken
        <https://bugs.webkit.org/show_bug.cgi?id=20577>

        r35006 changed stringProtoFuncSubstr() so that it is uses the more
        efficient jsSubstring(), rather than using UString::substr() and then
        calling jsString(). However, the change did not account for the case
        where the start and the length of the substring extend beyond the length
        of the original string. This patch corrects that.

        JavaScriptCore:

        * kjs/StringPrototype.cpp:
        (KJS::stringProtoFuncSubstr):

        LayoutTests:

        * fast/js/resources/string-substr.js: Added.
        * fast/js/string-substr-expected.txt: Added.
        * fast/js/string-substr.html: Added.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/StringPrototype.cpp
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/string-substr.js [new file with mode: 0644]
LayoutTests/fast/js/string-substr-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/string-substr.html [new file with mode: 0644]

index 6247968..79f8de3 100644 (file)
@@ -1,3 +1,19 @@
+2008-08-31  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Maciej Stachowiak.
+
+        Bug 20577: REGRESSION (r35006): Gmail is broken
+        <https://bugs.webkit.org/show_bug.cgi?id=20577>
+
+        r35006 changed stringProtoFuncSubstr() so that it is uses the more
+        efficient jsSubstring(), rather than using UString::substr() and then
+        calling jsString(). However, the change did not account for the case
+        where the start and the length of the substring extend beyond the length
+        of the original string. This patch corrects that.
+
+        * kjs/StringPrototype.cpp:
+        (KJS::stringProtoFuncSubstr):
+
 2008-08-31  Simon Hausmann  <hausmann@wekit.org>
 
         Unreviewed build fix (with gcc 4.3)
index be506d6..6718115 100644 (file)
@@ -564,17 +564,15 @@ JSValue* stringProtoFuncSubstr(ExecState* exec, JSObject*, JSValue* thisValue, c
 
     double start = a0->toInteger(exec);
     double length = a1->isUndefined() ? len : a1->toInteger(exec);
-    if (start >= len)
-        return jsEmptyString(exec);
-    if (length < 0)
+    if (start >= len || length <= 0)
         return jsEmptyString(exec);
     if (start < 0) {
         start += len;
         if (start < 0)
             start = 0;
     }
-    if (length > len)
-        length = len;
+    if (start + length > len)
+        length = len - start;
     return jsSubstring(exec, s, static_cast<unsigned>(start), static_cast<unsigned>(length));
 }
 
index 1762afa..d4bce3c 100644 (file)
@@ -1,3 +1,14 @@
+2008-08-31  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Maciej Stachowiak.
+
+        Tests for bug 20577: REGRESSION (r35006): Gmail is broken
+        <https://bugs.webkit.org/show_bug.cgi?id=20577>
+
+        * fast/js/resources/string-substr.js: Added.
+        * fast/js/string-substr-expected.txt: Added.
+        * fast/js/string-substr.html: Added.
+
 2008-08-30  Darin Adler  <darin@apple.com>
 
         Reviewed by Maciej.
diff --git a/LayoutTests/fast/js/resources/string-substr.js b/LayoutTests/fast/js/resources/string-substr.js
new file mode 100644 (file)
index 0000000..23b4661
--- /dev/null
@@ -0,0 +1,45 @@
+description(
+"This test checks the boundary cases of substr()."
+);
+
+shouldBe("'bar'.substr(0)", "'bar'");
+shouldBe("'bar'.substr(3)", "''");
+shouldBe("'bar'.substr(4)", "''");
+shouldBe("'bar'.substr(-1)", "'r'");
+shouldBe("'bar'.substr(-3)", "'bar'");
+shouldBe("'bar'.substr(-4)", "'bar'");
+
+shouldBe("'bar'.substr(0, 0)", "''");
+shouldBe("'bar'.substr(0, 1)", "'b'");
+shouldBe("'bar'.substr(0, 3)", "'bar'");
+shouldBe("'bar'.substr(0, 4)", "'bar'");
+
+shouldBe("'bar'.substr(1, 0)", "''");
+shouldBe("'bar'.substr(1, 1)", "'a'");
+shouldBe("'bar'.substr(1, 2)", "'ar'");
+shouldBe("'bar'.substr(1, 3)", "'ar'");
+
+shouldBe("'bar'.substr(3, 0)", "''");
+shouldBe("'bar'.substr(3, 1)", "''");
+shouldBe("'bar'.substr(3, 3)", "''");
+
+shouldBe("'bar'.substr(4, 0)", "''");
+shouldBe("'bar'.substr(4, 1)", "''");
+shouldBe("'bar'.substr(4, 3)", "''");
+
+shouldBe("'bar'.substr(-1, 0)", "''");
+shouldBe("'bar'.substr(-1, 1)", "'r'");
+
+shouldBe("'bar'.substr(-3, 1)", "'b'");
+shouldBe("'bar'.substr(-3, 3)", "'bar'");
+shouldBe("'bar'.substr(-3, 4)", "'bar'");
+
+shouldBe("'bar'.substr(-4)", "'bar'");
+shouldBe("'bar'.substr(-4, 0)", "''");
+shouldBe("'bar'.substr(-4, 1)", "'b'");
+shouldBe("'bar'.substr(-4, 3)", "'bar'");
+shouldBe("'bar'.substr(-4, 4)", "'bar'");
+
+shouldBe("'GMAIL_IMP=bf-i%2Fd-0-0%2Ftl-v'.substr(10)", "'bf-i%2Fd-0-0%2Ftl-v'");
+
+var successfullyParsed = true;
diff --git a/LayoutTests/fast/js/string-substr-expected.txt b/LayoutTests/fast/js/string-substr-expected.txt
new file mode 100644 (file)
index 0000000..c8989a4
--- /dev/null
@@ -0,0 +1,40 @@
+This test checks the boundary cases of substr().
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS 'bar'.substr(0) is 'bar'
+PASS 'bar'.substr(3) is ''
+PASS 'bar'.substr(4) is ''
+PASS 'bar'.substr(-1) is 'r'
+PASS 'bar'.substr(-3) is 'bar'
+PASS 'bar'.substr(-4) is 'bar'
+PASS 'bar'.substr(0, 0) is ''
+PASS 'bar'.substr(0, 1) is 'b'
+PASS 'bar'.substr(0, 3) is 'bar'
+PASS 'bar'.substr(0, 4) is 'bar'
+PASS 'bar'.substr(1, 0) is ''
+PASS 'bar'.substr(1, 1) is 'a'
+PASS 'bar'.substr(1, 2) is 'ar'
+PASS 'bar'.substr(1, 3) is 'ar'
+PASS 'bar'.substr(3, 0) is ''
+PASS 'bar'.substr(3, 1) is ''
+PASS 'bar'.substr(3, 3) is ''
+PASS 'bar'.substr(4, 0) is ''
+PASS 'bar'.substr(4, 1) is ''
+PASS 'bar'.substr(4, 3) is ''
+PASS 'bar'.substr(-1, 0) is ''
+PASS 'bar'.substr(-1, 1) is 'r'
+PASS 'bar'.substr(-3, 1) is 'b'
+PASS 'bar'.substr(-3, 3) is 'bar'
+PASS 'bar'.substr(-3, 4) is 'bar'
+PASS 'bar'.substr(-4) is 'bar'
+PASS 'bar'.substr(-4, 0) is ''
+PASS 'bar'.substr(-4, 1) is 'b'
+PASS 'bar'.substr(-4, 3) is 'bar'
+PASS 'bar'.substr(-4, 4) is 'bar'
+PASS 'GMAIL_IMP=bf-i%2Fd-0-0%2Ftl-v'.substr(10) is 'bf-i%2Fd-0-0%2Ftl-v'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/string-substr.html b/LayoutTests/fast/js/string-substr.html
new file mode 100644 (file)
index 0000000..ce91329
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/string-substr.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>