WebCore:
authorantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Aug 2007 23:09:39 +0000 (23:09 +0000)
committerantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Aug 2007 23:09:39 +0000 (23:09 +0000)
        Reviewed by Maciej.

        Fix <rdar://problem/5433144>
        REGRESSION: Unable to click "Select" link at Expedia for car rentals

        javascript: URLs need special handling when serializing. Escaping them like
        normal attribute values can do bad things. Try hard to not escape anything,
        escape quote characters only if really necessary. Try to match Firefox.

        Test: fast/innerHTML/javascript-url.html

        * editing/markup.cpp:
        (WebCore::urlAttributeToQuotedString):
        (WebCore::startMarkup):

LayoutTests:

        Reviewed by Maciej.

        Test for <rdar://problem/5433144>
        REGRESSION: Unable to click "Select" link at Expedia for car rentals

        * fast/innerHTML/javascript-url-expected.txt: Added.
        * fast/innerHTML/javascript-url.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/innerHTML/javascript-url-expected.txt [new file with mode: 0644]
LayoutTests/fast/innerHTML/javascript-url.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/markup.cpp

index 009f24213601aae2a7dba8e7239f3bb90d5a8147..c8f10fdf00e796aea73c707fc20479e2c35019f9 100644 (file)
@@ -1,3 +1,13 @@
+2007-08-27  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Maciej.
+        
+        Test for <rdar://problem/5433144>
+        REGRESSION: Unable to click "Select" link at Expedia for car rentals
+
+        * fast/innerHTML/javascript-url-expected.txt: Added.
+        * fast/innerHTML/javascript-url.html: Added.
+
 2007-08-27  Oliver Hunt  <oliver@apple.com>
 
         rs=adam
diff --git a/LayoutTests/fast/innerHTML/javascript-url-expected.txt b/LayoutTests/fast/innerHTML/javascript-url-expected.txt
new file mode 100644 (file)
index 0000000..03769dc
--- /dev/null
@@ -0,0 +1,13 @@
+Test that innerHTML does not mangle javascript: urls.
+r.innerHTML = r.innerHTML.replace(/&37;3C!--D--&37;3E/g, 123)
+PASS: r.innerHTML.indexOf('javascript:test(123)') > -1 should be true and is.
+r.firstChild.setAttribute('href', 'javascript:test("text<")')
+PASS: r.innerHTML.indexOf('javascript:test("text<")') > -1 should be true and is.
+r.firstChild.setAttribute("href", "javascript:test('text>')")
+PASS: r.innerHTML.indexOf("javascript:test('text>')") > -1 should be true and is.
+testString = javascript:test('text&',"test2&")
+r.firstChild.setAttribute("href", testString)
+PASS: r.innerHTML.indexOf("javascript:test('text&',&quot;test2&&quot;)") > 1 should be true and is.
+r.firstChild.setAttribute('href', 'http://www.google.fi/search?q=scarlett johansson&meta=&btnG=Google-haku')
+<a href="http://www.google.fi/search?q=scarlett johansson&amp;meta=&amp;btnG=Google-haku">link</a>
+link
diff --git a/LayoutTests/fast/innerHTML/javascript-url.html b/LayoutTests/fast/innerHTML/javascript-url.html
new file mode 100644 (file)
index 0000000..b530917
--- /dev/null
@@ -0,0 +1,62 @@
+<head>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+    
+function print(message, color) 
+{
+    var paragraph = document.createElement("div");
+    paragraph.appendChild(document.createTextNode(message));
+    paragraph.style.fontFamily = "monospace";
+    if (color)
+        paragraph.style.color = color;
+    document.getElementById("console").appendChild(paragraph);
+}
+
+function run(a)
+{
+    print(a);
+    try {
+        eval(a);
+    } catch(e) {
+        print(e);
+    }
+}
+
+function shouldBe(a, b)
+{
+    var evalA;
+    try {
+        evalA = eval(a);
+    } catch(e) {
+        evalA = e;
+    }
+    
+    if (evalA == b)
+        print("PASS: " + a + " should be " + b + " and is.", "green");
+    else
+        print("FAIL: " + a + " should be " + b + " but instead is " + evalA + ".", "red");
+}
+</script>
+</head>
+<body>
+Test that innerHTML does not mangle javascript: urls.
+<div id=console></div>
+<div id=jsurltest><a href='
+ javascript:test(&37;3C!--D--&37;3E)'>link</a></div>
+<script>
+var r = document.getElementById('jsurltest');
+run("r.innerHTML = r.innerHTML.replace(/&37;3C!--D--&37;3E/g, 123)");
+shouldBe("r.innerHTML.indexOf('javascript:test(123)') > -1", true);
+run("r.firstChild.setAttribute('href', 'javascript:test(\"text<\")')");
+shouldBe("r.innerHTML.indexOf('javascript:test(\"text<\")') > -1", true);
+run('r.firstChild.setAttribute("href", "javascript:test(\'text>\')")');
+shouldBe('r.innerHTML.indexOf("javascript:test(\'text>\')") > -1', true);
+testString = 'javascript:test(\'text&\',"test2&")';
+print("testString = " + testString);
+run('r.firstChild.setAttribute("href", testString)');
+shouldBe('r.innerHTML.indexOf("javascript:test(\'text&\',&quot;test2&&quot;)") > 1', true);
+
+run("r.firstChild.setAttribute('href', 'http://www.google.fi/search?q=scarlett johansson&meta=&btnG=Google-haku')");
+print(r.innerHTML);
+</script>
index 0a7cbbb7af0462d83dcc4e6e878e0b18e5406484..0d6bf44360118ee3424e7eca1170029460ee9bd3 100644 (file)
@@ -1,3 +1,20 @@
+2007-08-27  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Maciej.
+        
+        Fix <rdar://problem/5433144>
+        REGRESSION: Unable to click "Select" link at Expedia for car rentals
+        
+        javascript: URLs need special handling when serializing. Escaping them like
+        normal attribute values can do bad things. Try hard to not escape anything,
+        escape quote characters only if really necessary. Try to match Firefox.
+
+        Test: fast/innerHTML/javascript-url.html
+
+        * editing/markup.cpp:
+        (WebCore::urlAttributeToQuotedString):
+        (WebCore::startMarkup):
+
 2007-08-27  David Hyatt  <hyatt@apple.com>
 
         Fix for 5441224, micro-optimizations to improve the PLT by 1%.
