GNOME Bugzilla – Bug 606331
drop messaging.js, use notification-daemon protocol for IM stuff
Last modified: 2010-01-18 17:56:05 UTC
We had initially intended to use Telepathy in gnome-shell to implement the messaging-related features. I now think this is the wrong idea, and we should instead just have empathy implement the features we want, and use non-IM-specific APIs to communicate with gnome-shell. 1) We're not getting rid of empathy anyway; our current design still depends on empathy for the account editor, contact list, full-size chat window, non-chat-based actions (file sharing, desktop sharing), etc 2) While it is possible for the shell to watch Telepathy messages to get notified about messaging-related events, then make more Telepathy calls to fetch avatar images, and more Telepathy calls to subscribe to presence notifications for users we care about, and then watch Telepathy notifications to find out when avatars or presence status changes, this is kind of silly because empathy is already doing all of this, and already emitting notification-daemon-ready notifications when events occur. 3) We want some moderately deep integration between the shell and IM in some places (the "detail mode" of the message tray). However, we also want moderately deep integration between the shell and *other* apps too (eg, the rhythmbox example in Jon's mockup), so we know we're going to have to have a nice generic interface there that various apps can plug in to, and so empathy can just use these new APIs to provide the IM detail mode. 4) Having the shell implement the IM detail mode itself also doesn't make sense for other reasons, since it needs to get access to empathy-specific data (eg, chat logs) which would not be available via Telepathy D-Bus APIs, and since we want the mini chat window to be vaguely similar to the full chat window in terms of features. (Eg, spell checking) 5) The notification daemon protocol is sufficiently extensible to do what we want: the server provides a list of capabilities it supports, which can be used by the clients (empathy, rhythmbox, etc) to determine whether or not to use the "extended" notification style, and the Notify message includes a generic "hints" hash table that can be used to provide additional data to allow the client and daemon to negotiate the "detail mode". (Or we can just implement that as an additional D-Bus interface.) so, the attached patches add a few more missing bits to notificationDaemon.js, and then remove messaging.js
Created attachment 150985 [details] [review] Show both summary and body of notifications, and support body-markup
Created attachment 150986 [details] [review] remove messaging.js; use notificationDaemon.js for empathy messages
Review of attachment 150985 [details] [review]: Looks good. The commit message should probably have a "why" in its body, something like "Notification body has important content, such as the text of the chat message." The commit message can also mention that an appropriate backup icon gets set depending on the urgency. ::: js/ui/notificationDaemon.js @@ +156,3 @@ MessageTray.Source.prototype._init.call(this, sourceId); + hints = Params.parse(hints, { urgency: Urgency.NORMAL }, true); Should the default urgency by NORMAL or LOW? What concerns me is that we assign a warning icon to the NORMAL urgency messages, which might not be appropriate by default.
Review of attachment 150986 [details] [review]: Looks good. The commit message should have a "why" body. Can just move "notificationDaemon.js is handling the Empathy message." to the commit message body.
pushed with suggested changes, plus added a call to GLib.markup_escape_test() on the summary, since that's what notification-daemon does