Bug 293637 - Plasmoid disconnects the channel from the close button
Summary: Plasmoid disconnects the channel from the close button
Status: RESOLVED FIXED
Alias: None
Product: telepathy
Classification: Unclassified
Component: chat-plasmoid (show other bugs)
Version: unspecified
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: 0.6-next
Assignee: David Edmundson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-08 13:52 UTC by David Edmundson
Modified: 2013-04-10 12:20 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Edmundson 2012-02-08 13:52:28 UTC
I know I reviewed this code, turns out I'm an idiot.

An observer should never disconnect a channel. It doesn't just disconnect the observer, it closes the channel, making the Text UI be in a completely broken state.

Besides, there's no reason to do it by hand anyway, simply delete all references to the text channel and it will happen (in a much safer way) automatically.
Comment 1 Daniele E. Domenichelli 2012-04-17 15:30:08 UTC
can we just hide the close button in the plasmoid when the text-ui takes the channel?
Comment 2 David Edmundson 2012-04-18 17:02:21 UTC
No, I don't think there's any reason to ever call disconnect. 
We should just drop our reference to the channel. 

MC will disconnect if no-one else is dealing with it. Job done.

This is also the cause of missing messages in the text-ui.
Comment 3 David Edmundson 2012-04-19 13:20:43 UTC
Git commit 9dbdd3858227f6d1004faa7370284775c306d640 by David Edmundson.
Committed on 19/04/2012 at 15:16.
Pushed by davidedmundson into branch 'master'.

Mark the channel as disconnected when the channel disconnects, instead of watching the connection status
Related: bug 298023

Reviewed-By: Dominik Cermak

M  +1    -1    lib/chat-widget.cpp

http://commits.kde.org/telepathy-text-ui/9dbdd3858227f6d1004faa7370284775c306d640
Comment 4 David Edmundson 2012-05-17 11:03:37 UTC
Turns out I'm wrong. Dropping the final ref doesn't close the channel, and we should be calling "requestClose()"... however only when we're the handler not the approver.

Which means we need to keep track of that.
I may solve with a hack for now.
Comment 5 Daniele E. Domenichelli 2012-09-24 09:04:58 UTC
Is this fixed?
Comment 6 David Edmundson 2012-09-24 09:41:34 UTC
No. 

Tbh I'm waiting for https://bugs.freedesktop.org/show_bug.cgi?id=43594
when it will be easy to tell if we're handling the channel or not. Right now the delegateChannel call is a nasty hack.
Comment 7 David Edmundson 2013-03-17 00:26:12 UTC
I think this bug is no longer relevant.

Chat closes when the channel is delegated.
Therefore it can never be in the state where we think we are handling the channel when we are actually only the observer.
Comment 8 Aleix Pol 2013-03-22 01:45:12 UTC
It works here, so I'd say it's fixed. Here's what i tried:
- start chat on the plasmoid
- open the conversation with the same contact in text-ui

There I could follow the conversation from both clients, so I'd seems to work great.
Comment 9 David Edmundson 2013-03-22 08:09:36 UTC
If you're getting the conversation in both clients it doesn't sound like it will have been fixed.

Basically if you run qdbus org.freedesktop.Telepathy.Client.KDE.TextUi.ConversationWatcher org/freedesktop/Telepathy/Client/KDE/TextUI/ConversationWAtcher org.freedstkop.Telepathy.Client.Handler HandledChannels it should _NOT_ list the channel that has just been delegated.

As it leaves you with a bug that if you close the plasmoid you kill the chat that's in the text-ui. Which is wrong. You also have problems if MC crashes and restarts. 

I tried testing and I'm getting an even bigger issue. When I fixed it so the plasmoid closes the channel on exit, when I delegate to the text-ui it's killing the channel instantly and everything appears to be broken. Are you running with commit 642b552331ed5ef63064efe671ac6a214f2bf755?
Comment 10 David Edmundson 2013-03-22 08:14:24 UTC
Managed to test. Still broken :(

If you click on the "open in text-ui" button then it closes the chat so this would be irrelevant.
If you start a chat from the contact list, the plasmoid delegates (partly) so you get the chat in two places.

Chat plasmoid still thinks it's handling it.. even though the text-ui does too, so this bug is still relevant. I'll remove it from being "major" as it's not causing a lot of problems.
Comment 11 David Edmundson 2013-04-10 12:20:49 UTC
Git commit f3a30eb36e174a38ca95fd1b7d2ea6a6c17fa281 by David Edmundson.
Committed on 09/04/2013 at 14:40.
Pushed by davidedmundson into branch 'kde-telepathy-0.6'.

Make sure ConversationModel never close a channels it is not handling
Related: bug 317850
REVIEW: 109925

M  +10   -1    KTp/Declarative/conversation.cpp
M  +1    -2    KTp/Declarative/conversations-model.cpp

http://commits.kde.org/telepathy-common-internals/f3a30eb36e174a38ca95fd1b7d2ea6a6c17fa281