[Webkit-unassigned] [Bug 150642] [GTK] Use CROSS_PLATFORM_CONTEXT_MENUS

[bugzilla-daemon at webkit.org]

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

--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #4)
> Comment on attachment 264798 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264798&action=review
> 
> Quite a complex patch... I didn't review the whole thing, just adding a
> couple of comments.
> 
> > Source/WebKit2/Shared/gtk/WebContextMenuItemGtk.cpp:43
> > +        return GTK_STOCK_COPY;
> 
> Since GtkStock has been deprecated for several years now, you might take
> this opportunity to get rid of it.
> 
> GTK_STOCK_COPY would be replaced with "_Copy", for example.
> 
> See:
> https://docs.google.com/spreadsheets/d/1HavJQRPpMuq-N0GoN1wJR-9KEGXpKy3-
> NEPpZZkUGJY/pub?output=html
> 
> I guess that could be a follow-up patch, though.

Our API is based on deprecated API GtkAction, so I don't care if stock actions are deprecated too. I have real bugs and important feature to work on before.

> > Source/WebKit2/Shared/gtk/WebContextMenuItemGtk.cpp:150
> > +void WebContextMenuItemGtk::createGtkActionIfNeeded()
> 
> How hard is it to add support for GAction as an alternative to GtkAction?  I
> wouldn't expect you to do that in this patch, but Arnaud and I are planning
> to drop use of GtkAction in Epiphany, so we need a GAction API here in order
> to keep our custom context menu items.

Last time I checked it was not that easy, it would probably require a glib version bump to ensure it does everything we need, and it affects the public API. So, I don't see the point, GtkAction works very well for us.

> > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestContextMenu.cpp:-96
> > -    bool shouldShowInputMethodsMenu()
> 
> Eeep, this was broken for ages and nobody noticed? Anyway, I agree with
> removing it, but maybe you could do that in a separate patch and backport it
> to 2.10? It sucks to have broken context menu items.

Yes. Nobody noticed it because nobody uses it. It has been disabled by default in GTK+, and we only show it if gtk-show-input-method-menu setting is enabled. So, in real world we are not showing this menu item enymore, because nobody cares to change that (I don't even know how to change that setting). But it has never actually worked, I think. We build the menu from gtk_im_multicontext_append_menuitems (which is deprecated, of course). From that menu we create ContextMenuItems that extract the GtkAction associated with gtk_activatable_get_related_action, but those menu items no longer have a GtkAction associated (and I don't know they ever had it, to be honest). So, we are showing a menu of items that do nothing, because there's no action to connect to emit the activate signal.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151105/52a3c0c7/attachment.html>