Bug 13349

I've rebased the version of Olli's branch imported into smcv's "spec-conversions" repository onto current master, updated it to build with the current spec generation tools, and made some tweaks to the interface (mainly turning a getter into some D-Bus properties).

A snapshot of the interface is available at <http://people.freedesktop.org/~wjt/telepathy-spec-mail_notification/spec/org.freedesktop.Telepathy.Connection.Interface.MailNotification.DRAFT.html>.

Pre-emptive comments on my own branch:

* Should UnreadMailStatusChanged grow a dict of misc. for future expansion?
* Do protocols exist that would support MailReceived but not UnreadMailStatusChanged?
* Do protocols exist which don't give you a URL for your inbox, and if so do we need to do anything more elaborate than set URL to "" in those cases?

Nobody seems very excited about this spec. To the back burner it goes...

I'm trying to revive the subject. I've started a wiki to store information an reflection about that.

http://telepathy.freedesktop.org/wiki/Email%20Notification

The current draft need rework to be able to fit GMail thread base mail notification (e.g. multiple unread senders on a thread, the fact that we are talking about a thread, etc.). Also the definition of cookie does conceptually allow for cookie attribute like path, domain, expires, etc. I'll post my git tree soon.

p.s. Also take note that I may rename the spec to Email_Notification, this is cosmetic, but it's better to do it before the API is released. 

A new version of Email_Notification Draft can be found at:

git://git.collabora.co.uk/git/user/nicolas/telepathy-spec.git
branch email-notification

Pre-compiled version:
http://people.collabora.co.uk/~nicolas/telepathy-spec-email_notification/spec/

After discussion with Sjoerd and Will on #telepathy, I've pushed a new version of the interface.

- Changed cookies into array of strings (as), instread of array of struct (a(s))
- Changed Email type into enumration
- Changed Email into a hash map
- Removed priority from Email hash map (to be added if a protocol support it)
- Added Subscribe/Unsubscri method to allow CM to track users and reduce ovehead if nobody is using it.

Push a new update to the spec. I was using tp-type instread of tp:type at some plane. Fixing that reveled some other errors, we must use Email[] instead of Email_List and UnreadEmails property type was wrong (a(sv) instread of a(a{sv})).

After discussion, consensus was that Mail is shorter and less ambigues name for this API (e.g. Mail vs Email, EMail or E_Mail). So Ive reverted renaming and pushed a new version (HTML is also update this time).

* Removed Cookie Support 
It's currently not possible (without browser modification) to pass a cookies for a remote domain (security reason). Cookie sharing would be required, but that's a huge architecture thing, not a telepathy issue.

* Added capabilities
In the previous version, it was not very clear what and how the CM would provide information and notification. The capabilities bitmask allow CM to explicitly describe their behavior.

Branches: email-notification, remotes/dev/email-notification
Follows: telepathy-spec-0.18.0
Precedes: 

    Capability enum become Mail_Notification_Flags
    
    The enumeration name is changed from Capability to
    Mail_Notification_Flags, as Capability is too generic.
    Also as this is about flags, using tp:flags instead.
    
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

The unread mails update mechanism does not work very well since we can add and remove unread mail at the same time. Also, with emails thread, its possible that an email information just changed (e.g. snippet and senders). 

For this reason I'm going to remove the added/removed signal and add a changed signal. This signal will have a parameter with the list of mails to update or add and a parameter with the list of ids to remove.

While doing integration in butterfly, I discovered that mail URL and optional POST data may not be shared between different client. On some protocol the URL and/or POST data may contains authentication token that are not reusable. Also, those token may require extra network access to construct. 

For this reason, my solution would be to remove URL and POST data from properties and Mail type and introduce two methods, RequestInboxURL and RequestMailURL that will respectively return the correct information to open inbox and specific mail Web UI without re-authentication.

In Mail type, we also introduce an url_data field that can be used by CM to put any useful information to build an URL that opens a specific URL. This information (along with ID) will be usefull for protocol that cannot stores mails on CM side.

