ChatZilla needs /reconnect, /reconnect-all, /disconnect-all and /rejoin

RESOLVED FIXED

Status

--
enhancement
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Gijs, Assigned: rginda)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: 

ChatZilla would like to have a /reconnect, /reconnect-all and /disconnect-all
command.

Reproducible: Always

Steps to Reproduce:
Use any of the above commands.

Actual Results:  
Nothing.

Expected Results:  
ChatZilla should have reconnected, reconnected to all the servers or
disconnected from all the servers, respectively.
(Reporter)

Comment 1

14 years ago
Created attachment 174218 [details] [diff] [review]
Patch includes: reconnect, reconnect-all, disconnect-all, rejoin, bug fix for 'part' and 'leave', confirmEx function in utils.js

As I had the time for it I took the liberty of doing the following:
a) Add the /reconnect, /reconnect-all, /disconnect-all, /rejoin commands.
b) Add a confirmEx function to utils.js
c) Fix the issue where using a one-word part message with /part or /leave
appends 'null' to the part message.
d) Add the above commands to appropriate context menu's.

Requesting review from Silver, who helped me get on my way with this bug.
Attachment #174218 - Flags: review?(silver)
(Reporter)

Comment 2

14 years ago
Updating summary to suit patch.
Summary: ChatZilla needs /reconnect, /reconnect-all and /disconnect-all → ChatZilla needs /reconnect, /reconnect-all, /disconnect-all and /rejoin
(Reporter)

Comment 3

14 years ago
Created attachment 174248 [details] [diff] [review]
replacement fixing menu order, CIRCChannel.part and some other stuff

Patch based on Silver's comments in #chatzilla.
Attachment #174218 - Attachment is obsolete: true
Attachment #174248 - Flags: review?(silver)
(Reporter)

Updated

14 years ago
Attachment #174218 - Flags: review?(silver)
(Reporter)

Comment 4

14 years ago
Created attachment 174745 [details] [diff] [review]
Patch with better confirmEx, better rejoin requirements

Fixed the issue with rejoin (it did not require a connected server, and
therefor could be executed without any chance of it actually working) and
improved confirmEx. Changes suggested by Silver, I hope I did it right this
time :-).
Attachment #174248 - Attachment is obsolete: true
Attachment #174745 - Flags: review?(silver)
(Reporter)

Updated

14 years ago
Attachment #174248 - Flags: review?(silver)

Comment 5

14 years ago
Comment on attachment 174745 [details] [diff] [review]
Patch with better confirmEx, better rejoin requirements

These comments are short and sweet because Bugzilla has already lost my nice
long comments once and I REALLY can't be arsed trying to re-do them.

>Index: mozilla/extensions/irc/js/lib/irc.js
>+    // this.quitting signals the disconnect was intended
>+    e.quitting = this.quitting;

This comment is un-nessessary. It should be somewhere completely different, see
below.


>Index: mozilla/extensions/irc/js/lib/utils.js
>+function confirmEx(msg, btnAry, defaultBtn, checkTxt, checkVal, parent, title)
>+{
>+    /* Note that on versions before Mozilla 0.9, using 3 buttons,
>+     * the revert or dontsave button, or custom button titles will NOT work.
>+     *
>+     * The buttons should be listed in the 'accept', 'cancel' and 'extra' order,
>+     * and the exact button order is host app- and platform-dependant.
>+     * For example, on Windows, this is usually [button 1] [button 3] [button 2],
>+     * and on Linux [button 3] [button 2] [button 1]. */

Put the comment end marker on its own line.
  /*
   *
   */

>+    var PROMPT_CTRID = "@mozilla.org/embedcomp/prompt-service;1";
>+    var nsIPromptService = Components.interfaces.nsIPromptService;
>+    var ps = Components.classes[PROMPT_CTRID].getService(nsIPromptService);

    const ps = getService("@mozilla.org/embedcomp/prompt-service;1",
							   
"nsIPromptService");

>+    var flags = {
>+        ok: ps.BUTTON_TITLE_OK,
>+        cancel: ps.BUTTON_TITLE_CANCEL,
>+        yes: ps.BUTTON_TITLE_YES,
>+        no: ps.BUTTON_TITLE_NO,
>+        save: ps.BUTTON_TITLE_SAVE,
>+        revert: ps.BUTTON_TITLE_REVERT,
>+        dontsave: ps.BUTTON_TITLE_DONT_SAVE
>+    };

Many variables have bad names.
    flags      --> buttonConstants
    btnAry     --> buttons
    defaultBtn --> default/defaultButton
    checkTxt   --> checkText
    btnVal     --> buttonFlags
    btnText    --> buttonLabels

>+    var btnVal = 0;
>+    var btnText = [null, null, null];
>+
>+    if (!isinstance(btnAry, Array) || btnAry.length < 1 || btnAry.length > 3)

Parens please.

>+        throw "btnAry parameter must be an Array with 1-3 elements";
>+
>+    for (var i = 0; i < btnAry.length; i++)
>+    {
>+        if (btnAry[i].charAt(0) == "!" && btnAry[i].substr(1) in flags)

Parens again.

>+        {
>+            btnVal += ps["BUTTON_POS_" + i] * flags[btnAry[i].substr(1)];
>+        }
>+        else if (btnAry[i] != "")
>+        {
>+            btnVal += ps["BUTTON_POS_" + i] * ps.BUTTON_TITLE_IS_STRING;
>+            btnText[i] = btnAry[i];
>+        }

Why only if and else-if? else-if should be else.

>+    }
>+
>+    // ignore anything but a proper number
>+    if (typeof defaultBtn == "number" && arrayHasElementAt(btnAry, defaultBtn))

Parens.

>+        btnVal += ps["BUTTON_POS_DEFAULT_" + defaultBtn];
>+
>+    if (!parent)
>+        parent = window;
>+    if (!title)
>+        title = MSG_CONFIRM;
>+

Kill.

>+    if (!checkVal)
>+        checkVal = new Object();
>+
>+    rv = ps.confirmEx(parent, title, msg, btnVal, btnText[0], btnText[1],
>+                      btnText[2], checkTxt, checkVal);
>+    return rv;
>+}


