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)
Other Applications
ChatZilla
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Gijs, Assigned: rginda)
Details
Attachments
(2 files, 5 obsolete files)
26.79 KB,
patch
|
bugzilla-mozilla-20000923
:
review+
|
Details | Diff | Splinter Review |
882 bytes,
patch
|
bugzilla-mozilla-20000923
:
review+
|
Details | Diff | Splinter Review |
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•19 years ago
|
||
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•19 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•19 years ago
|
||
Patch based on Silver's comments in #chatzilla.
Attachment #174218 -
Attachment is obsolete: true
Attachment #174248 -
Flags: review?(silver)
Reporter | ||
Updated•19 years ago
|
Attachment #174218 -
Flags: review?(silver)
Reporter | ||
Comment 4•19 years ago
|
||
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•19 years ago
|
Attachment #174248 -
Flags: review?(silver)
Comment 5•19 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•19 years ago
|
||
Thanks for the comments, new patch with the changes :-).
Attachment #174745 -
Attachment is obsolete: true
Attachment #174817 -
Flags: review?(silver)
Reporter | ||
Comment 7•19 years ago
|
||
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•19 years ago
|
Attachment #174817 -
Flags: review?(silver) → review-
Comment 8•19 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•19 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
Closed: 19 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 10•19 years ago
|
||
Additional small patch to stop the channel view from being deleted. Thanks to rginda for the suggestion.
Attachment #175750 -
Flags: review?(silver)
Comment 11•19 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•19 years ago
|
||
Attachment #175750 -
Attachment is obsolete: true
Attachment #175752 -
Flags: review?(silver)
Comment 13•19 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•19 years ago
|
||
Checked in. You know the drill.
Assignee | ||
Comment 15•19 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•19 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•19 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
Closed: 19 years ago → 19 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.
Description
•