Ubuntu

Notifications use append instead of replace

Reported by Ken VanDine on 2009-11-06
36
This bug affects 8 people
Affects Status Importance Assigned to Milestone
Empathy
Fix Released
Wishlist
One Hundred Papercuts
Low
Ken VanDine
empathy (Ubuntu)
Low
Ken VanDine

Bug Description

Binary package hint: empathy

notify-osd notifications are using replace instead of append as described in the specification https://wiki.ubuntu.com/NotifyOSD

Changed in empathy (Ubuntu):
assignee: nobody → Ken VanDine (ken-vandine)
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package empathy - 2.29.3-1ubuntu1

---------------
empathy (2.29.3-1ubuntu1) lucid; urgency=low

  * debian/patches/20_libindicate.patch:
    - Updated to apply cleanly to 2.29.3
  * debian/patches/31_really_raise_window.patch
    - Force focus of the window when selected from the indicator (LP: #442389)
  * debian/patches/32_append_notifications.patch
    - Set append hint to notifications (LP: #476662)
 -- Ken VanDine <email address hidden> Mon, 07 Dec 2009 09:48:06 -0500

Changed in empathy (Ubuntu):
status: New → Fix Released
Ken VanDine (ken-vandine) wrote :
summary: - Notifications use replace instead of append
+ Notifications use append instead of replace
Conscious User (conscioususer) wrote :

I would like to reopen this bug for Lucid Beta 1. Empathy notifications from the same user are not merging.

Vish (vish) wrote :

Adding papercut task and milestones from dup.

Changed in hundredpapercuts:
importance: Undecided → Low
milestone: none → lucid-round-4
status: New → Fix Released
Vish (vish) wrote :

@Conscious User : when you are duping a bug , you need to carry over the other tasks as-well. Thanks for duping it though

Nicolò Chieffo (yelo3) wrote :

The patch 32_append_notifications.patch does not fix the problem
I'll attach a debdiff that fixes it

Changed in empathy (Ubuntu):
status: Fix Released → Confirmed
Ken VanDine (ken-vandine) wrote :

Thanks for the patch!

Two things:

1) We do need the same logic applied to empathy-status-icon.c, without it we don't get notifications when the chat window isn't open. Can you apply the same logic there?
2) It works well for IMs from one person, but if you have incoming IMs within the timeout period one of the notifications seems to get discarded. So it appends the notifications for the existing notification nicely, but new notifications from other people seem to not appear at all, even after the existing one expires.

tags: added: patch

Hi, I'm glad you already reviewed the patch!

> 1) We do need the same logic applied to empathy-status-icon.c, without it we don't get notifications when the chat window isn't open.  Can you apply the same logic there?

I'll try to do this

> 2) It works well for IMs from one person, but if you have incoming IMs within the timeout period one of the notifications seems to get discarded.  So it appends the notifications for the existing notification nicely, but new notifications from other people seem to not appear at all, even after the existing one expires.

This is strange, the only thing I can say is that maybe the mutex
gives problems.
Let's first add the same logic to empathy-status-icon.c and see what happens.

Nicolò Chieffo (yelo3) wrote :

Well the problem could be caused by calling g_object_unref too early...
I'll also try to postpone the call

Nicolò Chieffo (yelo3) wrote :

I also have a question: do I need to g_object_unref a notification
that has been shown, or is it automatic?
I think I have to unref it, so the "closed" event is raised. I
couldn't find this in the notify_notification doc.

Nicolò Chieffo (yelo3) wrote :

New patch, I changed also empathy-status-icon.c in the same way.

To try to fix the second problem I removed the g_object_unref before creating a new notification. I think it should only be called in the closed callback.

Please test it again

Nicolò Chieffo (yelo3) wrote :

After talking with upstream the mutex is not needed because there is only 1 thread.

Ken VanDine (ken-vandine) wrote :

Much better, and it no longer eats notifications for the second contact when the chat window is open. However it is still eating them when the chat window is not open.

Nicolò Chieffo (yelo3) wrote :

Are you sure it works as you expect, without the patch?
I never close the notifications so I can't understand why they are
eaten. It might be an already existing problem...

Nicolò Chieffo (yelo3) wrote :

Hi again, this is another version of the patch which addresses all comments made upstream.
Since it was done with upstream it doesn't apply to ubuntu, because it conflicts with another patch, but I can't undersstand which one.

Vish (vish) on 2010-03-27
Changed in empathy (Ubuntu):
importance: Undecided → Low
status: Confirmed → Triaged
Changed in hundredpapercuts:
status: Fix Released → Triaged
Nicolò Chieffo (yelo3) wrote :

