After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 599164 - Should use ContactCapability to get FT caps
Should use ContactCapability to get FT caps
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: File Transfer
2.28.x
Other Linux
: Normal enhancement
: ---
Assigned To: empathy-maint
Depends on:
Blocks: 599163
 
 
Reported: 2009-10-21 10:53 UTC by Guillaume Desmottes
Modified: 2010-01-14 13:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/ft-caps-599164 (6.01 KB, patch)
2010-01-13 14:24 UTC, Guillaume Desmottes
reviewed Details | Review

Description Guillaume Desmottes 2009-10-21 10:53:36 UTC
Empathy should use the new ContactCapability API to check if contacts are able to receive files.
Comment 1 Guillaume Desmottes 2009-10-21 10:57:51 UTC
This is blocked by Salut not implementing the ContactCapability stable API: https://bugs.freedesktop.org/show_bug.cgi?id=24653
Comment 2 Guillaume Desmottes 2010-01-13 14:24:08 UTC
Created attachment 151340 [details] [review]
http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/ft-caps-599164

 libempathy/empathy-tp-contact-factory.c |  129 ++++++++++++++++++++++++++++---
 1 files changed, 119 insertions(+), 10 deletions(-)
Comment 3 Cosimo Cecchi 2010-01-14 12:13:02 UTC
Review of attachment 151340 [details] [review]:

The code itself looks fine, but I think you should try to factor out the parsing of the ContactCapabilities hash table together with the parsing of the RequestableChannelClasses in get_requestable_channel_classes_cb() (the tp-spec actively encourages this).

Also, in the same callback, it's probably useless to cycle through the classes if the connection supports ContactCapabilities (the cycle seems to be used only to set priv->can_request_ft and priv->can_request_st which are unused if ContactCapabilities are supported).
Comment 4 Guillaume Desmottes 2010-01-14 13:01:10 UTC
(In reply to comment #3)
> Review of attachment 151340 [details] [review]:
> 
> The code itself looks fine, but I think you should try to factor out the
> parsing of the ContactCapabilities hash table together with the parsing of the
> RequestableChannelClasses in get_requestable_channel_classes_cb() (the tp-spec
> actively encourages this).

Good point! Done.

> Also, in the same callback, it's probably useless to cycle through the classes
> if the connection supports ContactCapabilities (the cycle seems to be used only
> to set priv->can_request_ft and priv->can_request_st which are unused if
> ContactCapabilities are supported).

Indeed. Actually we don't need to request the channel classes at all.

I've done these changes in my branch fixing bug #599163 as well which is a super set of the branch you reviewed. This saves me lot of conflicts.
http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/av-caps-599163
Comment 5 Cosimo Cecchi 2010-01-14 13:47:24 UTC
The branch looks good to me.
Comment 6 Guillaume Desmottes 2010-01-14 13:56:51 UTC
Awesome; thanks a lot.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.