Bug 87842

Summary:

[WK2-Gtk]Compilation errors with flag CONTEXT_MENUS=0

Product:

WebKit

Reporter:

chandra shekar vallala <chandra.vallala>

Component:

WebKit2

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

mrobinson, ravi.kasibhatla, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Linux

Attachments:

Description	Flags
fix	none
updated.patch	mrobinson: review-
updtaed	none
updated	none

chandra shekar vallala

Reported 2012-05-30 04:02:45 PDT

Disable CONTEXT_MENUS flag. Build webkit2.

Attachments
fix (2.87 KB, patch) 2012-05-30 04:35 PDT, chandra shekar vallala	no flags	Details Formatted Diff Diff
updated.patch (2.94 KB, patch) 2012-05-31 23:42 PDT, chandra shekar vallala	mrobinson: review-	Details Formatted Diff Diff
updtaed (2.61 KB, patch) 2012-06-04 06:45 PDT, chandra shekar vallala	no flags	Details Formatted Diff Diff
updated (2.95 KB, patch) 2012-06-06 05:24 PDT, chandra shekar vallala	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

chandra shekar vallala

Comment 1 2012-05-30 04:35:51 PDT

Created attachment 144779 [details] fix patch to fix compilation errors

Martin Robinson

Comment 2 2012-05-30 10:31:25 PDT

Comment on attachment 144779 [details] fix View in context: https://bugs.webkit.org/attachment.cgi?id=144779&action=review Looks good, but I don't think you're a committer, so r- for some minor issues. > Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:30 > + > +#if ENABLE(CONTEXT_MENUS) > + > #include "WebContextMenuProxyGtk.h" See below. > Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebContextMenuClientGtk.cpp:30 > #include "config.h" > + > +#if ENABLE(CONTEXT_MENUS) > + WebContextMenuClient.h already seems to be guarded by ENABLE(CONTEXT_MENUS) so you can move the #ifdef down before the second block.

chandra shekar vallala

Comment 3 2012-05-31 23:42:00 PDT

Created attachment 145217 [details] updated.patch updated patch

Martin Robinson

Comment 4 2012-06-01 09:14:01 PDT

Comment on attachment 145217 [details] updated.patch View in context: https://bugs.webkit.org/attachment.cgi?id=145217&action=review > Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:28 > #include "config.h" > + > #include "WebContextMenuProxyGtk.h" Okay, this a serious nit pick, but we don't typically put a newline here. If you were a committer, I'd just have you fix it and land yourself. I don't believe you are though, so do you mind uploading a new patch? Really appreciate it!

chandra shekar vallala

Comment 5 2012-06-04 06:45:35 PDT

Created attachment 145575 [details] updtaed addressed review comments.

Martin Robinson

Comment 6 2012-06-04 07:47:28 PDT

Comment on attachment 145575 [details] updtaed View in context: https://bugs.webkit.org/attachment.cgi?id=145575&action=review > Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebContextMenuClientGtk.cpp:30 > + > +#if ENABLE(CONTEXT_MENUS) > + My sincere apologies. I just noticed that WebContextMenuClient.h also guards the header with #if ENABLE(CONTEXT_MENUS), so this can down before the second block of headers as well.

chandra shekar vallala

Comment 7 2012-06-06 05:24:44 PDT

Created attachment 145996 [details] updated done.

WebKit Review Bot

Comment 8 2012-06-06 08:57:34 PDT

Comment on attachment 145996 [details] updated Clearing flags on attachment: 145996 Committed r119592: <http://trac.webkit.org/changeset/119592>

WebKit Review Bot

Comment 9 2012-06-06 08:57:48 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.