[Webkit-unassigned] [Bug 159631] [GTK] Add webkit_context_menu_item_new_from_gaction

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159631

--- Comment #20 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #19)
> Comment on attachment 310648 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310648&action=review
> 
> Nice! Reasoning about actions is tricky.

And it was so easy with GtkAction...

> We really ought to deprecate webkit_context_menu_item_new() and
> webkit_context_menu_item_get_action() in this patch. Deprecations are an
> optional service to let developers know that the function is going away
> eventually and ease porting to our future GTK+ API. It doesn't mean
> developers have to immediately stop using them; of course we'll keep them
> working in our GTK+ 3 API forever. But we are for sure that the GtkAction
> version of this function is going to be gone in our GTK+ 4 API, so best let
> developers know that now. r=me if you add deprecations for these two
> functions (both header file attributes and introspection annotations).

Ok, I'll deprecate it.

> > Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:84
> > +        if (menuItem.action() < ContextMenuItemBaseApplicationTag) {
> 
> Why did you have to add this conditional?

Because glib checks if there are handlers connected to activate signal, and we don't want that glib thinks we are going to handle the activate, because we are not, user is expected to do it (or glib when no handlers are connected). Note that WebPageProxy::contextMenuItemSelected() returns early with the same condition, notifying the client before, but we don't implement customContextMenuItemSelected(), because users can connect to activate of the signal themselves.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170522/995fd417/attachment.html>