Bug 120383

Summary: [Windows] Loader is not properly determining supported MIME Types
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebCore Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, eric.carlson, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch eric.carlson: review+, buildbot: commit-queue-

Description Brent Fulgham 2013-08-27 16:27:45 PDT
The Windows port of WebKit does not properly identify media MIME types supported by the current WebKit infrastructure. The easiest way to see this is to uninstall the QuickTime player and then attempt to play one of the natively supported media types.  WebKit does not check for internally supported media types, and rejects the content if it cannot find a plugin that provides support for the MIME type.
Comment 1 Brent Fulgham 2013-08-27 16:28:40 PDT
This bug is visible when requesting a WebView display a media type (e.g., http://test-safari.apple.com/captions/media/AVSCCsync720P_AW_122.mov).
Comment 2 Radar WebKit Bug Importer 2013-08-27 16:29:07 PDT
<rdar://problem/14851052>
Comment 3 Brent Fulgham 2013-08-27 16:31:39 PDT
This patch was put together with Eric Carlson, though the refactoring to avoid the String->BSTR->String round trip was added later.
Comment 4 Brent Fulgham 2013-08-27 16:31:57 PDT
Created attachment 209819 [details]
Patch
Comment 5 Brent Fulgham 2013-08-27 16:33:27 PDT
Created attachment 209820 [details]
Patch
Comment 6 Build Bot 2013-08-27 17:11:03 PDT
Comment on attachment 209820 [details]
Patch

Attachment 209820 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1625077
Comment 7 Darin Adler 2013-08-27 18:15:03 PDT
Comment on attachment 209820 [details]
Patch

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

> Source/WebKit/win/WebView.cpp:2579
> +    notImplemented();
> +    return true;

Sorry, I don’t get why this isn’t implemented.
Comment 8 Brent Fulgham 2013-08-27 22:15:59 PDT
Comment on attachment 209820 [details]
Patch

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

>> Source/WebKit/win/WebView.cpp:2579
>> +    return true;
> 
> Sorry, I don’t get why this isn’t implemented.

It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement.
Comment 9 Brent Fulgham 2013-08-27 22:16:01 PDT
Comment on attachment 209820 [details]
Patch

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

>> Source/WebKit/win/WebView.cpp:2579
>> +    return true;
> 
> Sorry, I don’t get why this isn’t implemented.

It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement.
Comment 10 Brent Fulgham 2013-08-28 09:18:02 PDT
(In reply to comment #6)
> (From update of attachment 209820 [details])
> Attachment 209820 [details] did not pass win-ews (win):
> Output: http://webkit-queues.appspot.com/results/1625077

This output seems wrong.  All individual projects build, but the overall state is marked 'Failed'.

I also do not see any build errors when performing a local build.
Comment 11 Eric Carlson 2013-08-28 09:41:31 PDT
Comment on attachment 209820 [details]
Patch

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

>>>> Source/WebKit/win/WebView.cpp:2579
>>>> +    return true;
>>> 
>>> Sorry, I don’t get why this isn’t implemented.
>> 
>> It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement.
> 
> It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement.

It might be worth filing about about implementing all of the missing parts of WebView.

> Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:926
> +bool WebFrameLoaderClient::canShowMIMEType(const String& MIMEType) const

Although we are inconsistent about whether a "MIME type" variable should be "MIMEType" or "mimeType", the later is more than 8x more common so I would suggest using that. If you prefer the former, you should at least make all uses in this patch consistent.
Comment 12 Brent Fulgham 2013-08-28 10:28:09 PDT
Committed r154759: <http://trac.webkit.org/changeset/154759>
Comment 13 Brent Fulgham 2013-08-28 10:52:34 PDT
(In reply to comment #11)
> (From update of attachment 209820 [details])
> It might be worth filing about about implementing all of the missing parts of WebView.

Done. Filed as https://bugs.webkit.org/show_bug.cgi?id=120426.