LibWeb: Implement Blob::text and Blob::array_buffer to spec #19404
+179
−27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
👀 pr-needs-review
shannonbooth
changed the title
mattco98
reviewed
Jun 15, 2023
Userland/Libraries/LibWeb/Streams/ReadableStreamDefaultReader.cpp
Outdated
Show resolved
Hide resolved
Userland/Libraries/LibWeb/Streams/ReadableStreamDefaultReader.cpp
Outdated
Show resolved
Hide resolved
|
@mattco98 just in case you know, would you have any idea what may be resulting in needing this FIXME? I've been looking through the spec, and haven't found anything yet for that. I'm thinking there is a possibility of it being a spec issue, but am not confident in that at all. |
I agree, this does seem like a spec issue. It looks like JSC also does this, though of course they're doing it asynchronously. |
shannonbooth
changed the title
|
Thanks @mattco98 for the review, think I should have hopefully addressed everything 😀 |
mattco98
approved these changes
Jun 17, 2023
|
No worries! |
kennethmyhra
suggested changes
Jun 17, 2023
I found myself needing to call this method when attempting to implement Blob::text and Blob::array_buffer. Turns out that the only caller outside of the Detail namespace already had a FIXME to make this a public API - so let's do that.
This algorithm is used by ReadableStreamDefaultReader to read all bytes from a given stream. Currently the algorithm used is somewhat naive as it is recursive, but given the initial use of this reader, it should not be a problem.
This is not to the specification, but as the FIXME comment for the function states, we need it to be able to properly interface with the FileAPI spcification, which seems to have not been updated to the non promise based API.
kennethmyhra
added
the
✅ pr-community-approved
github-actions
bot
removed
👀 pr-needs-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After adding ReadableStreamDefaultReader::read_all_bytes, all of the
pieces are in place to implement these functions to spec!