>Index: mozilla/extensions/irc/xul/content/commands.js
>+function cmdDisconnectAll(e)
>+{
>+    if (confirmEx(MSG_CONFIRM_DISCONNECT_ALL, ["!yes", "!no"]) != 0)
>+        return;

Blank line.

>+    var conNetworks = client.getConnectedNetworks();
>+    if (conNetworks.length <= 0)
>+    {
>+        display(MSG_NO_CONNECTED_NETS, MT_ERROR);
>+        return;
>+    }
>+
>+    if (typeof e.reason != "string")
>+        e.reason = client.userAgent;
>+
>+    for (var i = 0; i < conNetworks.length; i++)
>+    {
>+        conNetworks[i].quit(e.reason);
>+    }

Kill braces.

>+function cmdReconnect(e)
>+{
>+    if (e.network.isConnected())
>+    {
>+        // Set reconnect flag
>+        e.network.reconnect = true;
>+        // We need to disconnect first.

Comment makes no sense.

>+        if (typeof e.reason != "string")
>+            e.reason = MSG_RECONNECTING;
>+        e.network.quit(e.reason);
>+    }
>+    else
>+    {
>+        e.network.connect(e.network.requireSecurity);
>+    }
>+}
>+
>+function cmdReconnectAll(e)
>+{
>+    var reconnected = false;
>+    for (var net in client.networks)
>+    {
>+        if (client.networks[net].isConnected() || 
>+            "messages" in client.networks[net])

Parens.

>+        {
>+            client.networks[net].dispatch("reconnect", { reason: e.reason });
>+            reconnected = true;
>+        }
>+    }
>+    if (!reconnected)
>+        display(MSG_NO_RECONNECTABLE_NETS, MT_ERROR);
>+}
>+
>+function cmdRejoin(e)
>+{
>+    if (e.channel.joined)
>+    {
>+        if (!("reason" in e) || !(e.reason))

e.reason always exists. Use:
    if (!e.reason)

>+            e.reason = "";
>+        e.channel.part(e.reason);
>+    }
>+
>+    e.channel.join(e.channel.mode.key);
>+}

>-                    e.reason = e.channelName + " " + e.reason;
>+                    if (e.reason)
>+                        e.reason = e.channelName + " " + e.reason;
>+                    else
>+                        e.reason = e.channelName;

Could use ternary operator.


>Index: mozilla/extensions/irc/xul/content/handlers.js
>+    if (e.quitting)
>+    {

Earlier comment belongs here.

>+        msgType = "INFO";
>+        msg = getMsg(MSG_CONNECTION_QUIT, [this.getURL(), e.server.getURL()]);
>+    }


>Index: mozilla/extensions/irc/xul/content/menus.js
>+    var isChannel = "(cx.TYPE == 'IRCChannel')";
>     var inChannel = "((cx.TYPE == 'IRCChannel') and cx.channel.active)";
>+    var isNetwork = "(cx.network)";

Kill parens.

>+    var netConnected = "cx.network.isConnected()";

Could use netConnect & netDisconnect instead to simply code below.

> function getCommandContext (id, event)
> {
>     var cx = { originalEvent: event };
>-
>+    

Don't do that.


>Index: mozilla/extensions/irc/xul/locale/en-US/chatzilla.properties
>+msg.connection.quit    = Disconnected from %S (%S).
> msg.close.status       = Connection to %S (%S) closed with status %S.

Blank line.

>+msg.reconnecting       = Reconnecting...
>+msg.confirm.disconnect.all = Are you sure you want to disconnect from ALL networks?
>+msg.no.connected.nets      = You are not connected to any networks.
>+msg.no.reconnectable.nets  = There are no networks to reconnect to.
Attachment #174745 - Flags: review?(silver) → review-
(Reporter)

Comment 6

14 years ago
Created attachment 174817 [details] [diff] [review]
Patch with formatting changes suggested by Silver

Thanks for the comments, new patch with the changes :-).
Attachment #174745 - Attachment is obsolete: true
Attachment #174817 - Flags: review?(silver)
(Reporter)

Comment 7

14 years ago
Created attachment 174836 [details] [diff] [review]
Fixes confirmEx, menu's.

Probably the most humiliating thing that can happen to your own patches is
thinking you have them finished, and ending up to find it breaks ChatZilla as a
whole.

Fixed:
defaultButton was a) broken, b) broke ChatZilla as it was called 'default' on
one occasion, which is a reserved word and therefor broke the entire app when
loading.

