Review Request 123938: Don't add send delivery messages in the messages vector

Alexandr Akulich akulichalexander at gmail.com
Mon Jun 1 12:49:17 UTC 2015



> On May 31, 2015, 2:02 a.m., Alexandr Akulich wrote:
> > Hello.
> > I know about the issue for a long time, but did not figured out the root of problem.
> > You're right. :) I just checked it, specs says: "Incoming messages and delivery reports are signalled by MessageReceived". I'll fix it tomorrow.
> > Next time you'll find a mistake (or some unclear moment) in CM, please let me know. :)
> 
> Alexandr Akulich wrote:
>     Fixed in commit http://commits.kde.org/telepathy-morse/4161790d13048ed264853197e4a2b92aeb2ccf95 . Thanks.
> 
> Aleix Pol Gonzalez wrote:
>     Thank you very much Alexandr! :)
>     
>     Now what do we do with this patch?
> 
> Martin Klapetek wrote:
>     I think delivery messages should stay in the model, they can sometimes contain useful data from server when the delivery fails for example.
> 
> Aleix Pol Gonzalez wrote:
>     Hi Martin,
>     That's not what this patch is about, please re-read.
> 
> Martin Klapetek wrote:
>     Well can you then explain what the patch is about now when the source of the bug was fixed?

All incoming delivery reports must go through Channel.Interface.Messages MessageReceived signal.
All outgoing delivery reports must through Channel.Type.Text AcknowledgePendingMessages method with Channel.Interface.Messages PendingMessagesRemoved signal emission (who came up with idea to split method and signal into different interfaces O_o? ).

This way, the check, added to onMessageSent(), looks like sane protection against buggy CM (Hm, how they would be known to be buggy otherwise?).
The second part is not really correct. The good thing to show is delivery-error-message key. (http://telepathy.freedesktop.org/spec/Channel_Interface_Messages.html#Simple-Type:Delivery_Report_Header_Key)


- Alexandr


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123938/#review80984
-----------------------------------------------------------


On May 29, 2015, 10:58 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123938/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 10:58 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> -------
> 
> Currently, when using Telegram, we keep getting empty lines, this skips them.
> 
> OTOH, it _really_ looks like a bug in the CM, as these messages should go through the received messages rather than the sent, nevertheless I'm unsure if the patch should still go in.
> 
> 
> Diffs
> -----
> 
>   KTp/Declarative/messages-model.cpp dc1088c 
> 
> Diff: https://git.reviewboard.kde.org/r/123938/diff/
> 
> 
> Testing
> -------
> 
> The chat plasmoid works fine.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20150601/fe4a8575/attachment-0001.html>


More information about the KDE-Telepathy mailing list