[Webkit-unassigned] [Bug 102339] Provide page/window coordinates to plugin's local coordinates translation in WebPluginContainer.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=102339


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #174501|review?                     |review-
               Flag|                            |




--- Comment #8 from Adam Barth <abarth at webkit.org>  2012-11-15 13:18:03 PST ---
(From update of attachment 174501)
View in context: https://bugs.webkit.org/attachment.cgi?id=174501&action=review

This patch is looking good.  Some minor details.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:307
> -static WebPluginContainerImpl* pluginContainerFromNode(const WebNode& node)
> +WebPluginContainerImpl* WebFrameImpl::pluginContainerFromNode(const WebNode& node)

Should this be a function on WebNode?

> Source/WebKit/chromium/tests/WebPluginContainerTest.cpp:74
> +        if (params.mimeType == WebKit::FakeWebPlugin::mimeType())

No need for "WebKit::".  We're already in the WebKit namespace.

> Source/WebKit/chromium/tests/WebPluginContainerTest.cpp:84
> +static WebFrameClient* pluginWebFrameClient()
> +{
> +    DEFINE_STATIC_LOCAL(TestPluginWebFrameClient, pluginClient, ());
> +    return &pluginClient;
> +}

Why is this static?  We should just create one on the stack for each test.

> Source/WebKit/chromium/tests/WebPluginContainerTest.cpp:89
> +    return WebFrameImpl::pluginContainerFromNode(element);

Yeah, moving pluginContainerFromNode to WebElement might be the best thing.

> Source/WebKit/chromium/tests/WebPluginContainerTest.cpp:97
> +    static_cast<WebViewImpl*>(webView)->settings()->setPluginsEnabled(true);

Why WebViewImpl?  You should be able to do all this this through the API.

> Source/WebKit/chromium/tests/WebPluginContainerTest.cpp:102
> +    WebPluginContainerImpl* pluginContainerOne = getWebPluginContainer(webView, WebString::fromUTF8("translated-plugin"));

Why WebPluginContainerImpl?  You should be able to do all of this though the API.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.