Notification entries are not appended unless a replace is used

[Ted Gould]

Currently in the notify-osd code an append is only done if the type of update to the notification is a replace or update operation. This is incorrect and shoudl be changed to match the spec which states:

"Whenever there are two or more notification bubbles in the queue that have the same title and come from the same program (as identified by its D-Bus ID), and both of them have hinted that they allow concatenation, Notify OSD should merge them into a single bubble"
https://wiki.ubuntu.com/NotifyOSD#Concatenating%20existing%20bubbles

The important phrasing here is "two or more notification bubbles" not an updated bubble.

The reason for this is that the way that it's implemented now removes some capability from applications. The application needs to have the ability to update appended bubbles. So if I have two bubbles:

a) "hello"
b) "hi there"

And I decide that I want to change b to "oh, goodbye", if the append hint is used, and replace is used to specify append, the result will be:

hello
hi there
oh, goodbye

When the correct result should be

hello
oh, goodbye

This is currently how the Pidgin-libnotify plugin sends bubbles, it ensures that all messages are unique bubbles but will reuse bubbles for cases like sign on and sign off so that they get updated properly.

[David Barth (dbarth)]

Mirco, just confirming that your on top of this one, and that it is a critical feature to fix for the messaging indicator experience.

[Noel J. Bergman (noeljb)]

I believe that I can confirm this defect, which is still listed as New. If you take the documented sample that is supposed to show the "correct" way to use append notification, and add a second copy of the notification bubble, you will see that instead of merging the two bubbles, they are independently queued.

--- append-hint-example.py.original 2009-03-26 10:12:06.000000000 +0100
+++ append-hint-example.py 2009-03-26 10:13:00.000000000 +0100
@@ -136,6 +136,15 @@
      "Phil owned the place like no one before him!",
      "notification-message-IM")
   n.show ()
+ n = pynotify.Notification ("Cole Raby",
+ "Hey Bro Coly!",
+ "notification-message-IM")
+ n.set_hint_string ("x-canonical-append", "allowed");
+ n.show ()
+ n.update ("Cole Raby",
+ "Phil owned the place like no one before him!",
+ "notification-message-IM")
+ n.show ()
  else:
   print "The daemon does not support the append-hint!"

[Nicolas (nicolas-espina)]

Some news about that? If this is fixed then pidgin can display the messages like a charm! :P

Cheers!

[Abhishek Mukherjee (linkinpark342)]

I have a temporary workaround for pidgin-libnotify. I realize this probably isn't the place to put it but opening a new bug against pidgin-libnotify seemed like a bad idea because it isn't a bug on their end. It's just a one line change anyways for those impatient devs like me :P I'd recommend putting in your ~/.purple/plugins instead of system wide since this patch should never be put into any package :P

[Chow Loong Jin (hyperair)]

On Thu, 2009-04-02 at 19:14 +0000, Abhishek Mukherjee wrote:
> I have a temporary workaround for pidgin-libnotify. I realize this
> probably isn't the place to put it but opening a new bug against pidgin-
> libnotify seemed like a bad idea because it isn't a bug on their end.
> It's just a one line change anyways for those impatient devs like me :P
> I'd recommend putting in your ~/.purple/plugins instead of system wide
> since this patch should never be put into any package :P
>
> ** Attachment added: "Temporary workaround for NotifyOSD appending in pidgin-libnotify"
> http://launchpadlibrarian.net/24708071/append-patch.diff
>
Or grab pidgin-libnotify from my PPA.
--
Chow Loong Jin

[Abhishek Mukherjee (linkinpark342)]

Well... Not to be shown up by hyperair :P... I felt the urge to actually do what I think might be implementing the correct way of the spec into notifyosd itself.

Be gentle, this is my first time coding in a) gtk, b) launchpad related projects, and c) bzr (not that big a deal but i needed a "c)," felt it would be incomplete without one) The one thing i worry about the most as a result of this is I'm not 100% sure I'm freeing the old bubble properly. Anyways, I'm in your (the theoretical mass of developers reading this) to tell me what to do with this

[Mark Shuttleworth (sabdfl)]

Folks, can you confirm that Pidgin appending properly is on track for 9.04?

Mark

[Nicolas (nicolas-espina)]

Somebody can make it work? I try to compile the lastest pidgin-libnotify and i can't find the lines to change :S ... And the hyperair PPA has a old versions of pidgin-libnotify

[Chow Loong Jin (hyperair)]

