./Tools/Scripts/build-webkit --efl --cmakeargs="-DSHARED_CORE=ON" --minimal fails with this error: /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp: In member function `void EwkContextMenu::contextMenuItemSelected(const WebKit::WebContextMenuItemData&)': /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:91:23: error: invalid use of incomplete type `class WebKit::WebContextMenuProxyEfl' In file included from /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:30:0: /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/EwkView.h:70:7: error: forward declaration of `class WebKit::WebContextMenuProxyEfl' /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp: In function `Eina_Bool ewk_context_menu_item_select(Ewk_Context_Menu*, Ewk_Context_Menu_Item*)': /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:146:119: error: invalid use of incomplete type `class WebKit::WebContextMenuItemData' In file included from /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:30:0: /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/EwkView.h:69:7: error: forward declaration of `class WebKit::WebContextMenuItemData' In file included from /home/ed/git/wk/WebKit/Source/WTF/wtf/text/CString.h:31:0, from /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/EwkViewCallbacks.h:34, from /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/EwkView.h:26, from /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:30: /home/ed/git/wk/WebKit/Source/WTF/wtf/Vector.h: In instantiation of `const T& WTF::Vector<T, inlineCapacity>::at(size_t) const [with T = WebKit::WebContextMenuItemData; long unsigned int inlineCapacity = 0ul; size_t = long unsigned int]': /home/ed/git/wk/WebKit/Source/WTF/wtf/Vector.h:563:58: required from `const T& WTF::Vector<T, inlineCapacity>::operator[](size_t) const [with T = WebKit::WebContextMenuItemData; long unsigned int inlineCapacity = 0ul; size_t = long unsigned int]' /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:46:104: required from here /home/ed/git/wk/WebKit/Source/WTF/wtf/Vector.h:559:39: error: invalid use of incomplete type `class WebKit::WebContextMenuItemData' In file included from /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:30:0: /home/ed/git/wk/WebKit/Source/WebKit2/UIProcess/API/efl/EwkView.h:69:7: error: forward declaration of `class WebKit::WebContextMenuItemData' [ 92%] Building CXX object Source/WebKit2/CMakeFiles/ewebkit2.dir/UIProcess/API/efl/ewk_favicon_database.cpp.o make[2]: *** [Source/WebKit2/CMakeFiles/ewebkit2.dir/UIProcess/API/efl/ewk_context_menu.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [Source/WebKit2/CMakeFiles/ewebkit2.dir/all] Error 2 make: *** [all] Error 2
Created attachment 196506 [details] Patch
Created attachment 196744 [details] Patch Rebased. Please, review.
*** Bug 114098 has been marked as a duplicate of this bug. ***
Created attachment 196803 [details] Patch
Created attachment 196807 [details] Patch Rebased. Please review.
Comment on attachment 196807 [details] Patch Attachment 196807 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17517137
Comment on attachment 196807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196807&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context_menu.h:34 > +#if ENABLE(CONTEXT_MENUS) Should not use ENABLE() macro in public header.
Created attachment 196875 [details] Patch
Comment on attachment 196875 [details] Patch Attachment 196875 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17586012
Created attachment 196951 [details] Patch
Comment on attachment 196951 [details] Patch Attachment 196951 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17423127
Created attachment 197170 [details] Patch
Comment on attachment 197170 [details] Patch Attachment 197170 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17597017
Comment on attachment 197170 [details] Patch Attachment 197170 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/57050
Created attachment 200027 [details] Patch
Comment on attachment 200027 [details] Patch Attachment 200027 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/229554
Comment on attachment 200027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=200027&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:-166 > -static WKContextMenuItemTag getWKTagFromEwkAction(Ewk_Context_Menu_Item_Action action) Why do you remove this function ? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:117 > +#if ENABLE(CONTEXT_MENUS) Please don't use ENABLE() in public header. > Source/WebKit2/UIProcess/API/efl/ewk_view.h:178 > +#if ENABLE(CONTEXT_MENUS) ditto.
Gyuyoung, how can I distinguish between public and private headers? How can I avoid using use ENABLE(CONTEXT_MENUS) in Source/WebKit2/UIProcess/API/efl/ewk_view.h if m_contextMenu; is defined there?
(In reply to comment #18) > Gyuyoung, how can I distinguish between public and private headers? How can I avoid using use ENABLE(CONTEXT_MENUS) in Source/WebKit2/UIProcess/API/efl/ewk_view.h if m_contextMenu; is defined there? Headers in API/efl/ folder are public unless ending with '_private.h'. There is no "m_contextMenu" in ewk_view.h so I don't understand your question. Gyuyoung missed your ifdef to ewk_context_menu.h public header ;)
I can see a lot of ENABLEs in Source/WebKit2/UIProcess/API/efl/EwkView.h is it also private header?
(In reply to comment #20) > I can see a lot of ENABLEs in Source/WebKit2/UIProcess/API/efl/EwkView.h is it also private header? No it is not. I thought those files were in UIProcess/efl, sorry. Our public headers are written in C and are all lowercase. EwkView is C++ and camel case. Public headers are also listed in Source/WebKit2/PlatformEfl.cmake
Created attachment 201272 [details] Patch
Christophe, Thank you for your explanations!
Comment on attachment 201272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201272&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:28 > +#if ENABLE(CONTEXT_MENUS) This does not seem right. All the public functions in ewk_context_menu.h would be missing their implementation. You probably needs #ifdefs inside the body of each public API function.
Comment on attachment 201272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201272&action=review > Source/WebKit2/ChangeLog:17 > + * UIProcess/CoordinatedGraphics/WebView.cpp: FYI, you're touching some non-EFL WK2 files some you will probably need owner review (Source/WebKit2/Owners) unless Noam can do that now (since this is CoordinatedGraphics).
Comment on attachment 201272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201272&action=review >> Source/WebKit2/ChangeLog:17 >> + * UIProcess/CoordinatedGraphics/WebView.cpp: > > FYI, you're touching some non-EFL WK2 files some you will probably need owner review (Source/WebKit2/Owners) unless Noam can do that now (since this is CoordinatedGraphics). Apple were OK with not needing owners to review CoordintedGraphics. Either way, I'm ok with the changes to WebView. However, here you're touching PageClient.h as well.
Comment on attachment 201272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201272&action=review >>> Source/WebKit2/ChangeLog:17 >>> + * UIProcess/CoordinatedGraphics/WebView.cpp: >> >> FYI, you're touching some non-EFL WK2 files some you will probably need owner review (Source/WebKit2/Owners) unless Noam can do that now (since this is CoordinatedGraphics). > > Apple were OK with not needing owners to review CoordintedGraphics. Either way, I'm ok with the changes to WebView. > However, here you're touching PageClient.h as well. Thanks for clarifying Noam. I missed the PageClient change indeed. Definitely needs Owner review then.
(In reply to comment #19) > (In reply to comment #18) > Gyuyoung missed your ifdef to ewk_context_menu.h public header ;) I said as below, ;-) > Please don't use ENABLE() in public header.
Created attachment 201373 [details] Patch
Updated according to reviewer's comments.
Comment on attachment 201373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201373&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:869 > +#endif Please move this to a line above. > Source/WebKit2/UIProcess/API/efl/ewk_context_menu_private.h:29 > +#if ENABLE(CONTEXT_MENUS) Don't you need to wrap functions defined by this class up with ENABLE(CONTEXT_MENU) as well ? For instance, appendItem(), removeItem().
Closing this bug because the EFL port has been removed from trunk. If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.