1109486 - XMLHttpRequest.send(document) should unconditionally encode as UTF-8

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Philip Jägenstedt [:foolip]]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2202.3 Safari/537.36 OPR/27.0.1689.2 (Edition developer)

Steps to reproduce:

Compare spec and implementation:
https://xhr.spec.whatwg.org/#dom-xmlhttprequest-send
https://github.com/mozilla/gecko-dev/blob/master/dom/base/nsXMLHttpRequest.cpp#L2399


Actual results:

The spec says to unconditionally use UTF-8 while Gecko uses doc->GetDocumentCharacterSet().


Expected results:

Always send as UTF-8. Blink does this, but doesn't include ";charset=UTF-8" in the Content-Type header yet.

Comment 1

[Philip Jägenstedt [:foolip]]

Some ad-hoc testing shows that IE appends "; charset=utf-8" which isn't exactly what the spec says, but perhaps close enough.

Comment 2

[Philip Jägenstedt [:foolip]]

I just added the ";charset=UTF-8" in Blink:
https://codereview.chromium.org/791783002/

It probably makes sense to make both changes together in Gecko.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I'm told this is tested by web-platform-tests/XMLHttpRequest/send-entity-body-document.htm.

Comment 4

[Jeremy Chen ([e-mail])]

Attachment: v1 - Unconditionally use UTF-8 as default Charset

I avoided using DocumentCharacterSet encoding, and unconditionally set encoding to `UTF-8` as the spec stated.

Comment 5

[Jeremy Chen ([e-mail])]

Attachment: v1 - mochitest test case patch

v1 - mochitest test case patch

I updated the test case to match the spec.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8537151 [details] [diff] [review]
v1 - Unconditionally use UTF-8 as default Charset

could you fix the comment below
"  // Serialize to a stream so that the encoding used will
  // match the document's."

I guess you could just remove it.


This is a bit risky change, but let's try.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

(There has been changes like bug 361934 long ago)

Comment 8

[Jeremy Chen ([e-mail])]

Attachment: v2 - Unconditionally use UTF-8 as default Charset (combined with mochitest test case patch)

Unnecessary code comments have been removed.

please see the try server result here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=54d05a5c12e1

Comment 9

[Jeremy Chen ([e-mail])]

Attachment: v2 - Unconditionally use UTF-8 as default Charset

Unnecessary code comments have been removed.

please see the try server result here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=54d05a5c12e1

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89659f3f1dcf
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbd205a100a9

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/89659f3f1dcf
https://hg.mozilla.org/mozilla-central/rev/dbd205a100a9

Comment 12

[Kohei Yoshino [:kohei]]

https://developer.mozilla.org/en-US/Firefox/Releases/37/Site_Compatibility