Bug 20412 – webkitview.cpp passes negative-value constants as guint

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Daniel Macks]

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?

[Xan Lopez]

I think you are right, having them as negative numbers does not make sense IMHO.

[Christian Dywan]

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.

[Xan Lopez]

Created attachment 26667 [details]
0001-2009-01-13-Xan-Lopez-xan-gnome.org.patch

commit aeda6d41172041771a67768130db8d38f6ed28f8
Author: Xan Lopez <[e-mail]>
Date:   Tue Jan 13 17:36:57 2009 +0200

    2009-01-13  Xan Lopez  <[e-mail]>

            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(-)

[Holger Freyther]

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?

[Holger Freyther]

Comment on attachment 26667 [details]
0001-2009-01-13-Xan-Lopez-xan-gnome.org.patch

Okay, ABI compat but this was discussed.

[Holger Freyther]

	M	WebKit/gtk/webkit/webkitwebview.h
	M	WebKit/gtk/ChangeLog
r40739 = d2a2bab5d8673ca82cdd570cdb58d24220ae61e8 (git-svn)