[webkit-reviews] review denied: [Bug 15669] Build with -DXP_UNIX and -lXt for GTK+/X11 port : [Attachment 17561] Updated patch to use x11 instead of !mac:!win32-*:!directfb

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 28 09:07:38 PST 2007


Alp Toker <alp at atoker.com> has denied Rodney Dawes <dobey at wayofthemonkey.com>'s
request for review:
Bug 15669: Build with -DXP_UNIX and -lXt for GTK+/X11 port
http://bugs.webkit.org/show_bug.cgi?id=15669

Attachment 17561: Updated patch to use x11 instead of !mac:!win32-*:!directfb
http://bugs.webkit.org/attachment.cgi?id=17561&action=edit

------- Additional Comments from Alp Toker <alp at atoker.com>
>Index: JavaScriptCore/bindings/npapi.h
>===================================================================
>--- JavaScriptCore/bindings/npapi.h	(revision 27989)
>+++ JavaScriptCore/bindings/npapi.h	(working copy)
>@@ -89,6 +89,7 @@
> #ifdef XP_UNIX
>     #include <X11/Xlib.h>
>     #include <X11/Xutil.h>
>+    #include <stdio.h>
> #endif

Can you explain what this change is for in the ChangeLog entry?

>Index: WebCore/WebCore.pro
>===================================================================
>--- WebCore/WebCore.pro	(revision 27989)
>+++ WebCore/WebCore.pro	(working copy)
>@@ -130,6 +130,11 @@ DEPENDPATH += editing/qt history/qt load
> }
> 
> gtk-port {
>+x11 {
>+    DEFINES += XP_UNIX
>+    LIBS += -lXt
>+}
>+

I'm still not convinced that pulling in a dependency on another toolkit makes
much sense for the GTK+ port on X11, though this can definitely be an optional
switch for users to enable if they want the functionality.

>Index: JavaScriptCore/bindings/npruntime_proxy.h
>===================================================================
>--- JavaScriptCore/bindings/npruntime_proxy.h	(revision 0)
>+++ JavaScriptCore/bindings/npruntime_proxy.h	(revision 0)
>@@ -0,0 +1,26 @@
>+/*
>+ * Copyright (C) 2007 Collabora, Ltd.  All rights reserved.
>+ * 
>+ */

All rights reserved and no license?


More information about the webkit-reviews mailing list