-
Notifications
You must be signed in to change notification settings - Fork 323
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
Pass recursive=false to manual navigation redirect #1638
Conversation
<p>If <var>request</var>'s <a for=request>redirect mode</a> is "<code>manual</code>", then: | ||
|
||
<ol> | ||
<li><p>Assert: <var>request</var>'a <a for=request>mode</a> is "<code>navigate</code>". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect. You can always set it to "manual" and get an opaque response at the moment. Mainly useful in service workers with navigations though.
I suggest we do something like
Let recursive be true if ....; otherwise false.
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea but if it's manual non-navigation you wouldn't get to this bit as fetch would never process your redirect. Anyway, revised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, that's good pushback and solid rationale. Why revise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mainly because that assert is potentially more confusing than helpful and I wasn't sure about it. Should we keep it or throw it? I could go either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that it's correct it seems like a good addition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
This reverts commit f56575b.
Closes #1629
Preview | Diff