289135 - Close Tab does not cancel re-connection attempts

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, enhancement)

Description

[uberbone]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1

When you try to connect to a non-existent irc network, typo usually... and Close
Tab ... the tab will re-open on each re-try, unless you type /cancel. It will do
this upto and including the "giving up" message.



Reproducible: Always

Steps to Reproduce:
1. /attach mispelled or non-existent irc network
2. On notification, Close Tab
3. Do this each time it re-opens the tab
4. type /cancel and the problem goes away

Actual Results:  
The Tab re-opens for each of the 5 re-tries and also the "giving up" message

Expected Results:  
Close Tab should /cancel re-tries.

Comment 1

[:Gijs (he/him)]

Confirming.

Comment 2

[Rob Marshall [tH]]

Attachment: patch

This makes /delete-view cancel the connection attempt, but only close the view
after the "Connection process canceled" message is shown. It also fixes a
similar issue with the "You have left" message re-opening a closed channel
view.

Comment 3

[James Ross]

Comment on attachment 190545 [details] [diff] [review]
patch

>Index: extensions/irc/xul/content/commands.js

>     if (e.view.TYPE == "IRCChannel" && e.view.active)
>-        e.view.part();
>+        e.view.dispatch("part", { deleteWhenDone: true });
>     if (e.view.TYPE == "IRCDCCChat" && e.view.active)
>         e.view.disconnect();
>+    if (e.view.TYPE == "IRCNetwork" && (e.view.state == NET_CONNECTING || 
>+                                        e.view.state == NET_WAITING))
>+        e.view.dispatch("cancel", { deleteWhenDone: true });

I don't think this is going to work very well. As far as I can see, it
dispatches the /part of /cancel, then continues through and deletes the view.
Then the view will get recreated, and deleted again when the part or cancel
finish.

Comment 4

[:Gijs (he/him)]

Attachment: [checked in] New Patch

The aim of this patch was to make the previous patch actually work correct, by not deleting the view if we part or cancel something.

When doing this, several things came to my attention
- onDisconnect could spew one or more error messages when cancelling, because ChatZilla would try to read or write from the connection that had been closed by that time. This was fixed by not reading or writing to or from the streams if we were cancelling.
- onDisconnect would also fire when the onStreamClose event occurred, still spewing error messages. To fix this, I stopped it from displaying anything if we're cancelling the connection.
- When you /cancel-ed *after* onDoConnect had fired, but *before* ChatZilla received the 001 message, /cancel had no effect. Fixing this involved adding an extra case to network.cancel so we do the Right Thing (tm), and throw an onError ourselves after terminating the connection.

I'm pleased with the result, which works as far as I can tell. Requesting review.

Comment 5

[James Ross]

Comment on attachment 204574 [details] [diff] [review]
[checked in] New Patch

I think you need to also update the syntax for the /part or /leave command with the new parameter name. With that, r=silver

Comment 6

[:Gijs (he/him)]

Attachment: Patch with altered help text

The help text needed updating - you can only specify the parameter from script, so it was not in the syntax/usage. Anyhow, same patch as above, but with a change in cz.properties as well.

Comment 7

[:Gijs (he/him)]

Attachment: Patch without careless replacing

Sigh. deleteWhenDone does the opposite of what noDelete used to do. And I'd better use command parameter syntax.

Comment 8

[:Gijs (he/him)]

Attachment: [checked in] More nits to the help text.

Comment 9

[:Gijs (he/him)]

This should now be fixed. Yay. :-)