484305 - Workers: loaded scripts may be decoded using wrong text encoding.

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Dmitry Titov]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_6; en-us) AppleWebKit/525.27.1 (KHTML, like Gecko) Version/3.2.1 Safari/525.27.1
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090310 Minefield/3.2a1pre

Discussion in whatwg mail list clarifies that content of the scripts loaded as a result of 'new Worker(url)' or 'importScripts(url)' should be decoded using UTF-8 or the encoding specified in Content-Type http header, if present.
FF decodes the scripts using the encoding of the parent document. This is correct for <script> tag, but not for Workers.

Reproducible: Always

Comment 1

[Dmitry Titov]

Attachment: Worker file for the test.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Isn't this a duplicate of bug 482388?

Comment 3

[Dmitry Titov]

Attachment: repro.html - the test file (needs the worker file above)

Attached test file - download both repro.html and repro-worker.js and open repro.html.
The test itself is encoded using KOI8-r encoding (Russian). It has a test string to verify the encoding. The test spawns a worker from the file which contains the same test string encoded in both KOI8-R and UTF-8. The UTF-8 one should match because the script should be decoded using UTF-8 encoding by default.
Actual result: the KOI8-R matches because the scritp is decoded using the encoding of the parent document.

Comment 4

[Dmitry Titov]

(In reply to comment #2)
> Isn't this a duplicate of bug 482388?

No, because that one is about which encoding to use for the URL query portion that gets sent to the server.
This one is about which encoding to use while decoding the received script.
Sounds similar, but it might be the different places in code are affected.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Worker scripts should always be decoded as UTF-8.

Passes bug test case.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 814609 [details] [diff] [review]
Worker scripts should always be decoded as UTF-8.

Review of attachment 814609 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great. Can we please have a test before this gets checked in?

::: dom/workers/ScriptLoader.cpp
@@ +439,5 @@
>      nsIDocument* parentDoc = mWorkerPrivate->GetDocument();
>  
>      // Use the regular nsScriptLoader for this grunt work! Should be just fine
>      // because we're running on the main thread.
> +    // NOTE: Unlike <script> tags, Worker scripts are always decoded as UTF-8,

Nit: Adding 'NOTE:' to a comment is a little redundant. You're noting it already ;)

Comment 7

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Worker scripts should always be decoded as UTF-8.

Updated to add test case based on original failure example.

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Worker scripts should always be decoded as UTF-8.

Fixed test document.inputEncoding check.

Comment 9

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 815132 [details] [diff] [review]
Worker scripts should always be decoded as UTF-8.

Waiting for try-server results before checkin.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5436d332c969

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5436d332c969