Also, during development I changed the mail ID from 32bit integer to 64bit integer. It solved issue I had with Google, but at the end is not flexible enough. Thus I'll change this ID into a string.

New version of the interface can be found at:

http://people.collabora.co.uk/~nicolas/telepathy-spec-email_notification/spec/org.freedesktop.Telepathy.Connection.Interface.MailNotification.DRAFT.html

Thanks for reviewing

Spec has been updated base on comments I've received on the mailing list. Note that the Mail type has remain untouch. In turns out that there is very few benifit of using Message structure for e-mail. I'll collect comments of second pass comity review to make final decision on it. Here's the commit log:

commit 6e0138653eb7884d2e325894fb601e1a5e3825cc
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Tue Jan 26 10:37:29 2010 -0500

    MailNotification: fix some english errors and split too long lines
    
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

commit 21c3d4fbf83221919f85b9beaf34c9db558e9077
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Mon Jan 25 18:15:20 2010 -0500

    Documented the exact format for HTTP Post Data
    
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

commit 46b6e01a895c92427aa1b5c1d6f6fa04b807fd09
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Mon Jan 25 17:36:38 2010 -0500

    Fixed typo in url-data mail attribute
    
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

commit f8dbba0d2dc4378766692ded57a850eb256f388b
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Mon Jan 25 12:29:37 2010 -0500

    More mail notif capabilities doc and missing flags
    
    Added more documentation to capabilities, explaining the four possible
    groups and giving examples of wich protocols support it. Also added missing
    capabilities for Request methods and added a method to request a URL to
    compose e-mail.
    
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

commit 9d50afed288f6f16965e65feccfb6c950e2e2da9
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Fri Jan 22 12:30:08 2010 -0500

    Mail Notification: Clarify need for Subscribe/Unsubscribe methods
    
    Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

General comments:

On the properties, please explicitly say either "this property cannot change after the Connection becomes CONNECTED" or "change notification is via the <tp:member-ref>Foo</tp:member-ref> signal".

Since this interface has several orthogonal "features", I think it would be useful to say explicitly, for each method, signal and property (and, where applicable, method arguments) which capability flag it depends on.

You've used the NotCapable error; that error is meant to be for incapable *contacts* (see its description), so I'd prefer it if you used NotImplemented to represent an incapable server.

Method parameters and struct members should be in Ugly_Case for easy mapping to any case convention, e.g. Mail_URL should contain members URL, Method and POST_Data (or Post_Data if you prefer). Similarly, the members of the Mail_Type enum should be Single and Thread, not single and thread, and the parameters of RequestMailURL should be ID and URL_Data.

We use capitalized MUST, SHOULD, MAY, MUST NOT etc. in the same way as RFCs; it might be worth adding a few of these.

Intro:

Concrete examples of protocols are currently buried in the middle of the document; either move them to the intro, or say something like "see <tp:member-ref>Capabilities</tp:member-ref> for examples of some common protocols' capabilities" in the intro.

"When unread e-mails arrive into the into or unread e-mails are marked read or deleted the UnreadMailsChanged signal will be emitted" is not actually a true statement; you go on to explain that that only happens on more capable protocols. You should probably start by explaining that different clients have different capabilities, and clients should look at the Capabilities property; then explain (or reference), for each value of Capabilities, how clients should behave.

Subscribe:

Factual error: refcounted subscriptions are for the benefit of clients that have, or are, plugins, rather than connection managers that have plugins.

RequestMailURL:

Why are the ID and the URLData both needed? Can't you just use the URLData as the unique ID, or store a map { ID => URLData } inside the CM?

MailsReceived:

You say "An array of Mails. Those e-mails do not have a unique "ID".". Please write this in terms of the actual key that they don't have - it's "id", right?

