[Webkit-unassigned] [Bug 63445] [GTK] Add back/forward menu to MiniBrowser toolbar

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #2 from Martin Robinson <mrobinson at webkit.org>  2011-06-27 09:11:24 PST ---
(From update of attachment 98705)
View in context: https://bugs.webkit.org/attachment.cgi?id=98705&action=review

Looks good, just have a few suggestions.

> Tools/MiniBrowser/gtk/BrowserWindow.c:294
> +    WKURLRef url = WKBackForwardListItemCopyURL(item);
> +    gchar *name = WKURLGetCString(url);
> +    WKRelease(url);
> +
> +    WKStringRef title = WKBackForwardListItemCopyTitle(item);
> +    gchar *label = WKStringGetCString(title);
> +    WKRelease(title);
> +
> +    return gtk_action_new(name, label, 0, 0);

This seems wrong. Either name and label are leaking or you are using them after they are freed.

> Tools/MiniBrowser/gtk/BrowserWindow.c:325
> +        WKBackForwardListItemRef item = WKArrayGetItemAtIndex(list, i);
> +        GtkAction *action = createGtkActionFromBackForwardItem(item);
> +        if (!action)
> +            continue;
> +
> +        g_object_set_data_full(G_OBJECT(action), "back-forward-list-item", (gpointer)WKRetain(item), (GDestroyNotify)WKRelease);
> +        g_signal_connect_swapped(action, "activate", G_CALLBACK(browserWindowHistoryItemActivated), window);
> +
> +        GtkWidget *menuItem = gtk_action_create_menu_item(action);
> +        g_signal_connect_swapped(menuItem, "select", G_CALLBACK(browserWindowHistoryItemSelected), window);
> +        g_object_unref(action);
> +
> +        gtk_menu_shell_prepend(GTK_MENU_SHELL(menu), menuItem);
> +        gtk_widget_show(menuItem);
> +        hasItems = TRUE;

Please split this out into a helper.

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