Set encoding to UTF-8 for HTML Imports if no encoding is specified in the HTTP response

Force encoding to UTF-8 for HTML Imports

See spec bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=21275

R=abarth@chromium.org, morrita@chromium.org
BUG=336698
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171381

[abarth-chromium, 2014-02-24 22:25:29]

LGTM

This change seems a bit strange.  It makes it harder to know how something will
be parsed just looking at network traffic.  I guess the pro UTF-8 folks got
their way...

[adamk, 2014-02-24 22:26:56]

On 2014/02/24 22:25:29, abarth wrote:
> LGTM
> 
> This change seems a bit strange.  It makes it harder to know how something
will
> be parsed just looking at network traffic.  I guess the pro UTF-8 folks got
> their way...

You're welcome to comment on the spec bug at
https://www.w3.org/Bugs/Public/show_bug.cgi?id=21275 if you think this isn't the
right direction.

[abarth-chromium, 2014-02-24 22:29:52]

On 2014/02/24 22:26:56, adamk wrote:
> You're welcome to comment on the spec bug at
> https://www.w3.org/Bugs/Public/show_bug.cgi?id=21275 if you think this isn't
the
> right direction.

Hum...  I don't think this CL does what the spec says, at least not what's
linked from that bug.  I think the spec says to use UTF-8 all the time,
regardless of what the network says,.

[abarth-chromium, 2014-02-24 22:30:26]

https://codereview.chromium.org/178883002/diff/1/Source/core/html/imports/HTM...
File Source/core/html/imports/HTMLImportLoader.cpp (right):

https://codereview.chromium.org/178883002/diff/1/Source/core/html/imports/HTM...
Source/core/html/imports/HTMLImportLoader.cpp:101: m_writer =
DocumentWriter::create(m_importedDocument.get(), response.mimeType(),
textEncoding);
e.g., just pass "UTF-8" unconditionally here

[adamk, 2014-02-24 22:31:46]

On 2014/02/24 22:29:52, abarth wrote:
> On 2014/02/24 22:26:56, adamk wrote:
> > You're welcome to comment on the spec bug at
> > https://www.w3.org/Bugs/Public/show_bug.cgi?id=21275 if you think this isn't
> the
> > right direction.
> 
> Hum...  I don't think this CL does what the spec says, at least not what's
> linked from that bug.  I think the spec says to use UTF-8 all the time,
> regardless of what the network says,.

I re-read the spec again and I think you're right. Now I think it's crazy, so
I've commented on the spec bug myself.

FWIW, my confusion was due to an offline discussion with dglazkov where I got
the impression that the _intention_ was to obey Content-Type headers.

[adamk, 2014-03-06 01:11:18]

Per discussion on the spec bug, it sounds like the plan is to remove this "force
UTF8" behavior from the spec.

[Hajime Morrita, 2014-03-06 01:18:32]

On 2014/03/06 01:11:18, adamk wrote:
> Per discussion on the spec bug, it sounds like the plan is to remove this
"force
> UTF8" behavior from the spec.
Yeah, this is my current thinking.

[Hajime Morrita, 2014-04-10 23:07:52]

On 2014/03/06 01:18:32, morrita1 wrote:
> On 2014/03/06 01:11:18, adamk wrote:
> > Per discussion on the spec bug, it sounds like the plan is to remove this
> "force
> > UTF8" behavior from the spec.
> Yeah, this is my current thinking.

And pro-UTF8 opinion seems getting support. Thinking about just landing this.

[adamk, 2014-04-10 23:34:16]

On 2014/04/10 23:07:52, morrita1 wrote:
> On 2014/03/06 01:18:32, morrita1 wrote:
> > On 2014/03/06 01:11:18, adamk wrote:
> > > Per discussion on the spec bug, it sounds like the plan is to remove this
> > "force
> > > UTF8" behavior from the spec.
> > Yeah, this is my current thinking.
> 
> And pro-UTF8 opinion seems getting support. Thinking about just landing this.

