[Telepathy] [PATCH] TelepathyQt4: Various bugs + patches to fix them

[Andre Moreira Magalhaes]

Hey,

First thanks for the helping, below are some comments about the patches

George Kiagiadakis wrote:
> Hello everyone,
>
> I have been playing with Tp::AbstractClientApprover and 
> Tp::ChannelDispatchOperation this weekend in order to implement an approver 
> for my gsoc project (kcall) and I have found some bugs in the implementation 
> of those classes in TelepathyQt4. I am attaching some patches to fix them:
>
> Patch 0001 adds a finished() signal in ChannelDispatchOperation in order to 
> compy with the specification (I needed to use this signal, so I added it).
>
>   
We need to decide whether to use invalidated or finished here. We 
usually emit invalidated for DBusProxy objects that are no longer 
available (Channel.Clone, Connection.Disconnected, ...). I will discuss 
this with the other devs and see what is the best approach here, but 
yes, either way we need some mechanism to indicate the operation 
finished. Thanks for spotting this.


> Patch 0002 makes ClientApproverAdaptor use the full name of the Connection 
> property ("org.freedesktop.Telepathy.ChannelDispatchOperation.Connection") to 
> read the Connection object path from the properties that are passed to its 
> AddDispatchOperation method, in order to compy with the spec. Before I fix 
> that, it would crash a while after the channel dispatcher had called this 
> method, because the Connection object was assumed to be valid, although it 
> wasn't.
>
>   
This is wrong, the property _is_ Connection as stated on the spec:
|
Properties| − |a{sv}| (Qualified_Property_Value_Map 
<http://telepathy.freedesktop.org/spec.html#type-Qualified_Property_Value_Map>) 


    Properties of the channel dispatch operation. This MUST NOT include
    properties that could change, SHOULD include as many properties as
    possible given that constraint, and MUST include at least the
    Account
    <http://telepathy.freedesktop.org/spec.html#org.freedesktop.Telepathy.ChannelDispatchOperation.Account>,
    Connection
    <http://telepathy.freedesktop.org/spec.html#org.freedesktop.Telepathy.ChannelDispatchOperation.Connection>
    and PossibleHandlers
    <http://telepathy.freedesktop.org/spec.html#org.freedesktop.Telepathy.ChannelDispatchOperation.PossibleHandlers>
    properties. 

If your CD version is giving you a different property, it's wrong, and 
the CD must be fixed. What CD (MC) version are you using? We can discuss 
this on #telepathy at freenode if you prefer.

> Patch 0003 makes ChannelDispatchOperation read the "Channels" property from 
> the remote ChannelDispatchOperation object instead of reading 
> "ChannelDetailsLIst", which doesn't exist. This didn't crash before, but the 
> channels() method of ChannelDispatchOperation would return an empty list, 
> which is wrong.
>
>   
This is correct, good catch, I will apply this to my branch, and should 
be on next release.

Next time it would be great if you could use a git tree somewhere, it's 
easier to review the patches and to apply them.

Thanks again
BR
Andrunko