[Webkit-unassigned] [Bug 50225] [GTK] Use gtk_widget_render_icon() to render media buttons using gtk stock icons

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #75368|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #4 from Martin Robinson <mrobinson at webkit.org>  2010-12-02 05:04:00 PST ---
(From update of attachment 75368)
View in context: https://bugs.webkit.org/attachment.cgi?id=75368&action=review

This patch seems to contain some bits from your other media styling patch. Other than that it looks pretty good. I have a few comments.

> WebCore/platform/gtk/RenderThemeGtk.cpp:72
> +    GdkPixbuf* icon = gtk_widget_render_icon(widget, iconName, iconSize, "button");

Consider using a PlatformRefPtr here.

> WebCore/platform/gtk/RenderThemeGtk.cpp:484
> -    // FIXME: This should not be hard-coded.
> -    IntSize size = IntSize(14, 14);
> -    style->setWidth(Length(size.width(), Fixed));
> -    style->setHeight(Length(size.height(), Fixed));
> +    gint width, height;
> +    gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
> +    style->setWidth(Length(width, Fixed));
> +    style->setHeight(Length(height, Fixed));

Instead of basing this on the size of the icon, it should be based on the height / width of the search box. Otherwise the icon might paint outside the box or be very small in relation to the field size.

> WebCore/platform/gtk/RenderThemeGtk.cpp:504
> +    static Image* searchImage = loadStockIcon(GTK_WIDGET(gtkEntry()), GTK_STOCK_FIND, GTK_ICON_SIZE_MENU).releaseRef();

Now might be a good time to make searchImage a member of RenderThemeGtk so that it can be properly freed.

> WebCore/platform/gtk/RenderThemeGtk.cpp:524
> +    static Image* cancelImage = loadStockIcon(GTK_WIDGET(gtkEntry()), GTK_STOCK_CLEAR, GTK_ICON_SIZE_MENU).releaseRef();

Maybe time to make this a member as well?

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