Need INSPECTOR guard for using ContextMenuItemTagInspectElement.
Created attachment 179930 [details] Patch
Comment on attachment 179930 [details] Patch LGTM.
Comment on attachment 179930 [details] Patch rs=me.
Comment on attachment 179930 [details] Patch Sorry for free cq+. I would like to check if we can use ENABLE_XXX macro in public header a little further.
Comment on attachment 179930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179930&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context_menu_item.h:118 > +#if defined ENABLE_INSPECTOR && ENABLE_INSPECTOR If app want to use this INSPECT_ELEMENT type, app should define ENABLE_INSPECTOR macro, isn't it ? It looks app should do unnecessary job. I would suggest not to use ENABLE_INSPECTOR macro in public header as chromium port. Chromium port only uses macro in internal implementation. Any other comments ?
Comment on attachment 179930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179930&action=review > Source/WebKit/efl/ewk/ewk_contextmenu.h:112 > +#if defined ENABLE_INSPECTOR && ENABLE_INSPECTOR Wouldn't the use of #if ENABLE(INSPECTOR) do the same thing as above? I think its better to use the ENABLE(INSPECTOR) macro for consistency.
(In reply to comment #6) > (From update of attachment 179930 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179930&action=review > > > Source/WebKit/efl/ewk/ewk_contextmenu.h:112 > > +#if defined ENABLE_INSPECTOR && ENABLE_INSPECTOR > > Wouldn't the use of #if ENABLE(INSPECTOR) do the same thing as above? I think its better to use the ENABLE(INSPECTOR) macro for consistency. ENABLE(XXX) is defined to WTF/wtf/Platform.h. So, application which uses ewk_contextmenu.h should include the Platform.h to use ENABLE(XXX) macro. But, we don't install Platform.h file. We can't use ENABLE() in public header. Beside, as we already talked, to use macro in public header will make unneeded overhead when using public header.
Created attachment 180512 [details] Patch
Comment on attachment 180512 [details] Patch No #ifdefs in public header. I have removed them. Thank you for review :)
Comment on attachment 180512 [details] Patch Clearing flags on attachment: 180512 Committed r138370: <http://trac.webkit.org/changeset/138370>
All reviewed patches have been landed. Closing bug.