ASSIGNED Bug 120609
Web Inspector: Context Menu doesn't work on EFL port
https://bugs.webkit.org/show_bug.cgi?id=120609
Summary Web Inspector: Context Menu doesn't work on EFL port
Andre Loureiro (IRC: loureiro)
Reported 2013-09-02 12:37:47 PDT
Right click on Toolbar doesn't show the context menu allowing the user change among Icons layouts, for example: Icon and Text (Vertical) Icon and Text (Horizontal) Icon Only Text Only Small Icons
Attachments
showing context menu at WebInspector view (197.94 KB, image/png)
2013-10-04 12:13 PDT, Andre Loureiro (IRC: loureiro)
no flags
Patch attached (24.37 KB, patch)
2013-10-09 09:02 PDT, Andre Loureiro (IRC: loureiro)
no flags
Patch (24.71 KB, patch)
2013-10-09 11:52 PDT, Andre Loureiro (IRC: loureiro)
gyuyoung.kim: review-
Radar WebKit Bug Importer
Comment 1 2013-09-02 12:38:29 PDT
Andre Loureiro (IRC: loureiro)
Comment 2 2013-10-04 12:13:53 PDT
Created attachment 213387 [details] showing context menu at WebInspector view Screenshot from a initial implementation of context menu in a WebInspector view
Timothy Hatcher
Comment 3 2013-10-04 22:35:08 PDT
Is there a reason why the Linux ports don't use a native InspectorFrontEndHost like Mac and Windows? That would let you use a real context menu and not need to create it in HTML. It would be better to have a common path and no split the ports.
Andre Loureiro (IRC: loureiro)
Comment 4 2013-10-07 06:09:38 PDT
(In reply to comment #3) > Is there a reason why the Linux ports don't use a native InspectorFrontEndHost like Mac and Windows? That would let you use a real context menu and not need to create it in HTML. It would be better to have a common path and no split the ports. Do you mean there is a way to implement cross platform context menus? If so, Is it a feature of the old inspector that would be keep in the new inspector implementation? besides that, I don't know how it will impact in the EFL port, because it uses "EWK" to implement the UI. I believe someone from EFL port could explain better, @gyuyoung?
Timothy Hatcher
Comment 5 2013-10-07 09:14:28 PDT
The Mac and Windows ports use the native InspectorFrontEndHost, not InspectorFrontEndHostStubs.js. So we use WebCore's ContextMenu code. See: WebCore::InspectorFrontendHost::showContextMenu
Gyuyoung Kim
Comment 6 2013-10-07 18:29:50 PDT
CC'ing Michal, could you take a look this ?
Andre Loureiro (IRC: loureiro)
Comment 7 2013-10-09 09:02:24 PDT
Created attachment 213782 [details] Patch attached
WebKit Commit Bot
Comment 8 2013-10-09 09:05:00 PDT
Attachment 213782 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/PlatformEfl.cmake', u'Source/WebKit2/UIProcess/API/efl/ewk_view.cpp', u'Source/WebKit2/UIProcess/API/efl/ewk_view.h', u'Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp', u'Source/WebKit2/UIProcess/WebInspectorProxy.cpp', u'Source/WebKit2/UIProcess/WebInspectorProxy.h', u'Source/WebKit2/UIProcess/efl/WebInspectorProxyEfl.cpp', u'Tools/ChangeLog', u'Tools/MiniBrowser/efl/main.c']" exit_code: 1 Source/WebKit2/UIProcess/efl/WebInspectorProxyEfl.cpp:49: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/UIProcess/efl/WebInspectorProxyEfl.cpp:95: Use 0 instead of NULL. [readability/null] [5] Source/WebKit2/UIProcess/efl/WebInspectorProxyEfl.cpp:98: Declaration has space between type name and * in Evas_Object *bg [whitespace/declaration] [3] Source/WebKit2/UIProcess/efl/WebInspectorProxyEfl.cpp:106: Declaration has space between type name and * in Evas_Object *layout [whitespace/declaration] [3] Source/WebKit2/PlatformEfl.cmake:250: Alphabetical sorting problem. "${EET_LIBRARIES}" should be before "${EINA_LIBRARIES}". [list/order] [5] Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:477: Declaration has space between type name and * in Evas_Object *ewk_view_inspector_window_get [whitespace/declaration] [3] Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:493: Declaration has space between type name and * in Evas_Object *ewk_view_inspector_view_get [whitespace/declaration] [3] Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:501: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 8 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Andre Loureiro (IRC: loureiro)
Comment 9 2013-10-09 11:52:39 PDT
Created attachment 213796 [details] Patch Fix code style
Gyuyoung Kim
Comment 10 2013-10-09 23:15:47 PDT
Comment on attachment 213796 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213796&action=review > Source/WebKit2/ChangeLog:8 > + Use EFL Elementary Window widget set to create the a new inspector window instead Hmm, EFL community has objected to use Elementary inside WebKit. Because, they have a elm_web as a sub module of elementary. So, if we use Elementary inside WebKit, circular dependency occurs. Ryuan, how do you think about this ?
Ryuan Choi
Comment 11 2013-10-09 23:25:15 PDT
(In reply to comment #10) > (From update of attachment 213796 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213796&action=review > > > Source/WebKit2/ChangeLog:8 > > + Use EFL Elementary Window widget set to create the a new inspector window instead > > Hmm, EFL community has objected to use Elementary inside WebKit. Because, they have a elm_web as a sub module of elementary. So, if we use Elementary inside WebKit, circular dependency occurs. Ryuan, how do you think about this ? Yes, this is not acceptable.
Daniel Juyung Seo
Comment 12 2013-10-10 00:02:21 PDT
(In reply to comment #11) > (In reply to comment #10) > > (From update of attachment 213796 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=213796&action=review > > > > > Source/WebKit2/ChangeLog:8 > > > + Use EFL Elementary Window widget set to create the a new inspector window instead > > > > Hmm, EFL community has objected to use Elementary inside WebKit. Because, they have a elm_web as a sub module of elementary. So, if we use Elementary inside WebKit, circular dependency occurs. Ryuan, how do you think about this ? > > Yes, this is not acceptable. Hello, this is Daniel Juyung Seo from EFL team. Sorry but as Elementary depends on WebKit, WebKit can not depend on Elementary. That will lead to a circular dependency. "elm_web" widget of Elementary uses ewk_view internally. I know it will be good if elm_web is separated from Elementary and WebKit uses Elementary. But this is not what it is at the moment. That can happen in EFL 2.0 but there is no plan for EFL 2.0 yet. Thanks for understanding.
Andre Loureiro (IRC: loureiro)
Comment 13 2013-10-10 06:55:49 PDT
> Hello, this is Daniel Juyung Seo from EFL team. > Sorry but as Elementary depends on WebKit, WebKit can not depend on Elementary. > That will lead to a circular dependency. "elm_web" widget of Elementary uses ewk_view internally. > > I know it will be good if elm_web is separated from Elementary and WebKit uses Elementary. But this is not what it is at the moment. > > That can happen in EFL 2.0 but there is no plan for EFL 2.0 yet. > > Thanks for understanding. Ok, I got it, so what you suggest to implement it? Anyway thanks for you reply.
Gyuyoung Kim
Comment 14 2013-10-10 22:55:08 PDT
(In reply to comment #13) > Ok, I got it, so what you suggest to implement it? We can use element in MiniBrowser. So, we have delegate the elementary use to application side(e.g. MiniBrowser). Isn't there method to delegate the elementary use to MiniBrowser ? For instance, you may add new APIs for it.
Gyuyoung Kim
Comment 15 2013-10-10 22:55:51 PDT
(In reply to comment #14) > (In reply to comment #13) > > Ok, I got it, so what you suggest to implement it? > > We can use element in MiniBrowser. So, we have delegate the elementary use to application side(e.g. MiniBrowser). Isn't there method to delegate the elementary use to MiniBrowser ? For instance, you may add new APIs for it. We can use element in MiniBrowser. *element* -> *elementary*
Gyuyoung Kim
Comment 16 2013-10-10 22:58:45 PDT
Comment on attachment 213796 [details] Patch Set r- because of review comments.
Joseph Pecoraro
Comment 17 2013-10-14 11:51:56 PDT
(In reply to comment #3) > Is there a reason why the Linux ports don't use a native InspectorFrontEndHost like Mac and Windows? That would let you use a real context menu and not need to create it in HTML. It would be better to have a common path and no split the ports. If EFL has remote inspection via a WebSocket connection then there will need to be an HTML context menu. But for now it sounds like this is not regarding remote inspection and it sounds like EFL is working toward a good solution.
Note You need to log in before you can comment on or make changes to this bug.