| Summary: | Merge sync and async code paths for getting context menus | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||
| Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | berto, cgarcia, commit-queue, ews-watchlist, gustavo, joepeck, mcatanzaro, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | 181564 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Alex Christensen
2018-01-08 18:44:41 PST
Created attachment 330777 [details]
Patch
Could someone double check that context menus on linux work as before? Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API (In reply to Alex Christensen from comment #2) > Could someone double check that context menus on linux work as before? I can look at it tomorrow, sure. Note: lib/libwebkit2gtk-4.0.so.37.26.1: error: undefined reference to 'WebKit::WebContextMenuListenerProxy::useContextMenuItems(WTF::Vector<WTF::Ref<WebKit::WebContextMenuItem, WTF::DumbPtrTraits<WebKit::WebContextMenuItem> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)' lib/libwebkit2gtk-4.0.so.37.26.1: error: undefined reference to 'WebKit::WebContextMenuListenerProxy::WebContextMenuListenerProxy(WebKit::WebContextMenuProxy*)' Created attachment 330794 [details]
Patch
(In reply to Alex Christensen from comment #2) > Could someone double check that context menus on linux work as before? Yes, they're working fine! Comment on attachment 330794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330794&action=review r=me > Source/WebKit/UIProcess/API/glib/WebKitContextMenuClient.cpp:51 > webkitWebViewPopulateContextMenu(m_webView, menuItems, hitTestResultData, variant.get()); > - return true; > + contextMenuListener.useContextMenuItems({ }); So Glib does something special in `webkitWebViewPopulateContextMenu` instead of passing the new items to `contextMenuListener.useContextMenuItems`? I'm surprised this always passes an empty list to useContextMenuItems. Edit: Oh I think I see, gtk/WebContextMenuProxyGtk.cpp calls this with a mostly stub WebContextMenuListenerProxy, since WebContextMenuProxyGtk::showContextMenuWithItems does nothing (and which you just removed). I suppose it could be a real WebContextMenuListenerProxy that does the webkitWebViewPopulateContextMenu call instead of happening here in the client? Seems like it would amount to doing the same thing in the end. Re-opened since this is blocked by bug 181564 This broke API tests WebKit.ContextMenuImgWithVideo _WKDownload.RedirectedDownload Comment on attachment 330794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330794&action=review > Source/WebKit/UIProcess/API/APIContextMenuClient.h:54 > + virtual void getContextMenuFromProposedMenu(WebKit::WebPageProxy&, Vector<Ref<WebKit::WebContextMenuItem>>&& /* proposedMenu */, WebKit::WebContextMenuListenerProxy& listener, const WebKit::WebHitTestResultData&, API::Object* /* userData */) { listener.useContextMenuItems({ }); } This broke the tests. Instead of calling the listener with an empty list, I needed to move proposedMenu into the listener. Oops. Created attachment 331427 [details]
Patch
|