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

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 896242



Sign in to add a comment
link

Issue 651750: Chrome-only failures in web-platform-tests for XHR

Reported by foolip@chromium.org, Sep 30 2016 Project Member

Issue description

Comment 1 by dk...@chromium.org, Dec 6 2016

Cc: tyoshino@chromium.org
Ping! Can someone from the network team take a look at this?

Comment 2 by sshruthi@chromium.org, Jan 9 2017

Cc: hirosh...@chromium.org yhirano@chromium.org
Re-ping. Can one of you help triage this issue?

Comment 4 by ricea@chromium.org, Jan 20 2017

Status: Available (was: Untriaged)

Comment 5 by tyoshino@chromium.org, May 12 2017

Cc: -hirosh...@chromium.org

Comment 6 by bugdroid1@chromium.org, Nov 21 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75e4a3195fe136ef2965fc0aafbdaebc7321f88a

commit 75e4a3195fe136ef2965fc0aafbdaebc7321f88a
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Tue Nov 21 10:10:43 2017

[XHR] Do not replace request content-type "utf-8" charset

The spec asks us to replace the charset to "UTF-8" only when it is not
equal to "UTF-8" case insensitively.

https://xhr.spec.whatwg.org/#the-send()-method

"""
Otherwise, if the header whose name is a byte-case-insensitive match for
`Content-Type` in author request headers has a value that is a valid
MIME type, which has a `charset` parameter whose value is not a
byte-case-insensitive match for encoding, and encoding is not null, then
set all the `charset` parameters whose value is not a
byte-case-insensitive match for encoding of that header’s value to
encoding.
"""

The new behavior is aligned with Firefox:
https://wpt.fyi/XMLHttpRequest/send-content-type-charset.htm

Bug: 651750
Change-Id: Iff716e5ceece20e60bf8e69a73a980a4ea4fc6b9
Reviewed-on: https://chromium-review.googlesource.com/773942
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518203}
[modify] https://crrev.com/75e4a3195fe136ef2965fc0aafbdaebc7321f88a/third_party/WebKit/LayoutTests/external/wpt/XMLHttpRequest/send-content-type-charset-expected.txt
[modify] https://crrev.com/75e4a3195fe136ef2965fc0aafbdaebc7321f88a/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp

Comment 7 by bugdroid1@chromium.org, Nov 28 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edf7e300d04cff8e2aedc76c2d6b3b19a7c22fcc

commit edf7e300d04cff8e2aedc76c2d6b3b19a7c22fcc
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Tue Nov 28 12:21:05 2017

Revert "[XHR] Do not replace request content-type "utf-8" charset"

This reverts commit 75e4a3195fe136ef2965fc0aafbdaebc7321f88a.

Reason for revert: After talking with the spec editor, we decided to
go back to the old behavior. This is safe because the change has not
been to beta/stable yet.

Original change's description:
> [XHR] Do not replace request content-type "utf-8" charset
> 
> The spec asks us to replace the charset to "UTF-8" only when it is not
> equal to "UTF-8" case insensitively.
> 
> https://xhr.spec.whatwg.org/#the-send()-method
> 
> """
> Otherwise, if the header whose name is a byte-case-insensitive match for
> `Content-Type` in author request headers has a value that is a valid
> MIME type, which has a `charset` parameter whose value is not a
> byte-case-insensitive match for encoding, and encoding is not null, then
> set all the `charset` parameters whose value is not a
> byte-case-insensitive match for encoding of that header’s value to
> encoding.
> """
> 
> The new behavior is aligned with Firefox:
> https://wpt.fyi/XMLHttpRequest/send-content-type-charset.htm
> 
> Bug: 651750
> Change-Id: Iff716e5ceece20e60bf8e69a73a980a4ea4fc6b9
> Reviewed-on: https://chromium-review.googlesource.com/773942
> Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
> Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#518203}

