752037 - Do not use deprecated notify_notification_set_icon_from_pixbuf()

Status: NEW

(Core :: Widget: Gtk, defect, P4)

Description

[Andre Klapper]

./toolkit/system/gnome/nsAlertsIconListener.cpp:    notify_notification_set_icon_from_pixbuf(mNotification, aPixbuf);

http://developer.gnome.org/libnotify/0.7/NotifyNotification.html#notify-notification-set-icon-from-pixbuf states that this is deprecated and that http://developer.gnome.org/libnotify/0.7/NotifyNotification.html#notify-notification-set-image-from-pixbuf should be used instead.

Comment 1

[Karl Tomlinson (:karlt)]

Any idea what the difference between the methods is?
ISTR an ownership issue with an GdkPixbuf method.

See also bug 750141.

Comment 2

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (:karlt) from comment #1)
> ISTR an ownership issue with an GdkPixbuf method.

That was notify_notification_set_image_from_pixbuf in https://bugzilla.gnome.org/show_bug.cgi?id=633829

I don't know if or in which stable versions that bug existed.

Comment 3

[Karl Tomlinson (:karlt)]

Looks like the missing ref on the pixbuf was introduced in http://git.gnome.org/browse/libnotify/commit/libnotify/notification.c?id=f63e8ab8b192b291b81148fac53947cdea5841f1 for 0.6.1 and fixed in http://git.gnome.org/browse/libnotify/commit/libnotify/notification.c?id=49766bca28bb6fc57689f6061669bb44362d694d for 0.7.1.

But that's independent of this bug anyway because notify_notification_set_icon_from_pixbuf was essentially just an alias for notify_notification_set_image_from_pixbuf anyway.

It seems there are no differences in the methods, and the names can be switched when we know there are no libnotify libraries in use without the new function name.

Comment 4

[Hubert Figuiere [:hub]]

Attachment: Proposed patch

We require 0.8 for the lib. we should be perfectly fine now.

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 8621932 [details] [diff] [review]
Proposed patch

(In reply to Hubert Figuiere [:hub] from comment #4)
> We require 0.8 for the lib. we should be perfectly fine now.

Where do we require this?

The latest tag I see on https://git.gnome.org/browse/libnotify is 0.7.6.

Comment 6

[Hubert Figuiere [:hub]]

(In reply to Karl Tomlinson (ni?:karlt) from comment #5)
> Comment on attachment 8621932 [details] [diff] [review]
> Proposed patch
> 
> (In reply to Hubert Figuiere [:hub] from comment #4)
> > We require 0.8 for the lib. we should be perfectly fine now.
> 
> Where do we require this?
> 
> The latest tag I see on https://git.gnome.org/browse/libnotify is 0.7.6.

Uh oh. I thought we did in configure.in but it is something else. My bad.