475387 - irc://url.server.org (hostname) opens in a new tab when server is in "available networks" list

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[worms]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: 

When a network or server is in the list of available networks, then you get lines like:

Connecting to irc://moznet/ (irc://irc.mozilla.org/)… [Cancel]
Disconnected from irc://moznet/ (irc://irc.mozilla.org/). [Reconnect]

which contain two or three links.

Clicking on the first and thirds one will reconnect in the same chatzilla tab, whereas clicking on the second one will open a new chatzilla tab.

Is this the expected behaviour?

When you connect to a network which isn't in the list of available networks, say for example: irc.spidernet.org, you get:

Connecting to irc://irc.spidernet.org/ (irc://irc.spidernet.org/)… [Cancel]
Disconnected from irc://irc.spidernet.org/ (irc://irc.spidernet.org/). [Reconnect]

and all links will reconnect in the same tab, which, I believe, in the correct behaviour.

Reproducible: Always

Steps to Reproduce:
1. Connect to a network which is displayed in "available networks"
2. Cancel the connection or Disconnect, to get the lines with the network url
3. Click on the second proposed link (the hostname?)
Actual Results:  
A new tab opens called "the hostname"

Expected Results:  
Reconnection happens in the same (original) tab

Sugnim++

Comment 1

[Glen Mailer]

I suppose its reasonable to have at least the servers we're aware of mapped back onto the network names.

Comment 2

[:Gijs (he/him)]

I don't really like the idea of a full reverse map, as this could be quite costly if you have a long list of networks. We could probably just do with mapping the connected ones, would that work for people?

Comment 3

[James Ross]

In the example in comment 0, the network is not connected, so no, it wouldn't work. :)

Comment 4

[:Gijs (he/him)]

(In reply to comment #3)
> In the example in comment 0, the network is not connected, so no, it wouldn't
> work. :)

Right. I was not precise enough: keep a list of every network we have a view or connection for. The easiest way to do this would probably be to just save it on the first connection attempt. Which would cover comment 0, afaict, and although it would also include things for which both views and connections have been closed, I don't think that would generally be a problem.

Comment 5

[James Ross]

A naive search of 337 networks (http://users.skynet.be/fa258499/networks.txt) takes 3-4ms (for a non-existent target) here, and it always registers 0ms on my normal ChatZilla. We can live with the speed here.

Comment 6

[James Ross]

Attachment: Move some magic into gotoIRCURL and organise processing

This fixed bug 520332 too. Woo.

Comment 7

[Tony Mechelynck [:tonymec]]

In reply to comment #0
also, after connecting to the same network, once as irc://moznet/ and once as irc://irc.mozilla.org/ (not to mention ircs://irc.mozilla.org:6697/), I get separate sets of preferences (with separate channel preferences too) for each of them in the Chatzilla preferences. I suppose the _creation_ of distinct lists of preferences is the result of this bug, but merging them once this bug is fixed may require a lot of handiwork.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.7pre) Gecko/20091208 SeaMonkey/2.0.2pre - Build ID: 20091208000514
ChatZilla 0.9.85

Comment 8

[:Gijs (he/him)]

Comment on attachment 416259 [details] [diff] [review]
Move some magic into gotoIRCURL and organise processing

>diff --git a/xul/content/static.js b/xul/content/static.js
>--- a/xul/content/static.js
>+++ b/xul/content/static.js
>@@ -1899,104 +1899,96 @@ function gotoIRCURL(url, e)
> <snip>
>+    if (network.temporary)
>+        network.serverList[0].isSecure = url.scheme == "ircs";
>+
>+    // Adjust password for all networks (so user can override).
Nit: for all servers

>+    if (url.pass)
>+    {
>+        for (var s in network.servers)
>+            network.servers[s].password = url.pass;
>+    }
> <snip>

With the nit, r=me.

Comment 9

[James Ross]

Attachment: Swap needpass and server.password update around

I noticed this while writing the commit message (so I didn't commit); the original patch would update all the servers' passwords BEFORE prompting for needpass, which means needpass wouldn't work at all. This update includes the nit, and fixes this needpass issue by just swapping the two blocks, which were next to each other already (I kept the two "adjust" bits together too).

Comment 10

[:Gijs (he/him)]

Comment on attachment 416673 [details] [diff] [review]
Swap needpass and server.password update around

Good catch, r-me

Comment 11

[James Ross]

Pushed --> FIXED.

Comment 12

[Tony Mechelynck [:tonymec]]

See bug 536016 which I believe is a result of this "fix".