GNOME Bugzilla – Bug 632848
Ekiga needs to be adapted to libnotify-0.7.0
Last modified: 2010-11-04 20:39:45 UTC
notify_notification_attach_to_status_icon has been removed notify_notification_new takes only 3 arguments
Could you propose a patch to help us better?
Created attachment 172984 [details] [review] Proposed patch Ooops, actually wanted to post this right away. But somehow it got lost on the way...
Thanks. The problem is that on some of the current systems (debian unstable for ex.) this patch will break compilation, since they still use 0.5.0. Would you mind if you add also a checking of libnotify in configure.ac and use #define-d a compilation to allow both versions of libnotify?
May I also use NOTIFY_CHECK_VERSION(0,7,0). Seems to me to be straight forward...
Created attachment 173001 [details] [review] Using NOTIFY_CHECK_VERSION(0,7,0) I have only tested it with libnotify-0.7.0. Have no older version within reach...
It does not work... It shows a compilation error: CXX libnotify-main.lo ../lib/engine/components/libnotify/libnotify-main.cpp:162:26: error: missing binary operator before token "(" /usr/include/libnotify/notification.h: In member function ‘void LibNotify::on_notification_added(boost::shared_ptr<Ekiga::Notification>)’: /usr/include/libnotify/notification.h:79: error: too few arguments to function ‘NotifyNotification* notify_notification_new(const char*, const char*, const char*, GtkWidget*)’ ../lib/engine/components/libnotify/libnotify-main.cpp:165: error: at this point in file make[3]: *** [libnotify-main.lo] Error 1 This appears because NOTIFY_CHECK_VERSION does not exist in 0.5.0; it was introduced later: http://osdir.com/ml/general/2010-10/msg27687.html Do you have the time to fix the patch? I could do it, but it will take me maybe 30-60 min. Unfortunately, I think the only solution is to get the version from configure.ac and use it in the cpp files, like http://opalvoip.svn.sourceforge.net/viewvc/opalvoip?view=revision&revision=24100 for example (except 'configure' file).
Wait, I have just found a simpler solution, using #ifdef together with #if...
Ok, I made the patch. However, if notify_notification_attach_to_status_icon has been removed, what is it replaced with?
Sorry, wasn't able to work on it druing the weekend... Ability to attach to a status icon is replaced by the use of notification persistence. See .../libnotify-0.7.0/tests/test-persistence.c
Thanks. Should notify_notification_attach_to_status_icon in ekiga just be removed, as in your patch (if yes, why?), or should it be replaced with the use of notification, as you say in the previous comment (if yes, can you help by updating the patch)?
It should just be removed, like the nmapplet did ( https://bugzilla.gnome.org/review?bug=632327&attachment=172525 ). For further detail see: http://live.gnome.org/GnomeShell/Design/Guidelines/MessageTray/Compatibility I also read that you should use symbolic icons. See: http://live.gnome.org/JavierJardon/Gnome3PortGuide Don't know if this applies to ekiga...
There was an error in your patch: - notify = notify_notification_new (title, body, GM_ICON_LOGO, NULL); + notify = notify_notification_new (title, body, GM_ICON_LOGO + #if NOTIFY_CHECK_VERSION(0,7,0) + , NULL + #endif + ); The correct patch is with negation: #if !NOTIFY... Committed: http://git.gnome.org/browse/ekiga/commit/?id=6a964b0889. Thanks for the patch and info.