-
Notifications
You must be signed in to change notification settings - Fork 84
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
Fetching should be defined in terms of Fetch #261
Comments
duplicate of issue #188. (what is unclear about referer handling?) |
How is it clear? :-) |
Heh. Well each endpoint specifies whether it is sent or not, e.g.:
I don't disagree with the fetch suggestion, but is that line not clear? |
Ah, fair, that's actually pretty clear (though can a referrer policy override it?), though the algorithm I was looking at https://fedidcg.github.io/FedCM/#browser-api-rp-sign-in does not seem to specify anything. |
Ah yes. That should be better integrated with the other section, thanks for pointing that out |
I'm going to reopen this issue and narrow the scope to clarifying that issue |
(I can't figure out how to change the title of a github issue...) |
(next to the green "New issue" button there's a gray "Edit" button, both to the right of the heading) |
I must be missing some permission to do that :( |
It may be only @samuelgoto has the permissions to edit... |
We integrated one of the fetch calls in here https://fedidcg.github.io/FedCM/#check-the-root-manifest. Does this look OK? If so then we can move ahead and integrate the others similarly, if not let's improve the integration of that one more and then model the other ones on top of this one. |
This needs to be a list, no? You might want to assert provider’s configURL's scheme for clarity here.
You want to use https://fetch.spec.whatwg.org/#concept-header-extract-mime-type here. You also need to set many more fields of the request, as discussed in some of the other issues. |
Also, what you have attempted to define is that "check the root manifest" returns a boolean but instead it returns early and then the callback passed to processResponseConsumeBody returns a boolean which is then ignored. |
for reference, whatwg/fetch#1495 (if accepted) spec's a "web-identity" value for Sec-Fetch-Dest |
Closing but feel free to provide feedback if there's something off with how we integrated with Fetch. |
"check the root manifest" is still broken as it expects fetch to block or some such. |
@annevk help me understand how to actually use the
Is there a notion of 'await' in the spec so this becomes correct? Would it be ok to have step 3 be |
I mean in general you cannot really invoke fetch from "in parallel" without a lot of work so you'll have to account for it "calling you back" at some point. Otherwise you'd have to snapshot a bunch of state from the global object and take that with you somehow (because Fetch will need it for enforcing CSP and other things). And none of that is really worked out in detail. (And yeah, some of HTML is indeed broken in this way, but hopefully for new specs we don't repeat those mistakes.) |
Ok let me reopen for now. What's missing here is rewriting everything so that we need to call an algorithm from fetch instead of returning a variable updated by fetch. This requires a lot of updates to the algorithms but I'll try to get to it soon. |
Is this done now that #378 has landed? |
I think so, the remaining piece is the |
Requirements such as
are not needed if this specification would build on top of Fetch. That would also address a number of other things that are unclear, such as referrer handling, etc.
The text was updated successfully, but these errors were encountered: