Summary: | [EFL] Implement WebContextMenuProxyEfl::showContextMenu after r191194 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hunseop Jeong <hs85.jeong> | ||||||
Component: | WebKit EFL | Assignee: | Hunseop Jeong <hs85.jeong> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, gyuyoung.kim, lucas.de.marchi, ossy | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 150240 | ||||||||
Bug Blocks: | 150311 | ||||||||
Attachments: |
|
Description
Hunseop Jeong
2015-10-18 21:09:19 PDT
Created attachment 263439 [details]
Patch
Comment on attachment 263439 [details]
Patch
Can't we test the showContextMenu() using test_ewk2_context_menu.cpp ?
Yes, we can't test using the the showContextMenu() in test_ewk2_context_menu.cpp because we don't have the implementation of the showContextMenu() in WebContexMenuProxyEfl.cpp. After r191194, showContextMenu() generic implementation codes was moved to MacPort. So we need the implementation code. Now contextMenu didn't work correctly in the MiniBrowser and API tests. (In reply to comment #3) > Yes, we can't test using the the showContextMenu() in > test_ewk2_context_menu.cpp because we don't have the implementation of the > showContextMenu() in WebContexMenuProxyEfl.cpp. After r191194, > showContextMenu() generic implementation codes was moved to MacPort. > So we need the implementation code. > > Now contextMenu didn't work correctly in the MiniBrowser and API tests. If so, please file a new bug for EFL context menu. (e.g. [EFL] Context menu doesn't work on MiniBrowser.) Then link this bug to the new bug. (In reply to comment #4) > (In reply to comment #3) > > Yes, we can't test using the the showContextMenu() in > > test_ewk2_context_menu.cpp because we don't have the implementation of the > > showContextMenu() in WebContexMenuProxyEfl.cpp. After r191194, > > showContextMenu() generic implementation codes was moved to MacPort. > > So we need the implementation code. > > > > Now contextMenu didn't work correctly in the MiniBrowser and API tests. > > If so, please file a new bug for EFL context menu. (e.g. [EFL] Context menu > doesn't work on MiniBrowser.) Then link this bug to the new bug. Okay, I will make the new bug followed your comment. Comment on attachment 263439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263439&action=review > Source/WebKit2/UIProcess/efl/WebContextMenuProxyEfl.cpp:78 > + if (!m_ewkView) How can we call m_ewkView->showContextMenu() when m_ewkView is null ? Comment on attachment 263439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263439&action=review > Source/WebKit2/UIProcess/efl/WebContextMenuProxyEfl.cpp:45 > + , m_page(&page) We don't need to manage m_page with pointer. Just use a reference. Created attachment 263441 [details]
Patch
(In reply to comment #7) > Comment on attachment 263439 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263439&action=review > > > Source/WebKit2/UIProcess/efl/WebContextMenuProxyEfl.cpp:45 > > + , m_page(&page) > > We don't need to manage m_page with pointer. Just use a reference. I changed the m_page to use the reference. (In reply to comment #6) > Comment on attachment 263439 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263439&action=review > > > Source/WebKit2/UIProcess/efl/WebContextMenuProxyEfl.cpp:78 > > + if (!m_ewkView) > > How can we call m_ewkView->showContextMenu() when m_ewkView is null ? Oops.... I want to check the m_ewkView whether is null. Comment on attachment 263441 [details] Patch Clearing flags on attachment: 263441 Committed r191282: <http://trac.webkit.org/changeset/191282> All reviewed patches have been landed. Closing bug. |