Bug 87842 - [WK2-Gtk]Compilation errors with flag CONTEXT_MENUS=0
Summary: [WK2-Gtk]Compilation errors with flag CONTEXT_MENUS=0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-30 04:02 PDT by chandra shekar vallala
Modified: 2012-06-06 08:57 PDT (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description chandra shekar vallala 2012-05-30 04:02:45 PDT
Disable CONTEXT_MENUS flag.
Build webkit2.
Comment 1 chandra shekar vallala 2012-05-30 04:35:51 PDT
Created attachment 144779 [details]
fix

patch to fix compilation errors
Comment 2 Martin Robinson 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.
Comment 3 chandra shekar vallala 2012-05-31 23:42:00 PDT
Created attachment 145217 [details]
updated.patch

updated patch
Comment 4 Martin Robinson 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!
Comment 5 chandra shekar vallala 2012-06-04 06:45:35 PDT
Created attachment 145575 [details]
updtaed

addressed review comments.
Comment 6 Martin Robinson 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.
Comment 7 chandra shekar vallala 2012-06-06 05:24:44 PDT
Created attachment 145996 [details]
updated

done.
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2012-06-06 08:57:48 PDT
All reviewed patches have been landed.  Closing bug.