TBR=tyoshino@chromium.org,yhirano@chromium.org,foolip@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 651750
Change-Id: I72d4b8e9244dd7098748298800feecebb9a5b481
Reviewed-on: https://chromium-review.googlesource.com/792712
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519659}
[modify] https://crrev.com/edf7e300d04cff8e2aedc76c2d6b3b19a7c22fcc/third_party/WebKit/LayoutTests/external/wpt/XMLHttpRequest/send-content-type-charset-expected.txt
[modify] https://crrev.com/edf7e300d04cff8e2aedc76c2d6b3b19a7c22fcc/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp

Comment 8 by bugdroid1@chromium.org, Nov 29 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529

commit 7a94ffd61bf2df9eb5efa570eb922ca87d7f3529
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Wed Nov 29 01:24:32 2017

Refactor ParsedContentHeaderFieldParameters and related classes

This is a preliminary change for implementing charset replacement in
XHR on top of ParsedContentType.

This CL
 - Adds iterator getters for ParsedContentHeaderFieldParameters.
 - Makes ParsedContentHeaderFieldParameters a list of parameters
   instead of a dictionary.
 - Holds unmodified header names instead of lowered names.
 - Removes is_valid_ from ParsedContentHeaderFieldParameters, and adds
   the boolean (as part of Optional) to its holders.
 - Removed Mode::kStrict which can be checked after the parameters are
   parsed.

Bug: 651750
Change-Id: I35db41bee74de8ef33b0b95ae56d039599c5b556
Reviewed-on: https://chromium-review.googlesource.com/781208
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519944}
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/modules/media_capabilities/MediaCapabilities.cpp
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/HeaderFieldTokenizer.h
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentDisposition.cpp
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentDisposition.h
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentHeaderFieldParameters.cpp
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentHeaderFieldParameters.h
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentHeaderFieldParametersTest.cpp
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentType.cpp
[modify] https://crrev.com/7a94ffd61bf2df9eb5efa570eb922ca87d7f3529/third_party/WebKit/Source/platform/network/ParsedContentType.h

Comment 9 by foolip@chromium.org, Oct 12

Description: Show this description

Comment 10 by foolip@chromium.org, Oct 12

Description: Show this description

Comment 11 by foolip@chromium.org, Oct 12

Summary: Chrome-only failures in web-platform-tests for XHR (was: 10 failing web-platform-tests for XHR that pass in Firefox and Edge)
Of those originally reported, only these still fail in Chrome:
https://wpt.fyi/results/xhr/response-data-progress.htm
https://wpt.fyi/results/xhr/responsexml-document-properties.htm (not Chrome-only)
https://wpt.fyi/results/xhr/responsexml-media-type.htm (not Chrome-only)
https://wpt.fyi/results/xhr/security-consideration.sub.html
https://wpt.fyi/results/xhr/send-content-type-charset.htm

In addition, these now have Chrome-only failures:
https://wpt.fyi/results/xhr/setrequestheader-allow-empty-value.htm
https://wpt.fyi/results/xhr/setrequestheader-allow-whitespace-in-value.htm
https://wpt.fyi/results/xhr/setrequestheader-content-type.htm

Updating the bug title to reflect this.

Comment 12 by foolip@chromium.org, Oct 17

Blocking: 896242

Comment 13 by foolip@chromium.org, Oct 18

Blocking: -651572

Comment 14 by bugdroid, Jan 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d54d17c648c5233e4e0d275601d1174c408ff6a3

commit d54d17c648c5233e4e0d275601d1174c408ff6a3
Author: Erik Anderson <Erik.Anderson@microsoft.com>
Date: Fri Jan 25 03:39:27 2019

XHR: Remove extra progress event dispatch when handling request error

The XMLHttpRequest spec (https://xhr.spec.whatwg.org/#request-error-steps) describes what steps to take in the event of a request error. Chrome currently dispatches a progress, an error, and a loadend event. Firefox and Edge 18.17763 follow the spec and only dispatch the error and loadend events.

Bug: 651750
Change-Id: I71512e7bb3dec9d706d36dc9f31bc7dc95de3bba
Reviewed-on: https://chromium-review.googlesource.com/c/1433114
Commit-Queue: Erik Anderson <Erik.Anderson@microsoft.com>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625979}

Sign in to add a comment