A media file that does not provide MIME a type in the markup, eg. does not use <source type=...>, will not load if the url has a non-media file extension for which MIMETypeRegistry::getMediaMIMETypeForExtension() returns a type. This is caused by the changes in http://trac.webkit.org/changeset/102800 in many cases because we now correctly extract the file extension from query urls.
<rdar://problem/10904486>
<rdar://problem/11441432>
Created attachment 141590 [details] Proposed patch
Comment on attachment 141590 [details] Proposed patch Attachment 141590 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12679123
Comment on attachment 141590 [details] Proposed patch Attachment 141590 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12678503
Comment on attachment 141590 [details] Proposed patch Attachment 141590 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12666923
Created attachment 141600 [details] Updated patch - this time with not as many uninitialized variables.
Comment on attachment 141600 [details] Updated patch - this time with not as many uninitialized variables. View in context: https://bugs.webkit.org/attachment.cgi?id=141600&action=review > Source/WebCore/platform/graphics/MediaPlayer.cpp:328 > + , m_typeInferredFromExtension(false) This variable name sounds like it would be a type, not a boolean. m_typeWasInferredFromExtension should like a boolean. Or for additional clarity, m_contentMIMETypeWasInferredFromExtension. > Source/WebCore/platform/graphics/MediaPlayer.cpp:354 > + m_typeInferredFromExtension = false; Seems strange that for m_url, m_contentMIMEType, and m_contentTypeCodecs we use local variables and set the data member only at the end, but for m_typeInferredFromExtension we set the data member directly. The code would be easier to read if all worked the same way. > Source/WebCore/platform/graphics/MediaPlayer.cpp:369 > String mediaType = MIMETypeRegistry::getMediaMIMETypeForExtension(extension); > - if (!mediaType.isEmpty()) > + if (!mediaType.isEmpty()) { > type = mediaType; > + m_typeInferredFromExtension = true; > + } Seems like for clarity mediaType could be named inferredType.
(In reply to comment #8) > (From update of attachment 141600 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=141600&action=review > > > Source/WebCore/platform/graphics/MediaPlayer.cpp:328 > > + , m_typeInferredFromExtension(false) > > This variable name sounds like it would be a type, not a boolean. m_typeWasInferredFromExtension should like a boolean. Or for additional clarity, m_contentMIMETypeWasInferredFromExtension. > Done. > > Source/WebCore/platform/graphics/MediaPlayer.cpp:354 > > + m_typeInferredFromExtension = false; > > Seems strange that for m_url, m_contentMIMEType, and m_contentTypeCodecs we use local variables and set the data member only at the end, but for m_typeInferredFromExtension we set the data member directly. The code would be easier to read if all worked the same way. > Done. > > Source/WebCore/platform/graphics/MediaPlayer.cpp:369 > > String mediaType = MIMETypeRegistry::getMediaMIMETypeForExtension(extension); > > - if (!mediaType.isEmpty()) > > + if (!mediaType.isEmpty()) { > > type = mediaType; > > + m_typeInferredFromExtension = true; > > + } > > Seems like for clarity mediaType could be named inferredType. Excellent point, but I forgot to fix this before checking it in. I will clean this up the next time I modify this file.
http://trac.webkit.org/changeset/116960
(In reply to comment #10) > http://trac.webkit.org/changeset/116960 The test checked in with this change fails on Lion. http://build.webkit.org/results/Lion%20Debug%20(Tests)/r116960%20(6573)/http/tests/media/video-query-url-pretty-diff.html https://bugs.webkit.org/show_bug.cgi?id=86527 tracks this.