Navigate: remove "gone async" and define redirect handling #1476
Conversation
Redirect handling had the following issues that are now fixed: * /a navigating to /b which redirects to /a#test would not unload /a. * Headers set on the initial request were not preserved. * Redirects to javascript URLs would not result in an error. Fixes #314. * Location header was not parsed, and HTTP method was not changed where needed. Fixes #461. Another source of complexity was the "gone async" mechanism, which was used for two steps that could just as easily be factored out and had to be since they were run synchronously, but should not be. Changing that fixes #1446 which discusses the issue around unknown URL schemes in more detail.
|
Does the second commit close #1353? |
| specified scheme is not one of the supported protocols, then abort these steps and <span | ||
| data-x="hand-off to external software">proceed with that mechanism instead</span>.</p></li> | ||
| <li><p>Cancel any preexisting but not yet <span data-x="concept-navigate-mature">mature</span> | ||
| attempt to navigate <var>browsingContext</var>, including canceling any instances of the <span |
There was a problem hiding this comment.
Should this be more precise and use https://fetch.spec.whatwg.org/#concept-fetch-terminate with reason "end-user abort"?
There was a problem hiding this comment.
I guess this concept is also used when aborting a document. Perhaps fetch should define what "cancel" means?
Can be done in a follow-up if you'd rather.
There was a problem hiding this comment.
This needs to be done in terms of fetch groups. And yes, we need to carefully consider what termination means in Fetch, especially with regards to cancelable promises and such. I haven't really thought that through and I suspect user agents might reject at the moment for some of this.
|
Review done. Very exciting improvements. This section is starting to look manageable after all!! |
|
This commit doesn't exactly close #1353, but we should probably close it anyway since more research is needed around that and how it works exactly. |
|
2 nits remain:
LGTM after those are fixed |
No description provided.