Bug 51195 – MediaPlayer should look for MIME type in data: URL

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

MediaPlayer::load should try to get the MIME type from the url when passed a data: url.

[Eric Carlson]

Created attachment 76786 [details]
Proposed patch

[Darin Adler]

Comment on attachment 76786 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=76786&action=review

Can we make tests for this?

> WebCore/platform/graphics/MediaPlayer.cpp:306
> +    // If the MIME type is missing or is not meaningful, try to figure it out from the url.
> +    if (type.isEmpty() && protocolIs(url, "data"))

The comment says “is not meaningful”, but the code only checks for empty strings. Aren’t there other non-meaningful MIME types?

I suggest capitalizing URL in the comment.

I suggest we add protocolIsData to KURL.h. We already have it for actual KURL objects and I’d like to have it for strings. It’s not good to have the actual "data" string in the code. We should also adopt protocolIsData in KURL.cpp, KURLGoogle.cpp, DataURL.cpp, ResourceHandleSoup.cpp. None of that needs to be done right away or by you.

[Eric Carlson]

(In reply to comment #2)
> 
> Can we make tests for this?
> 
  The MIME type is only used internally within MediaPlayer so I couldn't think of a way to create a test.

> > WebCore/platform/graphics/MediaPlayer.cpp:306
> > +    // If the MIME type is missing or is not meaningful, try to figure it out from the url.
> > +    if (type.isEmpty() && protocolIs(url, "data"))
> 
> The comment says “is not meaningful”, but the code only checks for empty strings. Aren’t there other non-meaningful MIME types?
> 
  Good point, we try to infer the MIME type from the file extension when passed application/octetstream or text/plain, so we should look for type in a data url when passed those types as well.

[Eric Carlson]

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

[Jessie Berlin]

(In reply to comment #4)
> http://trac.webkit.org/changeset/74504

I think this change broke media/audio-data-url.html in the Windows 7 Release Tests:

http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r74504%20(7657)/media/audio-data-url-pretty-diff.html

[WebKit Review Bot]

http://trac.webkit.org/changeset/74504 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
media/audio-data-url.html

[Jessie Berlin]

Bug for the failing Windows 7 Release Tests

https://bugs.webkit.org/show_bug.cgi?id=51518

[Philippe Normand]

(In reply to comment #6)
> http://trac.webkit.org/changeset/74504 might have broken GTK Linux 64-bit Debug
> The following tests are not passing:
> media/audio-data-url.html

https://bugs.webkit.org/show_bug.cgi?id=51525 ...