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

Editorial: fix usage of processResponseConsumeBody #8412

Merged
merged 7 commits into from
Oct 31, 2022

Conversation

nicolo-ribaudo
Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo commented Oct 20, 2022

Ref #7996. I didn't update synchronous perform the fetch steps, because I couldn't find what's the signature of Fetch when using it synchronously.

TODO:


/infrastructure.html ( diff )
/links.html ( diff )
/semantics.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )
/worklets.html ( diff )

source Show resolved Hide resolved
@nicolo-ribaudo nicolo-ribaudo force-pushed the proper-processResponseConsumeBody branch from 97445cf to 07fb2c0 Compare October 20, 2022 22:15
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you as always!

source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@nicolo-ribaudo
Copy link
Contributor Author

I added a commit to also fix the signature for synchronous fetching, by removing synchronous fetching (since it's not supported anymore) and pausing while waiting for the result of a normal fetch.

Since there is now a single type of fetch, I reverted part of #8328: there is now again a single "perform the fetch" hook type, which accepts a callback that takes response and bytes. This is exactly how it works for the default fetch.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few minor nits. Thanks so much, again!! Ping the thread when service workers and manifest are ready to go. (Or if you decide you'd rather let the maintainers of those specs update themselves, which is totally valid; you've done so much already.)

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@nicolo-ribaudo
Copy link
Contributor Author

Done! I also updated w3c/ServiceWorker#1658, since I already had that PR open, and I would appreciate also a review on that side even if you are not the maintainer of that spec (but the changes are very similar to the changes in this PR).

nicolo-ribaudo added a commit to nicolo-ribaudo/manifest that referenced this pull request Oct 28, 2022
This commit implements the HTML changes done in whatwg/html#8412:
"processing a manifest" takes a response whose body has been already
consumed, so it needs to take the body bytes as a separate parameter.
marcoscaceres added a commit to w3c/manifest that referenced this pull request Oct 31, 2022
This commit implements the HTML changes done in whatwg/html#8412:
"processing a manifest" takes a response whose body has been already
consumed, so it needs to take the body bytes as a separate parameter.

Co-authored-by: Marcos Cáceres <marcos@marcosc.com>
@domenic domenic merged commit c196f1b into whatwg:main Oct 31, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the proper-processResponseConsumeBody branch October 31, 2022 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants