Attachment 19634 Details for Bug 17732 – patch v3, based on patch v2, move method scriptCharset from Document class to HTMLScriptElement class

[patch] patch v3, based on patch v2, move method scriptCharset from Document class to HTMLScriptElement class

patch.txt (text/plain), 8.42 KB, created by johnnyding on 2008-03-10 07:30:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: johnnyding

Created: 2008-03-10 07:30:24 PDT

Size: 8.42 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 30927)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2008-03-10  Johnny Ding  <johnnyding.webkit@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+        - fix http://bugs.webkit.org/show_bug.cgi?id=17732        
>+
>+        For an external javascript, when changing its src attribute, WebKit
>+        will request the script, decode and execute it. But if the script
>+        has not charset declaration in its script tag and encoding info in
>+        http response, WebKit will use "Latin1" as charset to decode it,
>+        which cuases the script can not be correctly decoded.
>+
>+        Test: fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html
>+
>+        * html/HTMLScriptElement.cpp:
>+        (WebCore::HTMLScriptElement::parseMappedAttribute): Use helper
>+        function to get proper charset for correct decoding script content.
>+        (WebCore::HTMLScriptElement::insertedIntoDocument):
>+        (WebCore::HTMLScriptElement::scriptCharset): A helper function for
>+        getting proper charset for script tag.
>+        * html/HTMLScriptElement.h:
>+        * html/HTMLTokenizer.cpp:
>+        (WebCore::HTMLTokenizer::parseTag): Use helper function to get
>+        proper charset for correct decoding script content.
>+
> 2008-03-10  Simon Hausmann  <hausmann@webkit.org>
> 
>         Reviewed by Lars.
>Index: WebCore/html/HTMLScriptElement.cpp
>===================================================================
>--- WebCore/html/HTMLScriptElement.cpp	(revision 30907)
>+++ WebCore/html/HTMLScriptElement.cpp	(working copy)
>@@ -82,7 +82,9 @@ void HTMLScriptElement::parseMappedAttri
>     
>         const AtomicString& url = attr->value();
>         if (!url.isEmpty()) {
>-            m_cachedScript = document()->docLoader()->requestScript(url, getAttribute(charsetAttr));
>+            // Try to get proper charset information for correct decoding content
>+            // of external script. See http://bugs.webkit.org/show_bug.cgi?id=17732
>+            m_cachedScript = document()->docLoader()->requestScript(url, scriptCharset());
>             if (m_cachedScript)
>                 m_cachedScript->ref(this);
>             else
>@@ -120,12 +122,7 @@ void HTMLScriptElement::insertedIntoDocu
>     
>     const AtomicString& url = getAttribute(srcAttr);
>     if (!url.isEmpty()) {
>-        String scriptSrcCharset = getAttribute(charsetAttr).string().stripWhiteSpace();
>-        if (scriptSrcCharset.isEmpty()) {
>-            if (Frame* frame = document()->frame())
>-                scriptSrcCharset = frame->loader()->encoding();
>-        }
>-        m_cachedScript = document()->docLoader()->requestScript(url, scriptSrcCharset);
>+        m_cachedScript = document()->docLoader()->requestScript(url, scriptCharset());
>         if (m_cachedScript)
>             m_cachedScript->ref(this);
>         else
>@@ -331,4 +328,17 @@ void HTMLScriptElement::setType(const St
>     setAttribute(typeAttr, value);
> }
> 
>+String HTMLScriptElement::scriptCharset() const
>+{
>+    // First we try to get encoding from charset attribute.
>+    String charset = getAttribute(charsetAttr).string().stripWhiteSpace();
>+    // If charset has not been declared in script tag, try to get
>+    // frameloader's encoding info.
>+    if (charset.isEmpty()) {
>+        if (Frame* frame = document()->frame())
>+            charset = frame->loader()->encoding();
>+    }
>+    return charset;
>+}
>+
> }
>Index: WebCore/html/HTMLScriptElement.h
>===================================================================
>--- WebCore/html/HTMLScriptElement.h	(revision 30907)
>+++ WebCore/html/HTMLScriptElement.h	(working copy)
>@@ -75,6 +75,9 @@ public:
>     String type() const;
>     void setType(const String&);
> 
>+    // Get proper charset info for script tag.
>+    String scriptCharset() const;
>+    
> private:
>     CachedScript* m_cachedScript;
>     bool m_createdByParser;
>Index: WebCore/html/HTMLTokenizer.cpp
>===================================================================
>--- WebCore/html/HTMLTokenizer.cpp	(revision 30907)
>+++ WebCore/html/HTMLTokenizer.cpp	(working copy)
>@@ -1448,10 +1448,6 @@ HTMLTokenizer::State HTMLTokenizer::pars
>                     if (m_doc->frame() && m_doc->frame()->scriptProxy()->isEnabled()) {
>                         if ((a = currToken.attrs->getAttributeItem(srcAttr)))
>                             scriptSrc = m_doc->completeURL(parseURL(a->value())).string();
>-                        if ((a = currToken.attrs->getAttributeItem(charsetAttr)))
>-                            scriptSrcCharset = a->value().string().stripWhiteSpace();
>-                        if (scriptSrcCharset.isEmpty())
>-                            scriptSrcCharset = m_doc->frame()->loader()->encoding();
>                     }
>                 }
>             }
>@@ -1465,6 +1461,8 @@ HTMLTokenizer::State HTMLTokenizer::pars
>                 } else if (tagName == scriptTag && n) {
>                     ASSERT(!scriptNode);
>                     scriptNode = n;
>+                    // Get proper charset for correct decoding script content.
>+                    scriptSrcCharset = static_cast<HTMLScriptElement*>(n.get())->scriptCharset();
>                     if (beginTag) {
>                         searchStopper = scriptEnd;
>                         searchStopperLen = 8;
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 30927)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2008-03-10  Johnny Ding  <johnnyding.webkit@gmail.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Bug http://bugs.webkit.org/show_bug.cgi?id=17732
>+        Test case for testing script decoding after setting its src attribute
>+
>+        * fast/dom/HTMLScriptElement/resources/script-decoding-error-after-setting-src.js: Added.
>+        * fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src-expected.txt: Added.
>+        * fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html: Added.
>+
> 2008-03-07  Simon Hausmann  <hausmann@webkit.org>
> 
>         Reviewed by Darin.
>Index: LayoutTests/fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src-expected.txt
>===================================================================
>--- LayoutTests/fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src-expected.txt	(revision 0)
>+++ LayoutTests/fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src-expected.txt	(revision 0)
>@@ -0,0 +1,2 @@
>+This case is for testing script decoding after setting its src attribute. See bug: http://bugs.webkit.org/show_bug.cgi?id=17732
>+SUCCESS
>Index: LayoutTests/fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html
>===================================================================
>--- LayoutTests/fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html	(revision 0)
>+++ LayoutTests/fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html	(revision 0)
>@@ -0,0 +1,22 @@
>+<html>
>+  <head>
>+    <meta http-equiv="Content-Type" content="text/html; charset=gb2312" />
>+  </head>
>+  <body onload="set_script_src()">
>+    This case is for testing script decoding after setting its src attribute. See bug: http://bugs.webkit.org/show_bug.cgi?id=17732<br>
>+    <script id="script_target"></script>
>+    <div id="content_panel">²âÊÔ</div>
>+    <script>
>+    function set_script_src() {
>+      if (window.layoutTestController) {
>+        layoutTestController.dumpAsText();
>+        layoutTestController.waitUntilDone();
>+      }
>+      var script_obj = document.getElementById("script_target");
>+      if (script_obj) {
>+        script_obj.src = "./resources/script-decoding-error-after-setting-src.js";
>+      }
>+    }
>+    </script>
>+  </body>
>+</html>
>Index: LayoutTests/fast/dom/HTMLScriptElement/resources/script-decoding-error-after-setting-src.js
>===================================================================
>--- LayoutTests/fast/dom/HTMLScriptElement/resources/script-decoding-error-after-setting-src.js	(revision 0)
>+++ LayoutTests/fast/dom/HTMLScriptElement/resources/script-decoding-error-after-setting-src.js	(revision 0)
>@@ -0,0 +1,10 @@
>+var panel_obj = document.getElementById("content_panel");
>+if (panel_obj) {
>+  var encoded_content = "²âÊÔ";
>+  if (encoded_content == panel_obj.innerHTML)
>+    panel_obj.innerHTML = "SUCCESS";
>+  else
>+    panel_obj.innerHTML = "FAILURE";
>+  if (window.layoutTestController)
>+    layoutTestController.notifyDone();
>+}

Flags:

ap: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 17732: 19618 | 19619 | 19620 | 19627 | 19630 | 19633 | 19634