Bug 44343

Summary: Media element canPlayType("application/octet-stream") not handled correctly
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, dglazkov, eric, gustavo, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 51249    
Attachments:
Description Flags
proposed patch
none
Patch mitz: review+

Eric Carlson
Reported 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
Attachments
proposed patch (7.36 KB, patch)
2010-08-20 12:01 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch (8.09 KB, patch)
2010-08-20 13:40 PDT, Eric Carlson 		mitz: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Carlson
Comment 1 2010-08-20 12:01:44 PDT
Created attachment 64967 [details] proposed patch
WebKit Review Bot
Comment 2 2010-08-20 12:06:01 PDT
Attachment 64967 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3737404
Early Warning System Bot
Comment 3 2010-08-20 12:08:21 PDT
Attachment 64967 [details] did not build on qt: Build output: http://queues.webkit.org/results/3732389
WebKit Review Bot
Comment 4 2010-08-20 12:10:56 PDT
Attachment 64967 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3799097
Eric Seidel (no email)
Comment 5 2010-08-20 12:10:58 PDT
Attachment 64967 [details] did not build on mac: Build output: http://queues.webkit.org/results/3728405
WebKit Review Bot
Comment 6 2010-08-20 13:27:26 PDT
Attachment 64967 [details] did not build on win: Build output: http://queues.webkit.org/results/3756470
Eric Carlson
Comment 7 2010-08-20 13:40:00 PDT
Created attachment 64987 [details] Patch Attach the correct version of the patch this time.
Eric Carlson
Comment 8 2010-08-20 15:21:37 PDT
http://trac.webkit.org/changeset/65758
Darin Adler
Comment 9 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.
Eric Carlson
Comment 10 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.
Ademar Reis
Comment 11 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>
Note You need to log in before you can comment on or make changes to this bug.