Hi back again, I did some further tests about the missing notifications.

Ken VanDine, I've tested the problem you were reporting using a vanilla version of empathy git (without my patches). It's a normal behavior of empathy: the missing notifications will appear as soon as you click on empathy status icon. Since we have no status icon in ubuntu, those notifications are eaten.

Conscious User (conscioususer) wrote :

Nicolò, the Ubuntu version of Empathy shows the old status icon if the option "use message indicators" is unchecked. Does everything work properly when it is unchecked?

Maybe it could be possible to "eat" the notifications only when this option is unchecked, but I don't know how hard would this be to implement.

Nicolò Chieffo (yelo3) wrote :

Sorry, I don't know how to fix this, eating the messages is not a
problem introduced by my patch.
Can someone file a bug upstream?

Conscious User (conscioususer) wrote :

But I think this bug is only downstream, as this eating does not occur with a vanilla Empathy. The solution probably relies on fixing the Ubuntu patch that adds support for the Messaging Menu and disables the status icon.

Nicolò Chieffo (yelo3) wrote :

No, the problem is also downstream, since the notifications are eaten
until you click on the status icon. This is completely a bug in my
opinion: all the notifications should appear always.

Nicolò Chieffo (yelo3) wrote :

I talked with upstream. They don't want to remove this behavior until
they have a way to selectively open chat windows (ubuntu has this).
The code logic is in empathy-status-icon.c

Nicolò Chieffo (yelo3) wrote :

This patch should disable notifications eating (only).
Ken, you should if it works for you

Nicolò Chieffo (yelo3) wrote :

Ken, you should TRY if it works for you. (I forgot a word, sorry)

Nicolò Chieffo (yelo3) wrote :

To tell the truth patch 35_check_actions_notifications.patch already
does a similar thing, so it's strange that notifications are eaten (anyway I removed the previous patch since it was useless)...
Anyway this is the new version of my patch, which applies to ubuntu
modifications.

Nicolò Chieffo (yelo3) wrote :

I did a mess, this should be the correct patch

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package empathy - 2.30.0-0ubuntu3

---------------
empathy (2.30.0-0ubuntu3) lucid; urgency=low

  * debian/patches/32_append_notifications.patch
    - Fix appending/merging of notifications (Nicolò Chieffo) (LP: #476662)
  * debian/patches/35_check_actions_notifications.patch
    - dropped patch that seems to actually no do anything
 -- Ken VanDine <email address hidden> Tue, 30 Mar 2010 21:17:04 -0400

Changed in empathy (Ubuntu):
status: Triaged → Fix Released
Changed in hundredpapercuts:
assignee: nobody → Ken VanDine (ken-vandine)
status: Triaged → Fix Released
Nicolò Chieffo (yelo3) wrote :

Should we open another bug for the missing notifications?

Conscious User (conscioususer) wrote :

Nicolò, you mean when the status icon is being used instead of the messaging menu? If I understood correctly, your patch already fixes the missing notifications when the messaging menu is being used, right?

Nicolò Chieffo (yelo3) wrote :

No, my patch fixes only the bug description, so the notifications from
users different from the first one are still missed.
I'm sure that the problem can be fixed in empathy-status-icon.c but
patch 35_check_actions_notifications.patch needs some additional work.
I'm sorry but I can't do this, because now I have no time.

Conscious User (conscioususer) wrote :

Ah, I see. Then I guess a new bug report would be adequate, in my opinion.

Conscious User (conscioususer) wrote :

Please let me know if you report it, so I can confirm it (just tested it).

(I could report it myself, but I don't know if you are going to and don't want to risk reporting a duplicate)

Nicolò Chieffo (yelo3) wrote :

Can you do the report? I'm very busy in these days, sorry.

Conscious User (conscioususer) wrote :

Bug #552543 reported. Please check if everything is okay over there.

Changed in empathy:
status: Unknown → Fix Released
Changed in empathy:
importance: Unknown → Wishlist

I hope I am not reviving an old bug unnecessarily. On my Ubuntu 11.04 Beta (fresh install of Beta 1, now all packages updated to newest version), Empathy shows one notification per message, which means the next message gets shown no sooner than when the notification for the previous one "timeouts". I would expect the next message from the same person get appended to the old one in the existing notification, like with Pidgin on Ubuntu 10.10 I used previously. Are my expectations correct, ie. should this bug be reopened?

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.