Bug 44343 - Media element canPlayType("application/octet-stream") not handled correctly
Summary: Media element canPlayType("application/octet-stream") not handled correctly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media Elements (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords:
Depends on:
Blocks: 51249
  Show dependency treegraph
 
Reported: 2010-08-20 11:12 PDT by Eric Carlson
Modified: 2011-01-24 12:19 PST (History)
7 users (show)

See Also:


Attachments
proposed patch (7.36 KB, patch)
2010-08-20 12:01 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch (8.09 KB, patch)
2010-08-20 13:40 PDT, Eric Carlson
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2010-08-20 11:12:33 PDT
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
Comment 1 Eric Carlson 2010-08-20 12:01:44 PDT
Created attachment 64967 [details]
proposed patch
Comment 2 WebKit Review Bot 2010-08-20 12:06:01 PDT
Attachment 64967 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3737404
Comment 3 Early Warning System Bot 2010-08-20 12:08:21 PDT
Attachment 64967 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3732389
Comment 4 WebKit Review Bot 2010-08-20 12:10:56 PDT
Attachment 64967 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3799097
Comment 5 Eric Seidel 2010-08-20 12:10:58 PDT
Attachment 64967 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3728405
Comment 6 WebKit Review Bot 2010-08-20 13:27:26 PDT
Attachment 64967 [details] did not build on win:
Build output: http://queues.webkit.org/results/3756470
Comment 7 Eric Carlson 2010-08-20 13:40:00 PDT
Created attachment 64987 [details]
Patch

Attach the correct version of the patch this time.
Comment 8 Eric Carlson 2010-08-20 15:21:37 PDT
http://trac.webkit.org/changeset/65758
Comment 9 Darin Adler 2010-08-24 13:02:54 PDT
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.
Comment 10 Eric Carlson 2010-08-25 11:05:31 PDT
(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.
Comment 11 Ademar Reis 2011-01-24 12:19:56 PST
Revision r65758 cherry-picked into qtwebkit-2.2 with commit f8fc17d <http://gitorious.org/webkit/qtwebkit/commit/f8fc17d>