Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Atomic HTTP redirect handling #55

Merged
merged 4 commits into from
Jan 11, 2015
Merged

Test Atomic HTTP redirect handling #55

merged 4 commits into from
Jan 11, 2015

Conversation

josh
Copy link
Contributor

@josh josh commented Jan 5, 2015

https://fetch.spec.whatwg.org/#atomic-http-redirect-handling

Should behave just like XHR's redirection.

@annevk is Response.prototype.url supposed to be the redirected location? That was one of my feature wish list items that XHR can't do.

Closes #42.

/cc @dgraham @annevk

@josh
Copy link
Contributor Author

josh commented Jan 5, 2015

Gah, PhantomJS doesn't implement 308.

@dgraham
Copy link
Contributor

dgraham commented Jan 6, 2015

is Response.prototype.url supposed to be the redirected location?

Section 4.2 steps 4.4 and 4.10 say yes.

@annevk
Copy link

annevk commented Jan 6, 2015

@josh XMLHttpRequest has responseURL these days. (And yes, it is the URL of the of the actual response.)

@josh
Copy link
Contributor Author

josh commented Jan 6, 2015

XMLHttpRequest has responseURL these days.

Nice! but looks like Safari still doesn't. I doubt IE does either. I wonder what we should set response.url to in the case that responseURL is undefined.

In a pre-responseURL world, github.com sends back a X-Request-URL header on every XHR. Then we can getResponseHeader that value. wdyt about this polyfill suggesting a similar fallback?

  1. Try responseURL
  2. Try getResponseHeader('X-Request-URL')
  3. Fallback to the originally requested url. Will be wrong if a redirect happened, but mostly right otherwise.

@josh
Copy link
Contributor Author

josh commented Jan 6, 2015

All tests pass on Chrome Canary (native fetch), Chrome and Firefox. Currently failing on Safari and IE because of missing responseURL.

@dgraham
Copy link
Contributor

dgraham commented Jan 6, 2015

I'm hesitant to move some of our github.com header logic into the polyfill. The native fetch implementations won't have that header, so polyfill users could become dependent on non-standard behavior that disappears when browsers enable window.fetch natively.

@josh
Copy link
Contributor Author

josh commented Jan 6, 2015

The native fetch implementations won't have that header

Native implementations already do the right thing. The header does nothing in that case.

josh added a commit that referenced this pull request Jan 11, 2015
Test Atomic HTTP redirect handling
@josh josh merged commit fcaa6f3 into master Jan 11, 2015
@josh josh deleted the test-redirect branch January 11, 2015 00:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test for HTTP redirect behavior
3 participants