[webkit-reviews] review denied: [Bug 112834] [WebKit2] Plugins without a MIME Type fail to load : [Attachment 194759] Rebased patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 112834: [WebKit2] Plugins without a MIME Type fail to load
https://bugs.webkit.org/show_bug.cgi?id=112834

Attachment 194759: Rebased patch
https://bugs.webkit.org/attachment.cgi?id=194759&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=194759&action=review


Som comments bellow + I think this deserve a test.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1333
> -void WebPageProxy::getPluginPath(const String& mimeType, const String&
urlString, const String& frameURLString, const String& pageURLString, String&
pluginPath, uint32_t& pluginLoadPolicy)
> +void WebPageProxy::getPluginPath(const String& mimeType, const String&
urlString, const String& frameURLString, const String& pageURLString, String&
pluginPath, String& newMimeType, uint32_t& pluginLoadPolicy)

The method become badly named with the addition.
I think it is okay to have the new MIME type returned here, but not from a
function named getPluginPath.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1337
> -    String newMimeType = mimeType.lower();
> +    newMimeType = mimeType.lower();

A bit unrelated but I am surprised you need to call lower() here. Usually, when
handling MIME types in WebCore, we make them lower case as soon as we get the
input for the user space.