-
Notifications
You must be signed in to change notification settings - Fork 44
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
Modernize Blob constructor a tiny bit #125
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks! This looks good to me. Do you happen to have tests for this as well?
I don't. I wanted to fix the general type setup, but I keep running out of time. |
need to close and reopen to trigger new travis |
@annevk, could you please fix the merge conflicts? The constructor can now be updated to new syntax. |
Once there are sufficient tests I'm happy to address the remainder (including filing bugs). |
ok, no probs. Was worried about bit rot. How about just getting it to a good state and we just don't merge until there are tests? The only thing I was unsure about was the merge conflict with the algorithm, the IDL seems like a quick fix. |
All implementations do something different for new Blob(undefined, { type: "x/x" }). Let's do something simple. Fixes #54.
abb6af3
to
07dc190
Compare
@annevk, just saw this was still pending. Should this be updated and merged? |
@wisniewskit did you end up doing whatwg/mimesniff#189 (comment) in Gecko? Maybe I should take some time to make the specification say that to try to move this along. |
Looks like the plan is to change our behavior first and see whether it's web compatible, before changing WPT: https://phabricator.services.mozilla.com/D208848 |
@annevk, I haven't yet landed the patches for Gecko to try that, as I'm still figuring out which test cases in our own codebase fail, and how to deal with them. But it's not looking intractable, so hopefully we can land it sometime soon and get data on whether there's any obvious fallout on sites. (Of course it would be faster and fairer if others also did the same, so I welcome anyone willing to help push other vendors along as well!) |
All implementations do something different for new Blob(undefined, { type: "x/x" }). Let's do something simple.
Fixes #54.
Preview | Diff