Is it the case that mails in MailsReceived *do not necessarily* have the "id" key, or *must not* have the "id" key? Please specify which. (I believe it's the former, since the latter would be strange.)

Capabilities:

We still need capability flags for the various URLs.

Mail:

For "id", I think it's worth saying "protocols with the Supports_Unread_Mails /Capability/ MUST provide this key in each /Mail/" (where // indicates a hyperlink).

Is "id" globally-unique, unique over multiple sessions for the same account, or only guaranteed to be unique within a session? (I suspect you intend it to be the last of those.)

sent-timestamp and received-timestamp should have the Unix_Timestamp64 type, with signature 'x'.

In Messages, we indicate truncation by setting a "truncated" flag and re-using "body", rather than creating a new "snippet" key - any particular reason not to do this?

HTTP_Post_Data:

This could really do with an example that includes characters that are special in both HTTP and HTML, like this:

> For example, if the POST data should contain a key "less-than" with value
> "<", and a key "percent" with value "%", this should be represented as
> two HTTP_Post_Data structures, ("less-than", "<") and ("percent", "%"),
> resulting in a POST request whose request body is "less-than=<&percent=%25".
> If a client passes this to a browser by writing it into an HTML form, it
> could do so by representing it as:
>
> <input type="hidden" name="less-than">&lt;</input>
> <input type="hidden" name="percent">%</input>

Mail_Address:

This could do with an example to make it clear that there are no angle-brackets etc., like ("Nicolas Dufresne", "nicolas.dufresne@collabora.co.uk") :-)

Mail_URL:

"Because of the nature of the POST data" is rather vague. "Because the POST data frequently contains short-lived credential tokens" would be better.

Few more commits to address Simon comments:

commit d87b72434aa6a81a18a43e92fb2165a1158bc1eb
Date:   Wed Jan 27 15:31:08 2010 -0500
    MailNotification: Added example for HTTP_Post_Data and Mail_Address

commit 5c56487650dcc4adc6257aadaad998757d304e60
Date:   Wed Jan 27 14:50:40 2010 -0500
    Clarified and rework mail structure
    
    Clarified mail structure documentation about "id" and the way the
    content of a message may be sent.

commit 40b7204133054d2a6754572cfac2c865eb58bd97
Date:   Wed Jan 27 13:46:22 2010 -0500
    MailNotification: Clarify "id" presence in ReceivedMails context

commit 4f34b8d0b4ef13f8fde52512f50d680c7e2a8bd8
Date:   Wed Jan 27 11:28:26 2010 -0500
    MailNotification: Rewrote intro moving caps group into it

commit 34f547d7a0a5ec67ef8da7089330b94bdb3eef3d
Date:   Tue Jan 26 15:37:22 2010 -0500
    MaiNotification: Clarify why POST Data MAY NOT be shared

commit 978f6ecf79248c638746613cf8026de10c8515fc
Date:   Tue Jan 26 15:31:28 2010 -0500
    MailNotification: Capitalized MUST, MAY, NOT, SHOULD, CANNOT

commit ebaea8ea30e06b08791b8539532ad3d00e8ad56a
Date:   Tue Jan 26 14:42:50 2010 -0500
    MailNotification: Use Ugly_Case for members, enumvalues and args

commit ab58b558c2086729a7fae172e7e5f22ba9c262e4
Date:   Tue Jan 26 13:27:25 2010 -0500
    MailNotification: Use NotImplemented instead of NoCapable

commit 60cf3a88f15db009e7653654acec0889150a2f41
Date:   Tue Jan 26 13:24:16 2010 -0500
    MailNotification: Document Capabilities for each property/method/signal

commit e07425754962683e95b08d70661a3dc692af9870
Date:   Tue Jan 26 12:59:19 2010 -0500
    MailNotification: Explicitly state where or when the value changes

