[Webkit-unassigned] [Bug 54827] [GTK] Add context menu support for Webkit2

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #91660|review?                     |review-
               Flag|                            |




--- Comment #8 from Martin Robinson <mrobinson at webkit.org>  2011-05-03 10:53:10 PST ---
(From update of attachment 91660)
View in context: https://bugs.webkit.org/attachment.cgi?id=91660&action=review

Very nice change. We should no longer duplicate code between WebCore/WebKit1 and WebKit2 though. Do you mind finding a way to avoid it. One way is to build a helper class which is used by both WebCore for WebKit1 and WebKit2.

> Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:175
> +        const WebContextMenuItemData& item = items.at(i);
> +        GtkWidget* menuItem;
> +
> +        if (item.type() == WebCore::SeparatorType)
> +            menuItem = gtk_separator_menu_item_new();
> +        else {
> +            GRefPtr<GtkAction> action = getActionForWebContextMenuItem(item);
> +            g_signal_connect(action.get(), "activate", G_CALLBACK(contextMenuItemActivatedCallback), m_page);
> +
> +            menuItem = gtk_action_create_menu_item(action.get());
> +
> +            if (item.type() == WebCore::SubmenuType) {
> +                GtkMenu* subMenu = createGtkMenu(item.submenu());
> +                gtk_menu_item_set_submenu(GTK_MENU_ITEM(menuItem), GTK_WIDGET(subMenu));
> +            }
> +        }
> +
> +        gtk_menu_shell_append(GTK_MENU_SHELL(menu), menuItem);

This could should probably be a helper function like convertContextMenuItemDataToGtkMenuItem. That would allow early returns and make the code a little cleaner, I think.

> Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:187
> +    GtkMenu* menu = createGtkMenu(items);
> +    webkitWebViewBaseShowContextMenu(WEBKIT_WEB_VIEW_BASE(m_webView), menu, position);

No need to cache the menu as a local variable here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.