New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 615307 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Charset autodetection doesn't work properly with preload header

Reported by nazar.mo...@gmail.com, May 27 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0

Example URL:
https://github.com/nazar-pc/preload-header-charset-bug

Steps to reproduce the problem:
1. Have static Js file with unicode characters served from server with Content-Type application/javascript
2. Have HTML page (utf-8) with preload header for mentioned JS file
3. Try to get content from that JS file (might be value of the variable or something)

What is the expected behavior?
Unicode characters shown correctly

What went wrong?
Unicode characters are not shown correctly, JS file is not recognized to be UTF-8 file

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? No 

Does this work in other browsers? Yes 

Chrome version: 53.0.2750.0  Channel: canary
OS Version: 
Flash Version: 

If some other file is preloaded with BOM everything starts working fine, also if file itself has BOM it works fine. Also problem doesn't happen with preload tag.
You can open GitHub repository to see reproducible demo.
 

Comment 1 by y...@yoav.ws, May 27 2016

Cc: mek@chromium.org
Owner: y...@yoav.ws
Status: Available (was: Unconfirmed)
My guess would be that header preloaded resources are not interpreted with their containing doc's charset.

A possible workaround could be to add a charset also to the preloaded resource's content type. 
Yes, specifying charset in Content-Type works fine, also BOM addition works as well (this was simpler workaround in my case).
But this is still a bug that should be fixed)
Components: -Blink Blink>TextEncoding Blink>Loader
Project Member

Comment 4 by bugdroid1@chromium.org, May 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/727d4183ad164bd4eaa4bde1c82eadb592e1c41c

commit 727d4183ad164bd4eaa4bde1c82eadb592e1c41c
Author: yoav <yoav@yoav.ws>
Date: Mon May 30 15:51:13 2016

Add Document encoding to header preloaded requests

When resources were preloaded using Link: headers, they weren't requested with
the document's charset, and therefore their decoding might have differed from non-preloaded resources.
This CL fixes that by requesting them with the right encoding.

BUG= 615307 

Review-Url: https://codereview.chromium.org/2019303002
Cr-Commit-Position: refs/heads/master@{#396735}

[add] https://crrev.com/727d4183ad164bd4eaa4bde1c82eadb592e1c41c/third_party/WebKit/LayoutTests/http/tests/preload/link_preload_charset.php
[add] https://crrev.com/727d4183ad164bd4eaa4bde1c82eadb592e1c41c/third_party/WebKit/LayoutTests/http/tests/preload/resources/charset.js
[modify] https://crrev.com/727d4183ad164bd4eaa4bde1c82eadb592e1c41c/third_party/WebKit/Source/core/loader/LinkLoader.cpp

Comment 5 by y...@yoav.ws, May 30 2016

Status: Fixed (was: Available)
Fixed AFAICT. Please verify and reopen if needed.
Works good now, thanks!

Quick question: will it be backported to stable or which target release will contain this fix? I'm not willing to keep workaround forever:)

Comment 7 by y...@yoav.ws, May 31 2016

Labels: -OS-Linux Merge-Request-52 OS-All
Currently this will go into Chrome 53. (stable at end of August)
I'll request a merge into 52. (current beta, stable at mid July)

Comment 8 by tin...@google.com, May 31 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Please have a the CL merged by EOD today(05/31), so it gets picked up for Beta Promotion scheduled on 06/02.

Please provide a sample testcase so that team could verify the fix in canary.
Project Member

Comment 10 by bugdroid1@chromium.org, May 31 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c732f3b13215ca042376f72e361d8a140ea81802

commit c732f3b13215ca042376f72e361d8a140ea81802
Author: Yoav Weiss <yoav@yoav.ws>
Date: Tue May 31 21:24:13 2016

Add Document encoding to header preloaded requests

When resources were preloaded using Link: headers, they weren't requested with
the document's charset, and therefore their decoding might have differed from non-preloaded resources.
This CL fixes that by requesting them with the right encoding.

BUG= 615307 

Review-Url: https://codereview.chromium.org/2019303002
Cr-Commit-Position: refs/heads/master@{#396735}
(cherry picked from commit 727d4183ad164bd4eaa4bde1c82eadb592e1c41c)

Review URL: https://codereview.chromium.org/2026673003 .

Cr-Commit-Position: refs/branch-heads/2743@{#150}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[add] https://crrev.com/c732f3b13215ca042376f72e361d8a140ea81802/third_party/WebKit/LayoutTests/http/tests/preload/link_preload_charset.php
[add] https://crrev.com/c732f3b13215ca042376f72e361d8a140ea81802/third_party/WebKit/LayoutTests/http/tests/preload/resources/charset.js
[modify] https://crrev.com/c732f3b13215ca042376f72e361d8a140ea81802/third_party/WebKit/Source/core/loader/LinkLoader.cpp

Comment 11 by y...@yoav.ws, May 31 2016

Merged. The test case I used is the one provided with this issue: https://github.com/nazar-pc/preload-header-charset-bug

The layout tests include a simplified version of it in http/tests/preload/link_preload_charset.html

Sign in to add a comment