the menu stuff was broken because I had simply forgotten to update conditions.
Fixed.


Note to self: don't try and submit patches past 1am anymore.
Attachment #174817 - Attachment is obsolete: true
Attachment #174836 - Flags: review?(silver)
(Reporter)

Updated

14 years ago
Attachment #174817 - Flags: review?(silver) → review-

Comment 8

14 years ago
Comment on attachment 174836 [details] [diff] [review]
Fixes confirmEx, menu's.

review=silver@warwickcompsoc.co.uk
Attachment #174836 - Flags: review?(silver) → review+

Comment 9

14 years ago
Checked in --> FIXED.

In approximately 40 minutes a build with this fix will be available from:
  http://twpol.dyndns.org/mozilla/chatzilla/nightly/
Status: UNCONFIRMED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

14 years ago
Created attachment 175750 [details] [diff] [review]
Patch to stop channel view from being discarded regardless of pref setting.

Additional small patch to stop the channel view from being deleted. Thanks to
rginda for the suggestion.
Attachment #175750 - Flags: review?(silver)

Comment 11

14 years ago
Comment on attachment 175750 [details] [diff] [review]
Patch to stop channel view from being discarded regardless of pref setting.

review-, see below.

>Index: mozilla/extensions/irc/xul/content/commands.js
>-        e.channel.part(e.reason);
>+        dispatch("part", { reason: e.reason, noDelete: true });

You need to call dispatch on the channel object, e.g.
  e.channel.dispatch(...);
or it won't work if you call *this* function using dispatch (it will assume the
current view always, which is wrong).
Attachment #175750 - Flags: review?(silver) → review-
(Reporter)

Comment 12

14 years ago
Created attachment 175752 [details] [diff] [review]
patch v2, use e.channel.dispatch instead
Attachment #175750 - Attachment is obsolete: true
Attachment #175752 - Flags: review?(silver)

Comment 13

14 years ago
Comment on attachment 175752 [details] [diff] [review]
patch v2, use e.channel.dispatch instead

r=silver@warwickcompsoc.co.uk
Attachment #175752 - Flags: review?(silver) → review+

Comment 14

14 years ago
Checked in. You know the drill.
(Assignee)

Comment 15

14 years ago
I was poking around to see if mozilla supported ircs, and did this...

 /disconnect

 /attach ircs://moznet
   This failed right away because there are no servers
   on the moznet network listed as doing ircs.
   The error message was "Connection attempts exhausted,
   giving up."  (There should probably be a better message
   here, somehow.)

 <...> I tried a few other ways to connect.

 /reconnect
  Instead if reconnecting, I got the attempts exhausted
  message again.

If I drag the moznet url back into the message window, chatzilla
reconnects properly.
(Reporter)

Comment 16

14 years ago
(In reply to comment #15)
> I was poking around to see if mozilla supported ircs, and did this...
> 
>  /disconnect
> 
>  /attach ircs://moznet
>    This failed right away because there are no servers
>    on the moznet network listed as doing ircs.
>    The error message was "Connection attempts exhausted,
>    giving up."  (There should probably be a better message
>    here, somehow.)
> 
>  <...> I tried a few other ways to connect.
> 
>  /reconnect
>   Instead if reconnecting, I got the attempts exhausted
>   message again.
> 
> If I drag the moznet url back into the message window, chatzilla
> reconnects properly.


As a sidenote, the main moznet server supports ircs, but only on port 6697,
which makes the standard ircs://moznet fail.

I'm looking into this, and reopening the bug with a very slightly adjusted summary.
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Summary: ChatZilla needs /reconnect, /reconnect-all, /disconnect-all and /rejoin → ChatZilla needs a working /reconnect, /reconnect-all, /disconnect-all and /rejoin
(Reporter)

Comment 17

14 years ago
Right, it turns out this does not have anything to do with this bug, which is
why I'm closing it again.

Sorry for the spam everyone.

See bug 284149 for more about the problem reported in comment #15
Status: UNCONFIRMED → RESOLVED
Last Resolved: 14 years ago14 years ago
Resolution: --- → FIXED
Summary: ChatZilla needs a working /reconnect, /reconnect-all, /disconnect-all and /rejoin → ChatZilla needs /reconnect, /reconnect-all, /disconnect-all and /rejoin
You need to log in before you can comment on or make changes to this bug.