[webkit-reviews] review denied: [Bug 36902] [Qt] Patch to allow java applet support through QWebPluginFactory... : [Attachment 52210] Java applet support through QWebPluginFactory::create I

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied adawit at kde.org's request for
review:
Bug 36902: [Qt] Patch to allow java applet support through QWebPluginFactory...
https://bugs.webkit.org/show_bug.cgi?id=36902

Attachment 52210: Java applet support through QWebPluginFactory::create I
https://bugs.webkit.org/attachment.cgi?id=52210&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
> diff --git a/WebKit/qt/ChangeLog b/WebKit/qt/ChangeLog
> index 3a4b096..f811546 100644
> --- a/WebKit/qt/ChangeLog
> +++ b/WebKit/qt/ChangeLog
> @@ -1,3 +1,14 @@
> +2010-03-31  Dawit Alemayehu	<adawit at kde.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Allow java applet support to be implemented through
> +	   QWebPluginFactory.

Please include a link to the bugzilla entry in the ChangeLog entry :)

> +	   * WebCoreSupport/FrameLoaderClientQt.cpp:
> +	   (WebCore::FrameLoaderClientQt::createPlugin):
> +	   (WebCore::FrameLoaderClientQt::createJavaAppletWidget):
> +
>  2010-03-26  Kenneth Rohde Christiansen  <kenneth at webkit.org>
>  
>	   Reviewed by Antti Koivisto.
> diff --git a/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp
b/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp
> index c5c5739..b0ff002 100644
> --- a/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp
> +++ b/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp
> @@ -1340,9 +1340,11 @@ PassRefPtr<Widget>
FrameLoaderClientQt::createPlugin(const IntSize& pluginSize,
>	       }
>	   }
>  
> -	   RefPtr<PluginView> pluginView = PluginView::create(m_frame,
pluginSize, element, url,
> -	       params, values, mimeType, loadManually);
> -	   return pluginView;
> +	   if (mimeType != "application/x-java-applet") {
> +	       RefPtr<PluginView> pluginView = PluginView::create(m_frame,
pluginSize, element, url, params, values, mimeType, loadManually);	  
> +	       if (pluginView->status() == PluginStatusLoadedSuccessfully)
> +		   return pluginView;
> +	   }

This part looks incorrect to me. Why are you avoiding the calls to
PluginView::create?

I thought especially on platforms where the Java support comes through an NPAPI
plugin - see Bug #33044 - AFAICS that call is needed.

>      return 0;
> @@ -1355,11 +1357,10 @@ void
FrameLoaderClientQt::redirectDataToPlugin(Widget* pluginWidget)
>      m_hasSentResponseToPlugin = false;
>  }
>  
> -PassRefPtr<Widget> FrameLoaderClientQt::createJavaAppletWidget(const
IntSize&, HTMLAppletElement*, const KURL&,
> -						       const Vector<String>&,
const Vector<String>&)
> +PassRefPtr<Widget> FrameLoaderClientQt::createJavaAppletWidget(const
IntSize& pluginSize, HTMLAppletElement* element, const KURL& baseURL,
> +						       const Vector<String>&
paramNames, const Vector<String>& paramValues)
>  {
> -    notImplemented();
> -    return 0;
> +    return FrameLoaderClientQt::createPlugin(pluginSize, element, baseURL,
paramNames, paramValues, "application/x-java-applet", false);
>  }

I'm fine with this part of the patch :)