[webkit-reviews] review denied: [Bug 49658] [GTK] Simplify context-menu handling code : [Attachment 74103] Patch to simplify context-menu handling in gtk port

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 49658: [GTK] Simplify context-menu handling code
https://bugs.webkit.org/show_bug.cgi?id=49658

Attachment 74103: Patch to simplify context-menu handling in gtk port
https://bugs.webkit.org/attachment.cgi?id=74103&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74103&action=review

Great patch! I have two small tweaks to suggest.

> WebCore/platform/ContextMenuItem.h:188
> +    typedef GtkMenuItem* PlatformMenuItemDescription;

I think we should make this a PlatformRefPtr<GtkMenuItem>(...).

> WebCore/platform/gtk/ContextMenuItemGtk.cpp:118
> +    : m_platformDescription(GTK_MENU_ITEM(g_object_ref(item)))

It would remove the need to call g_object_ref here.

> WebCore/platform/gtk/ContextMenuItemGtk.cpp:135
> +    GOwnPtr<char>actionName(g_strdup_printf("context-menu-action-%d",
action));

Missing a space here between <char> and actionName.

> WebCore/platform/gtk/ContextMenuItemGtk.cpp:156
> +    if (m_platformDescription)
> +	   g_object_unref(m_platformDescription);

A smart pointer also removes the need for this code.