On Fri, 2009-04-03 at 17:35 +0000, Nicolas wrote:
>
> Somebody can make it work? I try to compile the lastest
> pidgin-libnotify
> and i can't find the lines to change :S ... And the hyperair PPA has a
> old versions of pidgin-libnotify
>
There aren't any differences between my PPA package and the -0ubuntu5
version. Hence it's a regression between -0ubuntu5 => -0ubuntu7.
--
Chow Loong Jin

[Abhishek Mukherjee (linkinpark342)]

While experimenting a bit more, it seems my patch breaks normal replaces functionality. I'm looking into it right now; just don't do anything drastic with the code above...

Could i ask for a couple clarifications in the spec. What is to happen if replaces_id is also set when trying to append to a bubble? My interpretation says that appending takes precedence over replacing but I just wanted to make sure.

Because of my little replacing oversight, the result is a smidgen less beautiful but it's still readable... I diffed it against the HEAD (or similar for bzr), not against my previous patch

[Mark Shuttleworth (sabdfl)]

I would say that replacing overrides appending. In other words, if you
replace, you replace everything.

My rationale for that is that, in the absence of appending, each message
would be a different bubble. And a replace would completely replace that
bubble. Appending lets us keep older messages around while showing the
newer ones inside the same bubble. But a replace should reset the whole
lot, IMO.

And I think it would be easier to implement :-)

If you do get replace-just-the-one-message working nicely, though, I'd
be really interested to kick the tires on it and see how it feels.

Mark

[Matthew Paul Thomas (mpt)]

Agreed, replacing should override appending. I'll update the spec shortly to reflect that.

[Abhishek Mukherjee (linkinpark342)]

Picky picky :P. just kidding. Here's a new patch. Just to summarize to make sure everyone's on the same page, the order of importance is:

if replaceable bubble exists:
    replace old bubble text with new text
else if appendable bubble exists:
    append new text to old text
else
    make a new bubble

[Mark Shuttleworth (sabdfl)]

Do we want to support append-on-replace at all? I find the "Joe is
offline/Joe is online" append really ugly. That should be a replace -
period. The append only makes sense when it WOULD have been two
consecutive messages.

Ted, I think this sort of confusion is a result of us not being willing
to define appending more explicitly in the API. Because we wanted it to
be implicit and "easier for the developer", we've ended up with a
mishmash, both on our side (we aren't even implementing our own spec
correctly!) and in apps.

In a worst-case scenario, we might ship a bad implementation in Jaunty,
and then NOT BE ABLE TO CHANGE IT because third-party apps like Skype
implement to our bug, not our spec!

Mark

[Ted Gould (ted)]

On Tue, 2009-04-07 at 15:14 +0000, Mark Shuttleworth wrote:
> Do we want to support append-on-replace at all? I find the "Joe is
> offline/Joe is online" append really ugly. That should be a replace -
> period. The append only makes sense when it WOULD have been two
> consecutive messages.
>
> Ted, I think this sort of confusion is a result of us not being willing
> to define appending more explicitly in the API. Because we wanted it to
> be implicit and "easier for the developer", we've ended up with a
> mishmash, both on our side (we aren't even implementing our own spec
> correctly!) and in apps.

I'm sorry, perhaps I missed something in the conversation. We should
absolutely not support append on replace. Because if we do that we end
up in the case where we can't replace notifications that are appended.
I think that's pretty much what the original bug report stated.

If you send:

 "message A" -> append
 "message B" -> append

That might be what you want to appear as:

  +----------------+
  | message A |
  | message B |
  +----------------+

If for some reason "message A" changes then there is no way to refer to
it if the replace is only "message B".

[Mark Shuttleworth (sabdfl)]

Ted Gould wrote:
> On Tue, 2009-04-07 at 15:14 +0000, Mark Shuttleworth wrote:
>
>> Do we want to support append-on-replace at all? I find the "Joe is
>> offline/Joe is online" append really ugly. That should be a replace -
>> period. The append only makes sense when it WOULD have been two
>> consecutive messages.
>>
>> Ted, I think this sort of confusion is a result of us not being willing
>> to define appending more explicitly in the API. Because we wanted it to
>> be implicit and "easier for the developer", we've ended up with a
>> mishmash, both on our side (we aren't even implementing our own spec
>> correctly!) and in apps.
>>
>
> I'm sorry, perhaps I missed something in the conversation. We should
> absolutely not support append on replace. Because if we do that we end
> up in the case where we can't replace notifications that are appended.
> I think that's pretty much what the original bug report stated.
>
> If you send:
>
> "message A" -> append
> "message B" -> append
>
> That might be what you want to appear as:
>
> +----------------+
> | message A |
> | message B |
> +----------------+
>
> If for some reason "message A" changes then there is no way to refer to
> it if the replace is only "message B".
>
Agreed.

