Closed Bug 281706 Opened 19 years ago Closed 19 years ago

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

Categories

(Other Applications :: ChatZilla, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Gijs, Assigned: rginda)

Details

Attachments

(2 files, 5 obsolete files)

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.
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)
Updating summary to suit patch.
Summary: ChatZilla needs /reconnect, /reconnect-all and /disconnect-all → ChatZilla needs /reconnect, /reconnect-all, /disconnect-all and /rejoin
Patch based on Silver's comments in #chatzilla.
Attachment #174218 - Attachment is obsolete: true
Attachment #174248 - Flags: review?(silver)
Attachment #174218 - Flags: review?(silver)
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)
Attachment #174248 - Flags: review?(silver)
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-
Thanks for the comments, new patch with the changes :-).
Attachment #174745 - Attachment is obsolete: true
Attachment #174817 - Flags: review?(silver)
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)
Attachment #174817 - Flags: review?(silver) → review-
Comment on attachment 174836 [details] [diff] [review]
Fixes confirmEx, menu's.

review=silver@warwickcompsoc.co.uk
Attachment #174836 - Flags: review?(silver) → review+
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
Closed: 19 years ago
Resolution: --- → FIXED
Additional small patch to stop the channel view from being deleted. Thanks to
rginda for the suggestion.
Attachment #175750 - Flags: review?(silver)
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-
Attachment #175750 - Attachment is obsolete: true
Attachment #175752 - Flags: review?(silver)
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+
Checked in. You know the drill.
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.
(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
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
Closed: 19 years ago19 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.

Attachment

General

Creator:
Created:
Updated:
Size: