[webkit-reviews] review granted: [Bug 120383] [Windows] Loader is not properly determining supported MIME Types : [Attachment 209820] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 120383: [Windows] Loader is not properly determining supported MIME Types
https://bugs.webkit.org/show_bug.cgi?id=120383

Attachment 209820: Patch
https://bugs.webkit.org/attachment.cgi?id=209820&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
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.