Yeah, it does seem like most folks who've expressed an opinion prefer the
behavior embodied in this patch.

[Hajime Morrita, 2014-04-11 00:02:16]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-04-11 00:02:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/178883002/20001

[commit-bot: I haz the power, 2014-04-11 00:02:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-11 00:02:27]

Failed to apply patch for
LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
  Copied LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi ->
LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
  patching file LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
  Hunk #1 FAILED at 1.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi.rej

Patch:   NR 
LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi->LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
Index: LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
diff --git a/LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi
b/LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
similarity index 61%
copy from LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi
copy to LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
index
68635b78de3ee5042741908d30e7c775b4a27f75..26e855fc2d24e464f1b7d6f909d62ddbdf476cb2
100755
--- a/LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi
+++ b/LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
@@ -1,9 +1,6 @@
-#!/usr/bin/perl -wT
-use strict;
-
+#!/usr/bin/perl
 print "Content-Type: text/html\n";
 print "Access-Control-Allow-Credentials: true\n";
 print "Access-Control-Allow-Origin: http://127.0.0.1:8000\n\n";
 
-print "<html><body><h1>Hello, CORS!</h1></body></html>\n";
-
+print "<!DOCTYPE html><body>Hello, world</body>\n\n";

[Hajime Morrita, 2014-04-11 00:02:52]

On 2014/04/10 23:34:16, adamk wrote:
> On 2014/04/10 23:07:52, morrita1 wrote:
> > On 2014/03/06 01:18:32, morrita1 wrote:
> > > On 2014/03/06 01:11:18, adamk wrote:
> > > > Per discussion on the spec bug, it sounds like the plan is to remove
this
> > > "force
> > > > UTF8" behavior from the spec.
> > > Yeah, this is my current thinking.
> > 
> > And pro-UTF8 opinion seems getting support. Thinking about just landing
this.
> 
> Yeah, it does seem like most folks who've expressed an opinion prefer the
> behavior embodied in this patch.

OK, let's try cq.

[Hajime Morrita, 2014-04-11 00:06:32]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-04-11 00:06:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/178883002/20001

[commit-bot: I haz the power, 2014-04-11 00:06:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-11 00:06:50]

Failed to apply patch for
LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
  Copied LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi ->
LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
  patching file LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
  Hunk #1 FAILED at 1.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi.rej

Patch:   NR 
LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi->LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
Index: LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
diff --git a/LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi
b/LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
similarity index 61%
copy from LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi
copy to LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
index
68635b78de3ee5042741908d30e7c775b4a27f75..26e855fc2d24e464f1b7d6f909d62ddbdf476cb2
100755
--- a/LayoutTests/http/tests/htmlimports/resources/cors-basic.cgi
+++ b/LayoutTests/http/tests/htmlimports/resources/no-encoding.cgi
@@ -1,9 +1,6 @@
-#!/usr/bin/perl -wT
-use strict;
-
+#!/usr/bin/perl
 print "Content-Type: text/html\n";
 print "Access-Control-Allow-Credentials: true\n";
 print "Access-Control-Allow-Origin: http://127.0.0.1:8000\n\n";
 
-print "<html><body><h1>Hello, CORS!</h1></body></html>\n";
-
+print "<!DOCTYPE html><body>Hello, world</body>\n\n";

[adamk, 2014-04-11 00:11:58]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2014-04-11 00:12:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/178883002/40001

[commit-bot: I haz the power, 2014-04-11 06:28:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-11 06:28:51]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[Hajime Morrita, 2014-04-11 17:12:36]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-04-11 17:12:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/178883002/40001

[commit-bot: I haz the power, 2014-04-11 17:51:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-11 17:51:22]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[Hajime Morrita, 2014-04-11 23:41:40]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-04-11 23:41:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/178883002/40001

[commit-bot: I haz the power, 2014-04-12 00:10:04]

Change committed as 171381