|Summary:||XMLHttpRequest: make setRequestHeader() use `, ` as separator (including a space)|
|Product:||WebKit||Reporter:||Anne van Kesteren <annevk>|
|Component:||DOM||Assignee:||youenn fablet <youennf>|
|Severity:||Normal||CC:||achristensen, ap, cdumez, cgarcia, commit-queue, youennf|
|Version:||Safari Technology Preview|
Description Anne van Kesteren 2017-03-07 09:51:26 PST
See https://github.com/whatwg/xhr/issues/108. web-platform-tests at: XMLHttpRequest/setrequestheader-case-insensitive.htm XMLHttpRequest/setrequestheader-header-allowed.htm
Comment 1 Alexey Proskuryakov 2017-03-07 11:52:41 PST
We just changed it to never add a space, in part because doing otherwise would be complicated for CFNetwork based ports.
Comment 2 youenn fablet 2017-03-07 13:02:43 PST
If we change XHR to use ', ', we will also change it to fetch. Currently we are using ',' for both, except when CFNetwork is changing that behind the scene, which it should really not.
Comment 3 Anne van Kesteren 2017-03-08 00:44:33 PST
I have been trying to get your input in the issue, but am glad filing bugs is working out better. It sounds like the preference from WebKit is that we only have a single separator style, used by: XMLHttpRequest WebSocket Fetch API CORS Knowing that makes it easier to talk with the other browsers. I suspect what we want to do is use `, ` and also change Fetch to use that and any Fetch API tests. `Access-Control-Request-Headers` seems tricky though, as I believe no browser uses a space after the commas there now. Is that problematic?
Comment 4 youenn fablet 2017-03-08 08:18:46 PST
(In reply to comment #3) > I have been trying to get your input in the issue, but am glad filing bugs > is working out better. Sorry about that, I plan to get back to fetch/XHR soon. I should probably improve my GitHub notification filter. > It sounds like the preference from WebKit is that we only have a single > separator style, used by: > > XMLHttpRequest > WebSocket > Fetch API > CORS Right. > Knowing that makes it easier to talk with the other browsers. I suspect what > we want to do is use `, ` and also change Fetch to use that and any Fetch > API tests. Yes, that is probably the most interoperable choice. > `Access-Control-Request-Headers` seems tricky though, as I believe no > browser uses a space after the commas there now. Is that problematic? WebKit is already using `,` here so no problem, except the small inconsistency.
Comment 5 Anne van Kesteren 2017-03-10 07:28:00 PST
It seems Mozilla is tentatively okay with making this change. I prepared three pull requests to make this setup work: https://github.com/whatwg/fetch/pull/504 https://github.com/whatwg/xhr/pull/130 https://github.com/w3c/web-platform-tests/pull/5115 If I missed tests that need updating I'd appreciate your help with that. I'd also appreciate any review.
Comment 6 youenn fablet 2017-03-10 09:46:48 PST
Sure, I'll update WebKit side and upstream any needed change to WPT
Comment 7 Anne van Kesteren 2017-03-10 09:53:58 PST
(To be clear, I don't know if Mozilla is okay with lowercasing header names and reordering them for getAllResponseHeaders(). Those two items still need to be battle-tested somehow, but aligning on `, ` seems okay.)
Comment 9 WebKit Commit Bot 2017-03-11 22:07:08 PST
Comment on attachment 304069 [details] Patch Clearing flags on attachment: 304069 Committed r213766: <http://trac.webkit.org/changeset/213766>
Comment 10 WebKit Commit Bot 2017-03-11 22:07:13 PST
All reviewed patches have been landed. Closing bug.