(In reply to comment #16)
> 
> RequestMailURL:
> 
> Why are the ID and the URLData both needed? Can't you just use the URLData as
> the unique ID, or store a map { ID => URLData } inside the CM?

Encoding the ID in the URL_Data would lead to duplication of data inside the Mail hash map. URL_Data should be used to store information need to create the URL that is provided with the mail information (e.g. the base URL, or some special parameters). Also, it is more friendly to programmers if those two information are split since you don't have to parse it whenever you need one of the item. This is a bit like the split in address (name, addr) instead of parsable syntax like "Sender Name <sender@address>".

> 
> Capabilities:
> 
> We still need capability flags for the various URLs.

That was already there.

Spec-meeting conclusion is that this needs editorial changes, but is "the right shape" to merge as a draft. I'll proofread it and give you a branch with typo fixes, etc., after which we can merge it.

The wording for Mail_URL is a little confusing; I'll try to clarify.

> We use capitalized MUST, SHOULD, MAY, MUST NOT etc. in the same way as RFCs; it
> might be worth adding a few of these.

You seem to have used these in ways that don't follow <http://www.ietf.org/rfc/rfc2119.txt> - I'll check these while proofreading.

(In reply to comment #17)
> Encoding the ID in the URL_Data would lead to duplication of data inside the
> Mail hash map. URL_Data should be used to store information need to create the
> URL that is provided with the mail information (e.g. the base URL, or some
> special parameters). Also, it is more friendly to programmers if those two
> information are split since you don't have to parse it whenever you need one of
> the item. This is a bit like the split in address (name, addr) instead of
> parsable syntax like "Sender Name <sender@address>".

I infer that the rationale for this is something like:

| In some protocols, two pieces of information are needed to construct a URL; one is the unique identifier of the message, and one is a temporary authentication token, nonce, etc. If this method only took the ID as an argument, it would be necessary for the CM to keep a map from the ID to the temporary data, but in some protocols it's not possible for the CM to know when it can discard entries from this map; if this method only took the URL_Data as an argument, it would be necessary for the CM to encode both the ID and the temporary data into the URL_Data, then re-parse it.

Am I right?

"More friendly to programmers" would be better phrased by saying which programmers it's more friendly for - CM developers or client developers :-)

Seems the telepathy-bugs cc got lost; setting it as QA contact.

I've fixed various typos and made clarifications and editorial changes in <http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/mail-notification-editorial> - Nicolas, please review these, and merge them into nicolas/mail-notification if you approve?

Things I still don't like, after that branch is merged:

Mail_Type distinguishes between "is a mail" and "is a thread". A thread isn't a type of mail :-P so I think this should be called Mail_Notification_Type (both a Mail and a Thread could be said to be types of mail notification). Perhaps Mail should be renamed to Mail_Notification, too?

Subscribe is documented to raise Disconnected and NetworkError (which I can understand), but also NotAvailable and PermissionDenied (which I can't). When would it raise these errors?

sent-timestamp, received-timestamp are meaningless for threads, as currently written; please either forbid them for threads, or re-define them as "the most recent" or "the oldest" (your choice).

MailsReceived allows 'id' to be non-unique, which directly contradicts the definition of 'id'. Possible resolutions:

(A) Change the 'id' uniqueness requirement so it only applies if Supports_Unread_Mails is set. This seems weird and undesirable to me - the guarantees provided by a particular key should either always be set, or never be set.

(B) Relax the 'id' uniqueness requirement from "unique per Connection" to "unique until removed". This is basically the same change as (A), to be honest (since on protocols without Supports_Unread_Mails, you can't tell whether a mail was removed), but it seems like a nicer way to explain it.

(C) Require 'id' to be either unique or omitted, and munge the necessary info into url-data on protocols where IDs are non-unique. On MSN, this brings you back to the situation you wanted to avoid...

(D) Change url-data so it can have any type, and change the signature of RequestMailURL to RequestMailURL(v: URL_Data) or RequestMailURL(s: id, v: URL_Data); in MSN, omit the 'id' if it can't be guaranteed to be unique,
but put the URL data in the map as a pair of strings (id, POST data) or whatever. This requires the client to remember a GValue (or QVariant) rather than just a string, but lets the Connection put anything it wants in it (integers, strings, binary blobs, whatever).

(E) A restricted form of (D): change url-data's type to 'as', and change the signature of RequestMailURL to RequestMailURL(as: URL_Data) or RequestMailURL(s: id, as: URL_Data); in MSN, do basically the same thing as for (D).

(F) Anything cleverer that someone comes up with :-)

(In reply to comment #20)
> (B) Relax the 'id' uniqueness requirement from "unique per Connection" to
> "unique until removed".

Sjoerd and I discussed this in the office and we now think we should make this change anyway, forbid "id" entirely on connections where you can't tell whether it's still valid (i.e. those that don't have Supports_Unread_Mails), and deal with the RequestMailURL problem by putting more info in URL_Data:

> (D) Change url-data so it can have any type, and change the signature of
> RequestMailURL to RequestMailURL(v: URL_Data) or RequestMailURL(s: ID, v: URL_Data)

So, in protocols where you have several things to encode in URL_Data but no sufficiently-unique IDs, you don't need to glue the URL data into a single string and then parse it again - you can just give the client a string list in a variant (a GValue, in GLib terms) and expect the client to give it back to you intact.

(In reply to comment #20)
> Mail_Type distinguishes between "is a mail" and "is a thread". A thread isn't a
> type of mail :-P so I think this should be called Mail_Notification_Type (both
> a Mail and a Thread could be said to be types of mail notification). Perhaps
> Mail should be renamed to Mail_Notification, too?

Sjoerd doesn't seem to oppose renaming to Mail_Notification and Mail_Notification_Type. Nicolas, does that sound OK to you?

(In reply to comment #20)
> I've fixed various typos and made clarifications and editorial changes in
> <http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/mail-notification-editorial>
> - Nicolas, please review these, and merge them into nicolas/mail-notification
> if you approve?

I've merge you branch and apply some fixes, see comments below ...

> 
> Things I still don't like, after that branch is merged:
> 
> Mail_Type distinguishes between "is a mail" and "is a thread". A thread isn't a
> type of mail :-P so I think this should be called Mail_Notification_Type (both
> a Mail and a Thread could be said to be types of mail notification). Perhaps
> Mail should be renamed to Mail_Notification, too?

Mail_Type distinguish between "is a mail" and "is most recent mail of a thread". In the second case you have the mail information plus some more information about the thread (aka a context). The side effect is that you don't need to show the older mails withing the same thread since this would be outdated and redundant information.

Also Mail (if renamed Mail_Notification) is not an abstraction to all notifications that comes from the API. This naming imply a totally different design. It would feel like the API would provide a Mail_Notification for anything that happen.

> 
> Subscribe is documented to raise Disconnected and NetworkError (which I can
> understand), but also NotAvailable and PermissionDenied (which I can't). When
> would it raise these errors?

Turns out that errors where still random copy paste, fixed.

> 
> sent-timestamp, received-timestamp are meaningless for threads, as currently
> written; please either forbid them for threads, or re-define them as "the most
> recent" or "the oldest" (your choice).

Defined has the most recent (in link with the fact that thread == most recent mail of a thread).

> 
> MailsReceived allows 'id' to be non-unique, which directly contradicts the
> definition of 'id'. Possible resolutions:
> 

Fixed, 'id' must be unique otherwise unset.

Comments requested for <http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/mail-notification-editorial>:

(In reply to comment #23)
> Mail_Type distinguish between "is a mail" and "is most recent mail of a
> thread".

Based on IRC discussion I've promoted this indication to a flag on the Connection - either all Mails are really threads, or none are.

> Fixed, 'id' must be unique otherwise unset.

I've adjusted this as I suggested above, forbidden it on connections where its validity isn't visible, and made URL_Data into a variant as a way to pass several bits of information back into RequestMailURL.

This has been merged to telepathy-spec as a draft, and will be released in 0.19.1 later today. Repurposing this bug for the undrafted version.

Nothing here to review, at the moment.

Nicolas pointed out on the bug for the Gabble implementation that the display name in a Mail_Address can be empty. If this is the intention, the spec should explicitly say so, to remind client authors that they might need to fall back to displaying the email address.

Hi Nicolas,

* Do you have any more comments on this spec based on implementation experience in Gabble/Butterfly? Bug #27167 seems a little subtle, and could probably benefit from better spec wording, but shouldn't introduce incompatibility probably?

* Do we have UIs for this spec? Any objections based on implementation experience with them?

* You owe me a patch for Bug #27200 :-)

* Anything else we should sort out before undrafting this (= making it an API guarantee)?

(In reply to comment #28)
> Hi Nicolas,
> 
> * Do you have any more comments on this spec based on implementation experience
> in Gabble/Butterfly? Bug #27167 seems a little subtle, and could probably
> benefit from better spec wording, but shouldn't introduce incompatibility
> probably?

This is indeed subtle, since it happens during a very short period of time. I don't think the spec is to blame. The solution you proposed (to suspend async calls until dependent calls has complete) is the right solution. It's also true for any implementation of any specs. As I never wrote such a cascading I don't know what effort is required or if some tp-glib tools would help here to manage this.

> 
> * Do we have UIs for this spec? Any objections based on implementation
> experience with them?

Currently only MyZone has UI for it. It's a bit of work to do all the account/connection monitoring but when the MailNotification interface is reached it's quite straightforward to update UI with it.

> 
> * You owe me a patch for Bug #27200 :-)
Right, I was busy with proxy support, but I did not forgot. I'll do this today.

> 
> * Anything else we should sort out before undrafting this (= making it an API
> guarantee)?
> 
So far I'm happy with it and I'm confident that writing other UIs (What about Empathy, or a Gnome Applet/Tray) using this interface would not be an issue.

(In reply to comment #29)
> (In reply to comment #28)
> > Bug #27167 seems a little subtle
[...]
> The solution you proposed (to suspend async
> calls until dependent calls has complete) is the right solution.

I've asked for minor (documentation etc.) changes to the branch in Bug #27167, but I think the approach is now correct, so this isn't a blocker.

> Currently only MyZone has UI for it. It's a bit of work to do all the
> account/connection monitoring but when the MailNotification interface is
> reached it's quite straightforward to update UI with it.

That's encouraging!

I think undrafting this can go on the spec-meeting agenda, although there are a couple of remaining issues.

The Subscribe() and Unsubscribe() methods are rather generically named, and collisions with similar methods on other interfaces are likely. In Bug #21787 I added an Unsubscribe() method for presence subscriptions, which I think should "win" (i.e. be the one that keeps the name) if necessary. So, the question here is: do we care about bindings that can't use colliding methods?

In particular, in dbus-python it's hard, although possible, to implement colliding methods - you have to have them in different subclasses, or mixins, or something.

I believe most/all *client* bindings are happy enough to *call* two methods of the same name, though?

(In reply to comment #27)
> Nicolas pointed out on the bug for the Gabble implementation that the display
> name in a Mail_Address can be empty. If this is the intention, the spec should
> explicitly say so, to remind client authors that they might need to fall back
> to displaying the email address.

As far as I can see, it still doesn't.

(In reply to comment #30)
> (In reply to comment #29)
> > (In reply to comment #28)
> > > Bug #27167 seems a little subtle
> [...]
> > The solution you proposed (to suspend async
> > calls until dependent calls has complete) is the right solution.
> 
> I've asked for minor (documentation etc.) changes to the branch in Bug #27167,
> but I think the approach is now correct, so this isn't a blocker.

I missed that, sorry, I'll fix soon.

> The Subscribe() and Unsubscribe() methods are rather generically named, and
> collisions with similar methods on other interfaces are likely. In Bug #21787 I
> added an Unsubscribe() method for presence subscriptions, which I think should
> "win" (i.e. be the one that keeps the name) if necessary. So, the question here
> is: do we care about bindings that can't use colliding methods?
> 
> In particular, in dbus-python it's hard, although possible, to implement
> colliding methods - you have to have them in different subclasses, or mixins,
> or something.
> 
> I believe most/all *client* bindings are happy enough to *call* two methods of
> the same name, though?

That is a limitation I did not know before I wrote that API. D-Bus does not have any limitation in that sense since you always need to specify the interface name along with the method. But I forgot to consider that most OO langues simply add interfaces to the instance method table.

Because of that, I agree that generic names must be avoided (in interfaces), though I don't like the allusion to "win" a name, we should just not use them.

Any suggestion for renaming ?

> 
> (In reply to comment #27)
> > Nicolas pointed out on the bug for the Gabble implementation that the display
> > name in a Mail_Address can be empty. If this is the intention, the spec should
> > explicitly say so, to remind client authors that they might need to fall back
> > to displaying the email address.
> 
> As far as I can see, it still doesn't.

Right. Let me know what you think of http://git.collabora.co.uk/?p=user/nicolas/telepathy-spec.git;a=commitdiff;h=b320e559ea39dd9824a8b997704b7c36286f1aed;hp=fd72acdad39015aacf84be58174d1a12eb827389

> +        such as ("Nicolas Dufresne", "nicolas.dufresne@collabora.co.uk"). At
> +        least one of name and address MUST be provided. A missing element will
> +        be represented by the empty string.
> +        <tp:rationale>
> +          It is common for IM client to behave according to the UI design.
> +          Thus, some protocol may only provides one of name or address. Also,
> +          the display name is optional in an e-mail header. The goal here is
> +          to provide all available information with the highest accuracy. This
> +          way the client code does not have to do any guessing on what the
> +          address may contain.
> +        </tp:rationale>

Please wrap the main text in <p></p> when you add a second paragraph (tp:rationale always counts as a paragraph), and have <p></p> inside the <tp:rationale> too (to get nice spacing). To make this work properly you might need to add the XHTML xmlns to the <tp:docstring> (see any tp:docstring that contains XHTML for an example).

You don't need all that rationale (and I don't understand why you need to even mention a UI design). I'd just say something like this:

>        such as ("Nicolas Dufresne", "nicolas.dufresne@collabora.co.uk").
>        At least one of name and address MUST be provided. A missing element
>        will be represented by the empty string.</p>
>
>        <tp:rationale>
>          <p>The CM should provide as much information as possible, but not all
>            protocols provide both the displayed name and the address. (If a
>            protocol doesn't provide either, it should omit the appropriate
>            field from the <tp:type>Mail</tp:type> entirely.)</p>
>        </tp:rationale>

Done.

http://git.collabora.co.uk/?p=user/nicolas/telepathy-spec.git;a=commitdiff;h=8de3fd3dc1248ffe4ef47253a311338ba8b847dd;hp=fd72acdad39015aacf84be58174d1a12eb827389

Tell me when it's fine to merge.

Any suggestion for Subscribe/Unsubscribe() renaming ?

Cannot be undrafted before  bug 27835 "Avoid being notified about Connection features nobody cares about" is in place.

Not quite, technically you also needed:

-      <tp:docstring>
+      <tp:docstring xmlns="http://www.w3.org/1999/xhtml">

I've cherry-picked your patch into master (via a trivia branch) and added that change, in the interests of having one less branch hanging around :-)

(In reply to comment #33)
>  [...]
> Any suggestion for Subscribe/Unsubscribe() renaming ?

What about Enable/Disable ?

(In reply to comment #36)
> (In reply to comment #33)
> >  [...]
> > Any suggestion for Subscribe/Unsubscribe() renaming ?
> 
> What about Enable/Disable ?

We plan to solve this post-0.20 by merging the branch from Bug #27835 and using that feature instead.

I've just merged ClientInterests, so this can go back on the agenda.

Nicolas (or anyone else), do you have any further criticisms of this interface based on implementation experience, or do we think this is good to go?

(In reply to comment #38)
> Nicolas (or anyone else), do you have any further criticisms of this interface
> based on implementation experience, or do we think this is good to go?

I think this is good to go.

Undraft is blocked. On Bug #24901, wjt writes:
> One possible roadblock for re-using MailNotification [for voicemail
> as per Bug #24901]: GSM voicemail
> notifications need to be acked to the network. Maybe we could get away with
> Ring just acking them immediately, and caching them on disk, or something.
>
> Also: Rob wondered whether mail notification should be a singleton channel, to
> ensure that it's dispatched. Either that, or we need connection-observing API
> as Guillaume requests in bug 28974.

Either of these might require structural changes to MailNotification, depending how we resolve them :-(

(In reply to comment #40)
> Undraft is blocked. On Bug #24901, wjt writes:
> > One possible roadblock for re-using MailNotification [for voicemail
> > as per Bug #24901]: GSM voicemail
> > notifications need to be acked to the network. Maybe we could get away with
> > Ring just acking them immediately, and caching them on disk, or something.
From my point of view the voicemail is orthogonal to IM mail notification. The mail notification is lightweight notification, you have no guaranty to get them all and you usually have no mean to consult them all.

> >
> > Also: Rob wondered whether mail notification should be a singleton channel, to
> > ensure that it's dispatched. Either that, or we need connection-observing API
> > as Guillaume requests in bug 28974.
I don't agree, it's completely pointless to ensure notification dispatching. The way it works is very simple, read the state and stay inform. There exist no server that will resend notification or keep them offline until you are online. This would be a total waste of server resources.

I'm not advocating or blocking any particular design - I don't know what the best approach is here - but it seems worth delaying MailNotification a bit longer while we resolve whether or not it could be combined with voicemail.

(In reply to comment #41)
> From my point of view the voicemail is orthogonal to IM mail notification. The
> mail notification is lightweight notification, you have no guaranty to get them
> all and you usually have no mean to consult them all.

Putting that another way, is it true that your position is that new-mail notifications are a best-effort informational thing, like (say) presence or avatar changes, whereas voicemail notifications are expected to have reliable delivery, and so they're not similar enough to combine?

That'd mean we need to represent Voicemail elsewhere, as either a specially-formed Message on a magical Text channel (like the N900 does), a transient Channel per voicemail with its own type, or a singleton Channel with a Text-like queue.

On the other hand, if we want Voicemail to be the same as MailNotification, *and* we want the extra guarantees that Voicemail requires on GSM, then that suggests a third approach: represent both Voicemail and MailNotification as specially-formed Messages. This gives us more guarantees than we need for MailNotification, but that might not be such a big deal. On the other hand, it could lead to a confusing "everything is a Message, including the things that aren't" API.

> I don't agree, it's completely pointless to ensure notification dispatching.

The use-case for it would be if you wanted a UI like this:

* on boot, the mail notification widget isn't running at all
* when you connect to jabber.org (a server without integrated email), nothing happens
* when you connect to GTalk, the mail notification widget is automatically started, and notifies you about your new mail

(In reply to comment #42)
> The use-case for it would be if you wanted a UI like this:
> 
> * on boot, the mail notification widget isn't running at all
> * when you connect to jabber.org (a server without integrated email), nothing
> happens
> * when you connect to GTalk, the mail notification widget is automatically
> started, and notifies you about your new mail

Sounds like a perfect example of connection Observer / Monitor to me.

(In reply to comment #41)
> (In reply to comment #40)
> > Undraft is blocked. On Bug #24901, wjt writes:
> > > One possible roadblock for re-using MailNotification [for voicemail
> > > as per Bug #24901]: GSM voicemail
> > > notifications need to be acked to the network. Maybe we could get away with
> > > Ring just acking them immediately, and caching them on disk, or something.
> From my point of view the voicemail is orthogonal to IM mail notification. The
> mail notification is lightweight notification, you have no guaranty to get them
> all and you usually have no mean to consult them all.

I misunderstood: you don't have to ack GSM voicemail notifications.

> > > Also: Rob wondered whether mail notification should be a singleton channel, to
> > > ensure that it's dispatched. Either that, or we need connection-observing API
> > > as Guillaume requests in bug 28974.
> I don't agree, it's completely pointless to ensure notification dispatching.
> The way it works is very simple, read the state and stay inform. There exist no
> server that will resend notification or keep them offline until you are online.
> This would be a total waste of server resources.

True. Indeed GSM behaves like that: you only get the notification once, and you have to cache it locally if you want to save it.

So actually, this interface could be used for voicemail notifications without modification, only the addition of new keys.

Undrafted in 0.21.3. Thanks, everyone!