So how did we end up here? I think it's because we were too lazy fairy
(;-)) when we introduced append, in the misguided idea that "making it
just happen easily" would be good for developers. But in the end,
because we did not make it a concrete explicit process, where the
developer says "append this to THAT message", we end up in the vague
world where even our own notification system is confused, and developers
more so.

Mark

[Mirco Müller (macslow)]

Here is a screencast of the current notify-osd trunk: http://people.ubuntu.com/~mmueller/append-hint-example.ogg. This uses a new bubble with the append-hint set for the first four chunks of text passed. The fifth chunk of text uses a normal update (which acts as a replace operation).

This is what happens in pseudo-code :

bubble1 = bubble_new ()
bubble_set_append (bubble1, TRUE)
bubble_set_content (bubble1, "Cole Raby", "Hey Bro Coly!", im_icon)
...
bubble2 = bubble_new ()
bubble_set_append (bubble2, TRUE)
bubble_set_content (bubble2, "Cole Raby", "What's up dude?", im_icon)
...
bubble3 = bubble_new ()
bubble_set_append (bubble3, TRUE)
bubble_set_content (bubble3, "Cole Raby", "Did you watch the air-race in Oshkosh last week?", im_icon)
...
bubble4 = bubble_new ()
bubble_set_append (bubble4, TRUE)
bubble_set_content (bubble4, "Cole Raby", "Phil owned the place like no on before him!", im_icon)
...
bubble_update_content (bubble4, "Cole Raby", "Did you take any photos during the race?", im_icon)

[Matthew Paul Thomas (mpt)]

Mark, we ended up here because I didn't spec the corner case of what should happen when an application tries to replace and append at the same time. But we would have had the same issue if we'd chosen a different protocol to specify appending. For example, if we'd introduced an "append this to THAT message" parameter instead of the "x-canonical-append" hint, it would still be possible for applications to compile whose code included notifications with both that parameter and a a "replaces_id" parameter, so Notify OSD would still have to make the same decision about which to obey. The only difference would have been that it would be harder for applications to use appending (because they'd need to keep track of notification IDs).

I've now updated the spec to specify that replacing overrides appending. <https://wiki.ubuntu.com/NotifyOSD?action=diff&rev2=134&rev1=133> The money quote: "Notify OSD should merge two notifications into a single notification if all of the following are true: ... * the second notification is not specified as a replacement of any other notification."

Separately, as I said in Berlin, I think we should not use the word "update" when talking about notifications themselves. It can mean either of two very different things (appending and replacing), which makes bug reports like this one take longer to clarify.

[Mark Shuttleworth (sabdfl)]

Mirco, the mockup looks great.

I think replaces-of-appended messages are going to be VERY rare, though,
right? Replacement is useful for "updating a changing status" while
appending is useful for "continuations, where the old stuff is still
relevant".

MPT, if we had done "append" as a new operation in its own right, we
would not have the API issue you describe.

Mark

[Ted Gould (ted)]

On Wed, 2009-04-08 at 15:00 +0000, Mark Shuttleworth wrote:
> MPT, if we had done "append" as a new operation in its own right, we
> would not have the API issue you describe.

Doing that would have required changing libnotify, along with all the
other bindings that actually don't use libnotify but reimplement the
spec. I think that it was a strong selling point for us to be able to
say that we required no changes in libnotify, because we were just doing
the same thing, even if we did a twist on it.

Also, our application patches would have required conditional compiles
based on detecting our version of the libnotify library and would have
been much less likely to get upstream.

Even with the confusion, and I agree with mpt's analysis, I still think
we made the better choice between those two options.

[Mark Shuttleworth (sabdfl)]

Ted Gould wrote:
> Even with the confusion, and I agree with mpt's analysis, I still think
> we made the better choice between those two options.
>
Be that as it may, I would like you to make a different choice in
future. I'm not offering suggestions in this case.

Mark

[Mirco Müller (macslow)]

Lastest bit of notify-osd 0.9.11 with append now being even more solid http://people.ubuntu.com/~mmueller/append-example-with-sync-bubble.ogg

[Mark Shuttleworth (sabdfl)]

Mirco Müller wrote:
> Lastest bit of notify-osd 0.9.11 with append now being even more solid
> http://people.ubuntu.com/~mmueller/append-example-with-sync-bubble.ogg
>
We had extensive discussions about NOT sliding the bottom bubble under
any circumstances. Why is it sliding here?

Mark

[Nicolas (nicolas-espina)]

