[webkit-reviews] review denied: [Bug 54531] [GTK] [Windows] Plugins fail to load : [Attachment 148389] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 19 13:38:39 PDT 2012
Martin Robinson <mrobinson at webkit.org> has denied Kalev Lember
<kalevlember at gmail.com>'s request for review:
Bug 54531: [GTK] [Windows] Plugins fail to load
https://bugs.webkit.org/show_bug.cgi?id=54531
Attachment 148389: Patch
https://bugs.webkit.org/attachment.cgi?id=148389&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148389&action=review
Great work! It seems like we can remove the code duplication in GraphicsContext
though.
> Source/WebCore/GNUmakefile.am:979
> +if TARGET_WIN32
> +libWebCore_la_LIBADD = -lversion
> +endif
> +
Probably want to drop a little comment explaining why this is necessary.
> Source/WebCore/GNUmakefile.list.am:4823
> + Source/WebCore/plugins/gtk/PluginPackageGtk.cpp \
> + Source/WebCore/plugins/gtk/PluginViewGtk.cpp \
This looks like it breaks non-X11, non Win32 builds. For instance, DirectFB.
Granted plugins probably do not work there anyway.
> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1134
> +#if (PLATFORM(GTK) && OS(WINDOWS))
> +static void setRGBABitmapAlpha(unsigned char* bytes, size_t length, unsigned
char level)
Why not simply add the GraphicsContextWin.cpp and GraphicsContextCairoWin.cpp
files to the source list instead of duplicating all this code here?
> Source/WebCore/plugins/win/PluginViewWin.cpp:113
> + if (GdkWindow *window = gtk_widget_get_window(client))
Asterisks go next to the variable name in WebKit.
> Source/WebCore/plugins/win/PluginViewWin.cpp:114
> + return (HWND)GDK_WINDOW_HWND(window);
Are you sure you have to cast here? If you do, you should use a C++ style cast
(static_cast).
More information about the webkit-reviews
mailing list