Media element canPlayType("application/octet-stream") not handled correctly

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

The spec says:

The MIME type "application/octet-stream" with no parameters is never a type that the user agent knows it cannot render. User agents must treat that type as equivalent to the lack of any explicit Content-Type metadata when it is used to label a potential media resource.

Note: In the absence of a specification to the contrary, the MIME type "application/octet-stream" when used with parameters, e.g. "application/octet-stream;codecs=theora", is a type that the user agent knows it cannot render

[Eric Carlson]

Created attachment 64967 [details]
proposed patch

[WebKit Review Bot]

Attachment 64967 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3737404

[Early Warning System Bot]

Attachment 64967 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3732389

[WebKit Review Bot]

Attachment 64967 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3799097

[Eric Seidel (no email)]

Attachment 64967 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3728405

[WebKit Review Bot]

Attachment 64967 [details] did not build on win:
Build output: http://queues.webkit.org/results/3756470

[Eric Carlson]

Created attachment 64987 [details]
Patch

Attach the correct version of the patch this time.

[Eric Carlson]

http://trac.webkit.org/changeset/65758

[Darin Adler]

Comment on attachment 64987 [details]
Patch

> +    if (type == applicationOctetStream()) {

Since MIME type are not case sensitive, I assume that type is already lowercased. If not, then we want to use equalIgnoringCase.

> +    if (type.isEmpty() || type == applicationOctetStream() || type == textPlain()) {

Same comment.

> +    if (type == applicationOctetStream()) {

And here.

> +++ Test with <video> element.
> +EXPECTED (mediaElement.canPlayType('application/octet-stream') == 'maybe') OK
> +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=theora') == '') OK
> +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=mp4') == '') OK
> +
> +++ Test with <audio> element.
> +EXPECTED (mediaElement.canPlayType('application/octet-stream') == 'maybe') OK
> +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=theora') == '') OK
> +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=mp4') == '') OK
> +END OF TEST

Would be good to test case sensitivity too. For the MIME type itself, the string "codecs" and the code values as well.

[Eric Carlson]

(In reply to comment #9)
> (From update of attachment 64987 [details])
> > +    if (type == applicationOctetStream()) {
> 
> Since MIME type are not case sensitive, I assume that type is already lowercased. If not, then we want to use equalIgnoringCase.
> 
> > +    if (type.isEmpty() || type == applicationOctetStream() || type == textPlain()) {
> 
> Same comment.
> 
> > +    if (type == applicationOctetStream()) {
> 
> And here.
> 
> > +++ Test with <video> element.
> > +EXPECTED (mediaElement.canPlayType('application/octet-stream') == 'maybe') OK
> > +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=theora') == '') OK
> > +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=mp4') == '') OK
> > +
> > +++ Test with <audio> element.
> > +EXPECTED (mediaElement.canPlayType('application/octet-stream') == 'maybe') OK
> > +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=theora') == '') OK
> > +EXPECTED (mediaElement.canPlayType('application/octet-stream;codecs=mp4') == '') OK
> > +END OF TEST
> 
> Would be good to test case sensitivity too. For the MIME type itself, 

Good suggestion, we weren't doing the right thing here. I fixed that in https://bugs.webkit.org/show_bug.cgi?id=44577. 

> the string "codecs" and the code values as well.

"codecs" is already detected correctly in ContentType::parameter, but according to RFC 1521:

    Parameter values are normally case sensitive, but certain parameters are interpreted to 
    be case-insensitive, depending on the intended use.

so we leave the codec values alone.

[Ademar Reis]

Revision r65758 cherry-picked into qtwebkit-2.2 with commit f8fc17d <http://gitorious.org/webkit/qtwebkit/commit/f8fc17d>