Remove fallback to BlobBuilder if Blob constructor is missing #19277
Conversation
This file contains hidden or 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
|
View with "Hide whitespace" to see the real changes here. |
This code for handling the absence of Blob constructor goes back to 2012: a97aab0 I'm pretty there is nobody depending on this today. I think we would have to go back to chrome 32 or firefox 14 or safari 6 in order to actually need this: https://caniuse.com/blobbuilder I noticed this looked like it should be removed when following the link in tools/file_packager.py to https://crbug.com/124926, which was marked as WontFix over 10 years ago.
kripken
reviewed
May 2, 2023
| @@ -635,11 +635,7 @@ def generate_js(data_target, data_files, metadata): | |||
| Module['FS_createPreloadedFile'](this.name, null, byteArray, true, true, function() { | |||
| Module['removeRunDependency']('fp ' + that.name); | |||
| }, function() { | |||
| if (that.audio) { | |||
| Module['removeRunDependency']('fp ' + that.name); // workaround for chromium bug 124926 (still no audio with this, but at least we don't hang) | |||
| } else { | |||
There was a problem hiding this comment.
Why remove this? Don't we need the workaround if the bug is WONTFIX?
There was a problem hiding this comment.
The WONTFIX was for broken BlobBuilder. According to the bug:
Well, BlobBuilder appears to be deprecated (probably for quite a while now) and is undefined in current Linux Chrome dev M35. See comment #16 and http://updates.html5rocks.com/2012/06/Don-t-Build-Blobs-Construct-Them.
As attached, I modified the script in page.html to use the new Blob constructor instead, and the ogg file loads successfully and two sounds play, including when I replay.
And since we are using Blob constructor now there should no longer be an issue.
There was a problem hiding this comment.
I see. Yeah, that's probably right, then, though an earlier comment says
it happens with the Blob constructor too, not just the (deprecated) BlobBuilder
https://bugs.chromium.org/p/chromium/issues/detail?id=124926#c16
But the later comment says it works, and I do see it work locally when I try that modified page.html, so looks good.
kripken
approved these changes
May 2, 2023
sbc100
added a commit
that referenced
this pull request
May 3, 2023
Perhaps 10 years ago there was possibility that the URL object didn't exist, but today we don't support such browsers See https://caniuse.com/url. This is a followup to #19277 which did the same for the Blob constructor.
sbc100
added a commit
that referenced
this pull request
May 3, 2023
Perhaps 10 years ago there was possibility that the URL object didn't exist, but today we don't support such browsers See https://caniuse.com/url. This is a followup to #19277 which did the same for the Blob constructor.
sbc100
added a commit
that referenced
this pull request
May 3, 2023
Perhaps 10 years ago there was possibility that the URL object didn't exist, but today we don't support such browsers See https://caniuse.com/url. This is a followup to #19277 which did the same for the Blob constructor.
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.
This code for handling the absence of Blob constructor goes back to 2012: a97aab0
I'm pretty sure there is nobody depending on this today. I think we would have to go back to chrome 32 or firefox 14 or safari 6 in order to actually need this: https://caniuse.com/blobbuilder
I noticed this looked like it should be removed when following the link in tools/file_packager.py to https://crbug.com/124926, which was marked as WontFix over 10 years ago.