Bug 120383 – [Windows] Loader is not properly determining supported MIME Types

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

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.

[Brent Fulgham]

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).

[Radar WebKit Bug Importer]

<rdar://problem/14851052>

[Brent Fulgham]

This patch was put together with Eric Carlson, though the refactoring to avoid the String->BSTR->String round trip was added later.

[Brent Fulgham]

Created attachment 209819 [details]
Patch

[Brent Fulgham]

Created attachment 209820 [details]
Patch

[Build Bot]

Comment on attachment 209820 [details]
Patch

Attachment 209820 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1625077

[Darin Adler]

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.

[Brent Fulgham]

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.

[Brent Fulgham]

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.

[Brent Fulgham]

(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.

[Eric Carlson]

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.

[Brent Fulgham]

Committed r154759: <http://trac.webkit.org/changeset/154759>

[Brent Fulgham]

(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.