Bug 28918 - NewChannels fired twice when connecting to a bip account
Summary: NewChannels fired twice when connecting to a bip account
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: idle (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-05 03:55 UTC by Guillaume Desmottes
Modified: 2011-05-03 10:02 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Do not emit an extra NewChannels while handling a JOIN message (965 bytes, patch)
2011-04-18 17:45 UTC, Debarshi Ray
Details | Splinter Review
0002 Test to cover MUC JOIN initiated by the server (2.87 KB, patch)
2011-04-22 05:09 UTC, Debarshi Ray
Details | Splinter Review
0003 Strengthen the MUC JOIN test to forbid duplicate NewChannels (1.47 KB, patch)
2011-04-22 05:19 UTC, Debarshi Ray
Details | Splinter Review
Handle PART messages in our test IRC server (900 bytes, patch)
2011-05-02 16:10 UTC, Debarshi Ray
Details | Splinter Review
0002 Test to cover MUC JOIN initiated by the server (3.59 KB, patch)
2011-05-02 16:12 UTC, Debarshi Ray
Details | Splinter Review

Description Guillaume Desmottes 2010-07-05 03:55:33 UTC
Each time I connect to my bip account (which is connected to one channel) NewChannels is fired twice, making MC calling ObserveChannels() twice for the same channel.
Comment 1 Guillaume Desmottes 2010-07-05 04:00:57 UTC
tp_channel_manager_emit_new_channel is fired twice:

- In _join_handler() when we receive the JOIN command from bip
- In _channel_join_ready_cb()  when we actually joined the room

I guess we shouldn't emit the first NewChannels signal but I don't know enough of Idle/IRC to be sure that's the proprer fix.
Comment 2 Debarshi Ray 2011-04-18 13:37:46 UTC
The same thing happens when you get invited to a channel that you are not already a part of.

Basically whenever a JOIN message is received for a channel that you did not attempt to join yourself.
Comment 3 Debarshi Ray 2011-04-18 13:58:01 UTC
(In reply to comment #2)

> Basically whenever a JOIN message is received for a channel that you did not
> attempt to join yourself.

Oops, sorry! What was I thinking? Here is the correct analysis.

Whenever we try to handle an INVITE or a JOIN for which there is no channel, we do (as Guillaume wrote):
+ _muc_manager_new_channel
+ tp_channel_manager_emit_new_channel (emits NewChannels)

After that, in case of JOIN, irrespective of whether there was a channel or not, we do:
+ idle_muc_channel_join (emits NewChannels)

And in case of INVITE, we try a normal join operation if the user wants to, which leads to a NewChannels as well.

I am not sure about the behaviour in case of an INVITE, but for the _join_handler, the easy way out is to not call tp_channel_manager_emit_new_channel.

However, I am told that older versions of Bitlbee (http://www.bitlbee.org/main.php/news.r.html) does not want the client to leave the &bitlbee channel and sends a JOIN to the client whenever it does so. I don't know if this is a correct use of JOIN and since newer Bitlbee versions don't do so, I am not sure if we should support this.
Comment 4 Debarshi Ray 2011-04-18 17:45:04 UTC
Created attachment 45793 [details] [review]
Do not emit an extra NewChannels while handling a JOIN message
Comment 5 Debarshi Ray 2011-04-22 05:09:56 UTC
Created attachment 45949 [details] [review]
0002 Test to cover MUC JOIN initiated by the server
Comment 6 Debarshi Ray 2011-04-22 05:19:44 UTC
Created attachment 45950 [details] [review]
0003 Strengthen the MUC JOIN test to forbid duplicate NewChannels

Although not directly related to this, it might be a good idea to improve the existing channels/join-muc-channel.py to forbid duplicate NewChannels.
Comment 7 Will Thompson 2011-04-29 05:46:12 UTC
Review of attachment 45950 [details] [review]:

Looks good.
Comment 8 Will Thompson 2011-04-29 05:47:26 UTC
Review of attachment 45949 [details] [review]:

Could the test also check that PARTing a room, and having the server echo the PART followed immediately by JOINing you again, works?

::: tests/twisted/channels/join-muc-channel-bouncer.py
@@ +3,3 @@
+Test connecting to a IRC channel
+"""
+

I think this comment needs updating to match the test! :)
Comment 9 Will Thompson 2011-04-29 05:47:43 UTC
Review of attachment 45793 [details] [review]:

If all the tests pass, including your new one, then this looks fine!
Comment 10 Debarshi Ray 2011-05-02 16:10:43 UTC
Created attachment 46264 [details] [review]
Handle PART messages in our test IRC server
Comment 11 Debarshi Ray 2011-05-02 16:11:30 UTC
(In reply to comment #8)
> Review of attachment 45949 [details] [review]:
> 
> Could the test also check that PARTing a room, and having the server echo the
> PART followed immediately by JOINing you again, works?

Done.

> ::: tests/twisted/channels/join-muc-channel-bouncer.py
> @@ +3,3 @@
> +Test connecting to a IRC channel
> +"""
> +
> 
> I think this comment needs updating to match the test! :)

Done.
Comment 12 Debarshi Ray 2011-05-02 16:12:23 UTC
Created attachment 46265 [details] [review]
0002 Test to cover MUC JOIN initiated by the server
Comment 13 Debarshi Ray 2011-05-03 10:02:39 UTC
Committed and pushed.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.