index 2572b492d73f934ff380a554bb181a5535dfc0f8..b1b9e23cb66b615118ada5b7120fd93e23ece3f3 100644 (file)
@@ -115,6 +115,28 @@ static DeprecatedString escapeTextForMarkup(const String& in, bool isAttributeVa
 
     return s;
 }
+    
+static String urlAttributeToQuotedString(String urlString)
+{
+    UChar quoteChar = '"';
+    if (urlString.stripWhiteSpace().startsWith("javascript:", false)) {
+        // minimal escaping for javascript urls
+        if (urlString.contains('"')) {
+            if (urlString.contains('\''))
+                urlString.replace('"', "&quot;");
+            else
+                quoteChar = '\'';
+        }
+    } else
+        // FIXME this does not fully match other browsers. Firefox escapes spaces and other special characters.
+        urlString = escapeTextForMarkup(urlString.deprecatedString(), true);
+
+    String res;
+    res.append(quoteChar);
+    res.append(urlString);
+    res.append(quoteChar);
+    return res;
+}
 
 static String stringValueForRange(const Node *node, const Range *range)
 {
@@ -272,7 +294,10 @@ static DeprecatedString startMarkup(const Node *node, const Range *range, EAnnot
                     markup += " " + attr->name().localName().deprecatedString();
                 else
                     markup += " " + attr->name().toString().deprecatedString();
-                markup += "=\"" + escapeTextForMarkup(attr->value(), true) + "\"";
+                if (el->isURLAttribute(attr))
+                    markup += "=" + urlAttributeToQuotedString(attr->value()).deprecatedString();
+                else
+                    markup += "=\"" + escapeTextForMarkup(attr->value(), true) + "\"";
                 if (!documentIsHTML && namespaces && shouldAddNamespaceAttr(attr, *namespaces))
                     markup += addNamespace(attr->prefix(), attr->namespaceURI(), *namespaces).deprecatedString();
             }