Nice... The last commit works fine and is compatible with pidgin using the append option :D.... Thanks!

[Mat Tomaszewski (mat.t.)]

Mark Shuttleworth wrote:
> Mirco Müller wrote:
>
>> Lastest bit of notify-osd 0.9.11 with append now being even more solid
>> http://people.ubuntu.com/~mmueller/append-example-with-sync-bubble.ogg
>>
>>
> We had extensive discussions about NOT sliding the bottom bubble under
> any circumstances. Why is it sliding here?
>
> Mark
>
>
Also, the overflow should be handled slightly differently:
https://wiki.ubuntu.com/NotifyOSD#Overflow%20of%20long%20messages

m.

[Mirco Müller (macslow)]

@ Mark: I don't recall that. What I do remember is that we wanted to avoid sync. and async. bubbles switching positions and having a bubble-height gap between the screens top-edge and the normal position of an async. bubble. I'll get in contact with mt to give me an update on that.

@ Mat: I know, but it not simple to precisely extract the first line rendered by pango. Actually one cannot do that at all. So the current solution is the best compromise we can provide atm. This is purely based on the string and the contained newline-characters.

[Mat Tomaszewski (mat.t.)]

Mark Shuttleworth wrote:
> Mirco Müller wrote:
>
>> Lastest bit of notify-osd 0.9.11 with append now being even more solid
>> http://people.ubuntu.com/~mmueller/append-example-with-sync-bubble.ogg
>>
>>
> We had extensive discussions about NOT sliding the bottom bubble under
> any circumstances. Why is it sliding here?
>

Mirco - not your fault. We have not included this property in the spec.
Our bad.

David Barth is aware of the issue and the amended code will land as soon
as ready.

M.

[David Barth (dbarth)]

For the record, I worked with Mirco to make that change because it was the safest/quickest bug fix we could land in time for the freeze.

I understand that it is different from the specification. I estimated that a visual collision between two bubbles was worse than the sliding.

[Chow Loong Jin (hyperair)]

On Thu, 2009-04-09 at 12:58 +0000, Mirco Müller wrote:
> @ Mark: I don't recall that. What I do remember is that we wanted to
> avoid sync. and async. bubbles switching positions and having a
> bubble-
> height gap between the screens top-edge and the normal position of an
> async. bubble. I'll get in contact with mt to give me an update on
> that.
>
Regarding this, there's actually a bug where a low priority bubble
renders on top of a high priority bubble. See Bug #345837.
--
Chow Loong Jin

[Noel J. Bergman (noeljb)]

This is marked as FIX RELEASED, but when I run the documented example (https://wiki.ubuntu.com/NotificationDevelopmentGuidelines?action=AttachFile&do=view&target=append-hint-example.py) on a clean karmic install I do NOT see the documented append behavior. It simply replaces.

[Noel J. Bergman (noeljb)]

Checked with several other folks on IRC, including mac_v, and none of us see append working. None of us see the type of behavior expected or shown in Mirco's videos.

[Mirco Müller (macslow)]

Am Samstag, den 17.10.2009, 16:01 +0000 schrieb Noel J. Bergman:

> Checked with several other folks on IRC, including mac_v, and none of us
> see append working. None of us see the type of behavior expected or
> shown in Mirco's videos.

 This is certainly "Fix Released". I just have not - until now - updated
the wiki-page. Check the examples directory of notify-osd trunk for
up-to-date code-examples. Sorry for the inconvenience, Noel!

Best regards ...

Mirco

[Noel J. Bergman (noeljb)]

Marco, thank you for the update!

PLEASE either update the wiki or just replace the code listings there with a link to the current code (http://bazaar.launchpad.net/~notify-osd-developers/notify-osd/main/files/head%3A/examples/). Some of us are using your examples to try and update/maintain other tools. And, as you can see, having the wrong code on the wiki caught us out.

[Shinobi (dinko-tenev)]

Append does not work across runs of the test script (https://wiki.ubuntu.com/NotificationDevelopmentGuidelines?action=AttachFile&do=view&target=append-hint-example.py).

Run the test twice in succession: the messages from the second run are not appended to the bubble from the first run.

[Shinobi (dinko-tenev)]

Oops...I seem to have missed the "come from the same program" part in the spec.

[Shinobi (dinko-tenev)]

Then again, that may be irrelevant.

If "same program" means "same app_name parameter to org.freedesktop.Notifications.Notify" (which it is quite reasonable to assume,) then both runs' notifications should be treated as coming from the same program, and second run's notifications should be appended to the bubble created for the first run, which is not the case.

Can anyone shed some light into this, please?