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.
can we just hide the close button in the plasmoid when the text-ui takes the channel?
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.
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
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.
Is this fixed?
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.
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.
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.
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?
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.
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
M +10 -1 KTp/Declarative/conversation.cpp
M +1 -2 KTp/Declarative/conversations-model.cpp