Bug 107510

Summary:

[EFL][WK2] Add an API for getting context menu item's parent menu

Product:

WebKit

Reporter:

Michal Pakula vel Rutka <mpakulavelrutka>

Component:

WebKit EFL

Assignee:

Michal Pakula vel Rutka <mpakulavelrutka>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, dglazkov, gyuyoung.kim, lucas.de.marchi, rakuco, rniwa, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Bug Depends on:

109698

Bug Blocks:

102932

Attachments:

Description	Flags
patch	webkit.review.bot: commit-queue-
fixes	buildbot: commit-queue-
fixed patch	none
rebased patch	none
fixes after review	none
rebased patch	benjamin: review-
fixes	none

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-	Details Formatted Diff Diff
fixes (12.31 KB, patch) 2013-01-22 01:20 PST, Michal Pakula vel Rutka	buildbot: commit-queue-	Details Formatted Diff Diff
fixed patch (12.32 KB, patch) 2013-01-25 04:27 PST, Michal Pakula vel Rutka	no flags	Details Formatted Diff Diff
rebased patch (12.30 KB, patch) 2013-02-15 07:17 PST, Michal Pakula vel Rutka	no flags	Details Formatted Diff Diff
fixes after review (11.53 KB, patch) 2013-02-18 05:55 PST, Michal Pakula vel Rutka	no flags	Details Formatted Diff Diff
rebased patch (11.35 KB, patch) 2013-04-15 07:09 PDT, Michal Pakula vel Rutka	benjamin: review-	Details Formatted Diff Diff
fixes (11.24 KB, patch) 2013-04-16 02:45 PDT, Michal Pakula vel Rutka	no flags	Details Formatted Diff Diff
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.