[webkit-reviews] review denied: [Bug 47087] Use pixbufs instead of pixmaps when creating platform cursors : [Attachment 69922] Updated patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Carlos Garcia Campos
<carlosgc at gnome.org>'s request for review:
Bug 47087: Use pixbufs instead of pixmaps when creating platform cursors
https://bugs.webkit.org/show_bug.cgi?id=47087

Attachment 69922: Updated patch
https://bugs.webkit.org/attachment.cgi?id=69922&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=69922&action=review

> WebCore/ChangeLog:6
> +

Usually the 'syntax' for this is, title in one line, URL in the next line. We
also usually use [GTK] in the title when the bug is GTK specific.

> WebCore/ChangeLog:13
> +

So... like this, but it goes at the beginning.

> WebCore/ChangeLog:15
> +

If you don't add tests you have to remove this.

> WebCore/platform/gtk/CursorGtk.cpp:51
> +    cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_A1,
32, 32);

Should use PlatformRefPtrCairo here, see
WebCore/platform/graphics/cairo/PlatformRefPtrCairo.{cpp,h}

> WebCore/platform/gtk/CursorGtk.cpp:52
> +    cairo_t* cr = cairo_create(surface);

Same thing.

> WebCore/platform/gtk/CursorGtk.cpp:60
> +    GdkPixbuf* pixbuf = gdk_pixbuf_get_from_surface(surface, 0, 0, 32, 32);

Use a GRefPtr here.

> WebCore/platform/gtk/GtkVersioning.h:97
> +

Group this with the already existing #ifdef GTK_API_VERSION_2 thing?