IE 9+ fixes #28
IE 9+ fixes #28
Conversation
| @@ -115,7 +115,7 @@ | |||
| this.body = options.body | |||
| this.credentials = options.credentials || null | |||
| this.headers = new Headers(options.headers) | |||
| this.method = options.method || 'GET' | |||
| this.method = (options.method || 'GET').toUpperCase() | |||
There was a problem hiding this comment.
This isn't the right level for this patch. Sites that want to support IE9 need to pass in uppercase method verbs. The polyfill needs to pass the argument through to the XHR untouched.
There was a problem hiding this comment.
patch (lowercase) also doesn't work on Safari and Chrome. Since XHR is supposed to accept most methods as case-insensitive and that's not the case, it's a nice bonus point to have this discrepancy corrected. I would like to believe that when modern browsers actually implement fetch(), they will allow both lower-case and upper-case names.
In fact, the fetch spec says that methods should be case-insensitive. Unfortunately, the spec only lists DELETE, GET, HEAD, OPTIONS, POST, and PUT as supported methods.
There was a problem hiding this comment.
Ok, section 2.1.1 says to uppercase as normalization so this looks good.
@annevk Should PATCH be a supported method in the Fetch spec?
There was a problem hiding this comment.
Actually the spec explicitly states that Request constructor should normalize the method by uppercasing it:
14. If init's method member is present, let method be it and run these substeps:
1. If method is not a method or method is a forbidden method, throw a TypeError.
2. Normalize method.
3. Set request's method to method.
There was a problem hiding this comment.
Ok, section 2.1.1 says to uppercase as normalization so this looks good.
To be fair, it only says normalization should happen if it matches one of the whitelisted methods. We would also have to throw a TypeError when encountering one of 3 "forbidden" methods. I feel that we wouldn't gain much by following these details word by word, though, so I'm OK with the simplistic approach here.
There was a problem hiding this comment.
You gain compliance... Method names are case-sensitive in HTTP. That Blink/WebKit have a bug for "patch" and maybe some other names, does not mean it's okay to just uppercase them all.
There was a problem hiding this comment.
@annevk I'm sorry, the patch problem wasn't due to WebKit, but Node.js server (that we use in testing) rejected to treat it as PATCH. I forgot to check whether the backend supports it before blaming the browsers.
So, you were right. HTTP method names are case-sensitive, so I've reverted to normalizing only those HTTP methods that are whitelisted in fetch spec.
This fixes PUT/DELETE for IE 9 and PATCH for Node.js.
Now only the whitelisted method names get normalized by uppercasing and the rest are left untouched since HTTP method names in general are case-sensitive. In tests we must use `PATCH` (uppercase) because Node.js server doesn't support `patch` (lowercase).
PhantomJS doesn't seem to send request body for PATCH: ariya/phantomjs#11384
By default, all static files served from the test server were "text/plain". IE auto-detects HTML and JavaScript content as such, but refuses to display the style sheet until it is served as "text/css".
If the body passed to `xhr.send()` is `undefined`, the text sent would literally be "undefined" on IE.
IE caches xhr responses pretty aggressively and after restoring the previous response from cache, it blanks out the "Date" header (probably by design). The solution for this test is to use a cache-busting query parameter.
Reject all HTTP codes that are not in the 100..599 range.
When an empty reply is received from the server, `xhr.onerror` callback
should get triggered. IE 9, however, triggers the `onload` callback with
status code 12152 that stands for:
ERROR_HTTP_INVALID_SERVER_RESPONSE
The server response could not be parsed.
http://support.microsoft.com/kb/193625
IE 9 fails on 204s with error code 1223 http://www.enhanceie.com/ie/bugs.asp Rewrite the code back to 204. Note: response headers remain unavailable in this case. This was fixed in IE 10.
|
|
||
| function normalizeMethod(method) { | ||
| var upcased = method.toUpperCase() | ||
| return (methods.indexOf(upcased) > -1) ? upcased : method |
Conflicts: fetch.js
Previously renamed to _body to detect these types of errors.
See #30 for details.
|
Thanks, @mislav! |
Fix when running on Firefox with fetchPonyfill (and any other error cases)
Fix when running on Firefox with fetchPonyfill (and any other error cases)
The only test that now fails is the FormData test on IE 9. Maybe we should skip that test if FormData is unavailable?