Bug 32874 - Offline contacts do not have any capabilities
Summary: Offline contacts do not have any capabilities
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/jo...
Whiteboard: NB#209774 review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-01-06 06:22 UTC by Xavier Claessens
Modified: 2011-02-03 00:51 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xavier Claessens 2011-01-06 06:22:34 UTC
When calling GetContactAttributes() with an offline contact and asking for org.freedesktop.Telepathy.Connection.Interface.ContactCapabilities, gabble's reply omit the capabilities of that contact.

Offline contacts should still have the text capability.
Comment 1 Jonny Lamb 2011-01-10 07:52:39 UTC
CZECH IT OUT!
Comment 2 Xavier Claessens 2011-01-10 08:19:40 UTC
+      DEBUG ("don't know %u's presence; assuming text chat caps.", handle);

I don't like telling you assume text caps there as it's a decision of im_factory. So if we change there, likely that debug will become wrong.

It is not unlikely that at some point Call cap will be added for offline contacts, for voicemail. AFAIK Skype does it already.

But that's just my 2cents, I don't know much that gabble code.
Comment 3 Jonny Lamb 2011-01-10 08:26:02 UTC
(In reply to comment #2)
> +      DEBUG ("don't know %u's presence; assuming text chat caps.", handle);
> 
> I don't like telling you assume text caps there as it's a decision of
> im_factory. So if we change there, likely that debug will become wrong.

Well, right, I agree -- if the IM factory behaviour changes, this will break, which is why I added the big NOTE in that function. I also think the test is strict enough to only allow the text caps, isn't it?

> It is not unlikely that at some point Call cap will be added for offline
> contacts, for voicemail. AFAIK Skype does it already.

Sure. At that point of course we'll have to change this behaviour, but it's not so bad now, is it? Perhaps we could add a GabbleCapsChannelManagerClass->get_assumed_caps vfunc so we can iterate through the channel managers and get text caps from im-factory and Call caps from call-factory, but that can be done later, no?
Comment 4 Will Thompson 2011-01-12 11:04:13 UTC
I think the implementation is functionally fine. But here's a bunch of trivia.

+  /* IM factory */
+  GabbleImFactory *im_factory;

This comment does not add anything except unnecessary scrolling.

+"""
+Test for fd.o#32874
+"""

I'm offline on a train. What was that bug?

+    # new ContactCapabilities
+    ccaps_map = conn.ContactCapabilities.GetContactCapabilities([bob_handle])
+    assertEquals(1, len(ccaps_map))
+
+    assertEquals(1, len(ccaps_map[bob_handle]))

assertLength

+    assertSameSets({cs.CHANNEL_TYPE: cs.CHANNEL_TYPE_TEXT,
+                    cs.TARGET_HANDLE_TYPE: cs.HT_CONTACT}, fixed)

assertEquals

+    print all_caps

No.

I'm upset that there's no convenience function to expect a roster query and send a reply, but I won't hold it against you.
Comment 5 Jonny Lamb 2011-01-13 02:19:18 UTC
(In reply to comment #4)
> +  /* IM factory */
> +  GabbleImFactory *im_factory;
> 
> This comment does not add anything except unnecessary scrolling.

I've added a comment.

> +"""
> +Test for fd.o#32874
> +"""
> 
> I'm offline on a train. What was that bug?

I've added yet another comment.

> assertLength
...
> assertEquals
...
> +    print all_caps
> 
> No.

All three fixed.
Comment 6 Will Thompson 2011-02-02 08:25:44 UTC
hooray!
Comment 7 Jonny Lamb 2011-02-03 00:51:46 UTC
Awesome.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.