Bug 29377

When implementing this, remember to add the three new immutable properties. (See Bug #30950.)

Any chance to have this soonish? One of our goal for Empathy 3.0 is to switch to Messages, but we can't easily do it until Messages became mandatory (bug #29376).

AFAIK, sofiasip is the last CM not implementing it.
If no one else has time to do it I can give it a try but I never hacked on tp-sofiasip.

(In reply to comment #2)
> AFAIK, sofiasip is the last CM not implementing it.
> If no one else has time to do it I can give it a try but I never hacked on
> tp-sofiasip.

Feel free to go ahead. The text channel code in telepathy-sofiasip is rather basic.
While you are at it, you could try supporting arbitrary MIME types on body payloads, or at least non-multipart cases. But this is completely optional.

Here is a first version:
http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=shortlog;h=refs/heads/messages-29377

I wasn't able to properly test it because for some reason my SIP provider (voipbuster) refuses to send messages.


test-message.py doesn't pass any more because ListPendingMessages() now returns a delivery report. Is that expected? Should I only create *fail* delivery report when TP_MESSAGE_SENDING_FLAG_REPORT_DELIVERY is set? But then SendError won't be fired...

(In reply to comment #4)
> test-message.py doesn't pass any more because ListPendingMessages() now returns
> a delivery report. Is that expected?

That sounds okay to me.

> Should I only create *fail* delivery
> report when TP_MESSAGE_SENDING_FLAG_REPORT_DELIVERY is set? But then SendError
> won't be fired...

No, you're allowed to signal delivery reports that weren't requested; the flags are really for the cases where you have to do extra work to get the reports.

(In reply to comment #4)
> Here is a first version:
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=shortlog;h=refs/heads/messages-29377

+  if (content_type == NULL || tp_strdiff (content_type, "text/plain"))

Do we get the "content-type" value normalized? If not, this should be a case-insensitive check.

I should perhaps add extraction of "message-sent" and an appropriate unique token from SIP headers, in a follow-up change.

http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=a5a7b7909152cde4d7f575ab88ddb41fb3cb3fb5

This part looks fine, except I can't see where the delivery token is exposed in the sent message.

(In reply to comment #5)
> (In reply to comment #4)
> > test-message.py doesn't pass any more because ListPendingMessages() now returns
> > a delivery report. Is that expected?
> 
> That sounds okay to me.

Cool, I changed the test then.


(In reply to comment #6)
> (In reply to comment #4)
> > Here is a first version:
> > http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=shortlog;h=refs/heads/messages-29377
> 
> +  if (content_type == NULL || tp_strdiff (content_type, "text/plain"))
> 
> Do we get the "content-type" value normalized? If not, this should be a
> case-insensitive check.

I think so; that's how it's done in all the other CM.

> I should perhaps add extraction of "message-sent" and an appropriate unique
> token from SIP headers, in a follow-up change.

Please feel free :)


(In reply to comment #7)
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=a5a7b7909152cde4d7f575ab88ddb41fb3cb3fb5
> 
> This part looks fine, except I can't see where the delivery token is exposed in
> the sent message.

That's the 4th argument of tp_message_mixin_sent(). My patch was wrongly recorded; it should be clearer now:
http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=13285043020ed6bdaa72b30bf65ac9b7d6b0207e

(In reply to comment #8)
> > Do we get the "content-type" value normalized? If not, this should be a
> > case-insensitive check.
> 
> I think so; that's how it's done in all the other CM.

The specification does not demand it. Filed as bug #31170.

> > This part looks fine, except I can't see where the delivery token is exposed in
> > the sent message.
> 
> That's the 4th argument of tp_message_mixin_sent(). My patch was wrongly
> recorded; it should be clearer now:
> http://git.collabora.co.uk/?p=user/cassidy/telepathy-sofiasip;a=commitdiff;h=13285043020ed6bdaa72b30bf65ac9b7d6b0207e

OK.

Did you review the whole branch? Can I merge it?

(In reply to comment #10)
> Did you review the whole branch? Can I merge it?

Yes; the only remaining issue (bug #31170) requires a spec decision, so it can be addressed later.

Merged to master; thanks!

For the record, this will be in 0.7.0. Mikhail, is there anything blocking a 0.7.0 release?

(In reply to comment #13)
> For the record, this will be in 0.7.0. Mikhail, is there anything blocking a
> 0.7.0 release?

I'm going to fix #30538 so we can release something reasonably compliant with the current specification.