[Webkit-unassigned] [Bug 24150] Add virtual ScriptExecutionContext::encoding()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=24150


ap at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27999|review?                     |review-
               Flag|                            |




------- Comment #6 from ap at webkit.org  2009-02-26 04:52 PDT -------
(From update of attachment 27999)
> +    String inputEncoding() const { return encoding(); }
> +    String charset() const { return encoding(); }
> +    String characterSet() const { return encoding(); }

You need to use "return Document::encoding();" syntax to avoid virtual
dispatch. Please also consider making encoding() private (that would improve
safety, but decrease clarity somewhat, as the functions won't be near each
other any more).

> +    // FIXME: nested workers need loading support. Consider adopting ThreadableLoader here.

Capital "N" is needed in "nested".

> -    // FIXME: does this need to provide a charset, like Document::completeURL does?
> -    return KURL(m_location->url(), url);
> +
> +    if (m_encoding.isNull())
> +        return KURL(m_location->url(), url);
> +
> +    return KURL(m_location->url(), url, TextEncoding(m_encoding));
>  }

Have you tested that this is what Firefox does? I wouldn't be surprised if it
always used UTF-8, and didn't inherit the encoding from the document at all.

This change needs an automated test.

> +        String m_encoding;

Why is it better to store it as String, not as TextEncoding?

> -        WorkerThread(const KURL&, const String& userAgent, const String& sourceCode, WorkerObjectProxy*);
> +        WorkerThread(const KURL&, const String& userAgent,  const String& encoding, const String& sourceCode, WorkerObjectProxy*);

Two spaces here.

r-, because this needs an automated test (XMLHttpRequest URL completion for
now, but it's worth testing how Firefox decodes nested worker scripts and
importScripts() content).


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.