Bug 20412

Summary: webkitview.cpp passes negative-value constants as guint
Product: WebKit Reporter: Daniel Macks <dmacks>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
0001-2009-01-13-Xan-Lopez-xan-gnome.org.patch zecke: review+

Description Daniel Macks 2008-08-16 15:07:06 PDT
Compiling webkit source (nightly r35788) on OS X 10.4, using all gtk and other
non-apple-supplied support libs using latest available from fink unstable
tree...

1. ./autogen.sh --prefix=/sw --with-target=x11 --with-http-backend=curl
--with-font-backend-pango --disable-video

no configure problems

2. make

WebKit/gtk/webkit/webkitwebview.cpp: In function 'void webkit_web_view_init(WebKitWebView*)':
WebKit/gtk/webkit/webkitwebview.cpp:1431: warning: passing negative value 'WEBKIT_WEB_VIEW_TARGET_INFO_HTML' for argument 4 to 'void gtk_target_list_add(GtkTargetList*, _GdkAtom*, guint, guint)'
WebKit/gtk/webkit/webkitwebview.cpp:1432: warning: passing negative value 'WEBKIT_WEB_VIEW_TARGET_INFO_TEXT' for argument 2 to 'void gtk_target_list_add_text_targets(GtkTargetList*, guint)'
WebKit/gtk/webkit/webkitwebview.cpp:1436: warning: passing negative value 'WEBKIT_WEB_VIEW_TARGET_INFO_HTML' for argument 4 to 'void gtk_target_list_add(GtkTargetList*, _GdkAtom*, guint, guint)'
WebKit/gtk/webkit/webkitwebview.cpp:1437: warning: passing negative value 'WEBKIT_WEB_VIEW_TARGET_INFO_TEXT' for argument 2 to 'void gtk_target_list_add_text_targets(GtkTargetList*, guint)'

Sure enough, webkitwebview.h defines those constants as negative-values:

typedef enum
{
    WEBKIT_WEB_VIEW_TARGET_INFO_HTML = - 1,
    WEBKIT_WEB_VIEW_TARGET_INFO_TEXT = - 2
} WebKitWebViewTargetInfo;

I don't know how the gtk_lists themselves are used in gtk but the functions are prototyped as guint in gtkselection.h (from gtk-2.12.11). The only other place this value (appears to my casual reading) to be used is in WebCore/platform/gtk/PasteboardGtk.cpp, where the stored token is casted to a gint.

To a first guess, could just flip those enums to positive values?
Comment 1 Xan Lopez 2009-01-13 07:27:14 PST
I think you are right, having them as negative numbers does not make sense IMHO.
Comment 2 Christian Dywan 2009-01-13 07:34:58 PST
Doesn't make much sense to me either to have negatives here. Applications shouldn't even use the actual values, the enumeration should just be fixed.
Comment 3 Xan Lopez 2009-01-13 07:38:25 PST
Created attachment 26667 [details]
0001-2009-01-13-Xan-Lopez-xan-gnome.org.patch

commit aeda6d41172041771a67768130db8d38f6ed28f8
Author: Xan Lopez <xan@gnome.org>
Date:   Tue Jan 13 17:36:57 2009 +0200

    2009-01-13  Xan Lopez  <xan@gnome.org>

            Reviewed by NOBODY (OOPS!).

            Reported by Daniel Macks.

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

            Use positive numbers for the target info IDs, gtk_target_list_add
            casts them to 'guint'. Also just start them from 0, since the
            values are not relevant or magic in any way, they are just used as
            tokens for the user of the API.

            * webkit/webkitwebview.h:

 WebKit/gtk/ChangeLog              |   15 +++++++++++++++
 WebKit/gtk/webkit/webkitwebview.h |    4 ++--
 2 files changed, 17 insertions(+), 2 deletions(-)
Comment 4 Holger Freyther 2009-01-16 02:06:35 PST
This will create a weird compability problem?! Changing the enum values means old code will pass -1 and -2 and new code 0 and 1. Should we consider that? Do we care?
Comment 5 Holger Freyther 2009-02-02 06:49:24 PST
Comment on attachment 26667 [details]
0001-2009-01-13-Xan-Lopez-xan-gnome.org.patch

Okay, ABI compat but this was discussed.
Comment 6 Holger Freyther 2009-02-06 16:11:36 PST
	M	WebKit/gtk/webkit/webkitwebview.h
	M	WebKit/gtk/ChangeLog
r40739 = d2a2bab5d8673ca82cdd570cdb58d24220ae61e8 (git-svn)