[webkit-reviews] review granted: [Bug 63249] [UNIX] Fix compile warnings in NetscapePluginX11.cpp : [Attachment 98337] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 23 08:10:41 PDT 2011


Martin Robinson <mrobinson at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 63249: [UNIX] Fix compile warnings in NetscapePluginX11.cpp
https://bugs.webkit.org/show_bug.cgi?id=63249

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98337&action=review

Looks good, but I have a few minor requests before landing.

> Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:362
> +    case WebMouseEvent::LeftButton:
> +    default:
> +	   xButton.button = Button1;
> +	   break;

If we are receiving other types of events shouldn't we be passing them properly
to the plugin. I think it's more correct to simply have ASSERT_NOT_REACHED()
for default:. This makes me wonder whether wheel events are supported or not.

> Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:414
> +    default:
> +	   return false;

I would rather you list them explicitly here. Do you mind changing that before
landing?


More information about the webkit-reviews mailing list