RESOLVED FIXED 107510
[EFL][WK2] Add an API for getting context menu item's parent menu 
https://bugs.webkit.org/show_bug.cgi?id=107510
Summary [EFL][WK2] Add an API for getting context menu item's parent menu
Michal Pakula vel Rutka
Reported 2013-01-21 23:46:26 PST
Elementary context popup after selecting an item, fires a callback which returns data set by user while creating an context popup item. Ewk context menu API when selecting an item, needs two pointers to be passed: a pointer to a menu and to an item. I have added an API to receive Ewk_Context_Menu_Item's parent menu, so only Ewk_Context_Menu_Item have to be passed to a callback fired when context popup item was selected.
Attachments
patch (12.28 KB, patch)
2013-01-22 00:11 PST, Michal Pakula vel Rutka 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
fixes (12.31 KB, patch)
2013-01-22 01:20 PST, Michal Pakula vel Rutka 		buildbot: commit-queue-
DetailsFormatted DiffDiff
fixed patch (12.32 KB, patch)
2013-01-25 04:27 PST, Michal Pakula vel Rutka 		no flags
DetailsFormatted DiffDiff
rebased patch (12.30 KB, patch)
2013-02-15 07:17 PST, Michal Pakula vel Rutka 		no flags
DetailsFormatted DiffDiff
fixes after review (11.53 KB, patch)
2013-02-18 05:55 PST, Michal Pakula vel Rutka 		no flags
DetailsFormatted DiffDiff
rebased patch (11.35 KB, patch)
2013-04-15 07:09 PDT, Michal Pakula vel Rutka 		benjamin: review-
DetailsFormatted DiffDiff
fixes (11.24 KB, patch)
2013-04-16 02:45 PDT, Michal Pakula vel Rutka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Michal Pakula vel Rutka
Comment 1 2013-01-22 00:11:59 PST
Created attachment 183907 [details] patch
WebKit Review Bot
Comment 2 2013-01-22 01:08:27 PST
Comment on attachment 183907 [details] patch Attachment 183907 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16037465 New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Michal Pakula vel Rutka
Comment 3 2013-01-22 01:20:40 PST
Created attachment 183920 [details] fixes removed const from parentMenu() in ewk_context_menu_item_private.h
Build Bot
Comment 4 2013-01-22 05:25:50 PST
Comment on attachment 183920 [details] fixes Attachment 183920 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16035583 New failing tests: svg/as-image/img-relative-height.html
Michal Pakula vel Rutka
Comment 5 2013-01-25 04:27:41 PST
Created attachment 184723 [details] fixed patch added guard in ewk_context_menu.cpp
Michal Pakula vel Rutka
Comment 6 2013-02-15 07:17:27 PST
Created attachment 188564 [details] rebased patch
Mikhail Pozdnyakov
Comment 7 2013-02-15 07:30:12 PST
Comment on attachment 188564 [details] rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=188564&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:66 > + m_contextMenuItems = eina_list_append(m_contextMenuItems, item); if (EwkContextMenuItem* item = static_cast<EwkContextMenuItem*>(data)) { ... } and "if (!data) continue;" could be skipped. > Source/WebKit2/UIProcess/API/efl/ewk_context_menu_item_private.h:41 > + static PassOwnPtr<EwkContextMenuItem> create(WKContextMenuItemRef item, EwkContextMenu* parentMenu) if parentMenu can be null maybe it's worth making default value for it?
Michal Pakula vel Rutka
Comment 8 2013-02-18 04:18:14 PST
Comment on attachment 188564 [details] rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=188564&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:66 >> + m_contextMenuItems = eina_list_append(m_contextMenuItems, item); > > if (EwkContextMenuItem* item = static_cast<EwkContextMenuItem*>(data)) { > ... > } and "if (!data) continue;" could be skipped. OK >> Source/WebKit2/UIProcess/API/efl/ewk_context_menu_item_private.h:41 >> + static PassOwnPtr<EwkContextMenuItem> create(WKContextMenuItemRef item, EwkContextMenu* parentMenu) > > if parentMenu can be null maybe it's worth making default value for it? OK, I will add default NULL value
Michal Pakula vel Rutka
Comment 9 2013-02-18 05:55:40 PST
Created attachment 188865 [details] fixes after review fixes after Mikhail's review
Michal Pakula vel Rutka
Comment 10 2013-04-15 07:09:46 PDT
Created attachment 198123 [details] rebased patch
Mikhail Pozdnyakov
Comment 11 2013-04-15 07:33:29 PDT
Comment on attachment 198123 [details] rebased patch lgtm
Benjamin Poulain
Comment 12 2013-04-15 13:55:35 PDT
Comment on attachment 198123 [details] rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=198123&action=review Signed off by me for WebKit2 if fixed. The patch needs a review from a EFL reviewer. For all the comments in test_ewk2_context_menu.cpp: your comment should not explain the next line. Instead, it should say _why_ that particular value is expected. > Source/WebKit2/UIProcess/API/efl/ewk_context_menu_item_private.h:66 > + EwkContextMenu* parentMenu() { return m_parentMenu; } const > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context_menu.cpp:49 > + // Check if parent menu equals when context menu items where created by WebKit. where -> were > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context_menu.cpp:73 > + // Check if parent menu equals when context menu items where created using ewk_context_menu_item_new and added using ewk_context_menu_item_append. ? "Check if parent menu equals when" ? > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context_menu.cpp:85 > + // Check if parent menu equals when context menu items where created using ewk_context_menu_new_with_items Ditto. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context_menu.cpp:93 > + // Check if parent menu equals when context menu items where created using ewk_context_menu_item_new_with_submenu and added using ewk_context_menu_item_append. Ditto.
Michal Pakula vel Rutka
Comment 13 2013-04-16 02:45:30 PDT
Created attachment 198296 [details] fixes
WebKit Commit Bot
Comment 14 2013-04-16 08:33:13 PDT
Comment on attachment 198296 [details] fixes Clearing flags on attachment: 198296 Committed r148514: <http://trac.webkit.org/changeset/148514>
WebKit Commit Bot
Comment 15 2013-04-16 08:35:53 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.