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

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Michal Pakula vel Rutka]

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.

[Michal Pakula vel Rutka]

Created attachment 183907 [details]
patch

[WebKit Review Bot]

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]

Created attachment 183920 [details]
fixes

removed const from parentMenu() in ewk_context_menu_item_private.h

[Build Bot]

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]

Created attachment 184723 [details]
fixed patch

added guard in ewk_context_menu.cpp

[Michal Pakula vel Rutka]

Created attachment 188564 [details]
rebased patch

[Mikhail Pozdnyakov]

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 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]

Created attachment 188865 [details]
fixes after review

fixes after Mikhail's review

[Michal Pakula vel Rutka]

Created attachment 198123 [details]
rebased patch

[Mikhail Pozdnyakov]

Comment on attachment 198123 [details]
rebased patch

lgtm

[Benjamin Poulain]

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]

Created attachment 198296 [details]
fixes

[WebKit Commit Bot]

Comment on attachment 198296 [details]
fixes

Clearing flags on attachment: 198296

Committed r148514: <http://trac.webkit.org/changeset/148514>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.