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

Fix MIME type handling #559

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix MIME type handling #559

wants to merge 2 commits into from

Conversation

chrisn
Copy link
Member

@chrisn chrisn commented Aug 13, 2024

See #511. This PR removes the Valid Media MIME Type definition and references mimesniff for "parse a MIME type".

Further work is needed to address #512.


Preview | Diff

</p>
</li>
<li>
<p>
Let <var>container</var> be the container type specified by <var>content
type</var>.
If <var>mimeType</var> is <code>failure</code> or is unrecognized,
Copy link
Member Author

Choose a reason for hiding this comment

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

What does "unrecognized" mean here?

Copy link
Member

Choose a reason for hiding this comment

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

I was not the author of that phrase, but I think the distinction between "invalid"/"failure" and "unrecognized" is that you could have a valid MIME type that is not recognized as supported for media playback by the UA, e.g. "image/png".

If this is indeed the intent, I think the term "unrecognized" is not the best we could come up with.

How about If <var>mimeType</var> is <code>failure</code> or is not a supported type for media playback? Is that clear?

Copy link
Member

Choose a reason for hiding this comment

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

Joey's suggestion is better, but ideally we actually list the MIME types. See also https://mimesniff.spec.whatwg.org/#supported-by-the-user-agent and references.

Copy link
Member

Choose a reason for hiding this comment

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

I not convinced that an explicit list of supported media types is practical, given the variability in what is supported across the entire ecosystem of media streaming devices.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a related PR for Media Capabilities, see "Check MIME Type Support" in https://github.com/w3c/media-capabilities/pull/222/files. We may be able to reuse this, or maybe even just reference it. But in that PR we reference https://mimesniff.spec.whatwg.org/#supported-by-the-user-agent rather than list MIME types.

Copy link
Member

Choose a reason for hiding this comment

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

I think "supported by the user agent" plus limiting it to media in some way is reasonable. It will at least give us a central place to work on improving this longer term.

@chrisn
Copy link
Member Author

chrisn commented Aug 14, 2024

@annevk @joeyparrish How does this look? There may be more we could do to be more precise, e.g., "unrecognized", or how to identify the container type from the MIME type, but for now I've tried to reference mimesniff but without introducing other changes.

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Looks good % the choice of the word "unrecognized". It's fine with me if we keep this PR focused on mimesniff and follow-up to clarify "unrecognized" in another PR.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Thanks for working on tackling this!

Let <var>container</var> be the container type specified by <var>content
type</var>.
If <var>mimeType</var> is <code>failure</code> or is unrecognized,
continue to the next iteration.
Copy link
Member

Choose a reason for hiding this comment

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

At some point you might want to adopt https://infra.spec.whatwg.org/#iteration-continue and other Infra conventions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, let's do this in a separate PR.

encrypted-media-respec.html Outdated Show resolved Hide resolved
@chrisn
Copy link
Member Author

chrisn commented Aug 19, 2024

It's fine with me if we keep this PR focused on mimesniff and follow-up to clarify "unrecognized" in another PR.

I agree, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants