292014 - auto-accept dcc downloads

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, enhancement)

Description

[r579srysw57fto]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2

Chatzilla doesn't allow one to determine who one wants to accept dcc sends from
like mirc. It requires every dcc send to be manually accepted which isn't
feasible unless I could live at my keyboard.

Reproducible: Always

Comment 1

[Rob Marshall [tH]]

I've been working on this a bit.
Prefs I plan to add are:
- dcc.downloadsFolder - Default save location for dcc files. Per-network and global, defaults to profilePath/downloads.
- dcc.autoAccept.enabled - Per-network, defaults to false.
- dcc.autoAccept.list - Whitelist of hostmasks. Per-network, empty by default.
- dcc.autoAccept.delay - Number of seconds to wait before auto-accepting something, allowing user time to cancel or save a file manually. Global, hidden, defaults to 10.

I'm also adding a command to add users to the whitelist quickly. Comments/other suggestions welcome.

Comment 2

[Rob Marshall [tH]]

Attachment: first attempt

I decided to drop the enabled pref, since an empty list is the same as disabled.
Added the commands /dcc-accept-add, /dcc-accept-remove, and /dcc-accept-list.

Comment 3

[James Ross]

Comment on attachment 209471 [details] [diff] [review]
first attempt

>Index: extensions/irc/xul/content/commands.js
>@@ -3891,47 +3896,127 @@ function cmdDCCList(e) {
>                                           cmds]));
>         client.munger.entries[".inline-buttons"].enabled = false;
>     }
>     display(getMsg(MSG_DCCLIST_SUMMARY, [counts.pending, counts.connected,
>                                          counts.failed]));
>     return true;
> }
> 
>+function cmdDCCAutoAcceptList(e)
>+{
>+    if (!jsenv.HAS_SERVER_SOCKETS)
>+        return display(MSG_DCC_NOT_POSSIBLE);
>+    if (!client.prefs["dcc.enabled"])
>+        return display(MSG_DCC_NOT_ENABLED);
>+
>+    var list = e.network.prefs["dcc.autoAccept.list"];
>+
>+    if(list.length == 0)

Nit: space before open parenthesis.

>+        display(MSG_DCCACCEPT_DISABLED);
>+    else
>+        display(getMsg(MSG_DCCACCEPT_LIST, arraySpeak(list)));


>+        maskObj = getHostmaskParts(list[m]);
>+        if(e.nickname == list[m] ||
>+           (e.user && hostmaskMatches(e.user, maskObj, e.server)))

Missing space, and because this is multiple lines the contents of the if and else clauses need to be in braces.

>+            display(getMsg(MSG_DCCACCEPT_DEL, list[m]));
>+        else
>+            newList.push(list[m]);
>+    }
>+
>+    if(list.length > newList.length)

Nit: space again.

>+        e.network.prefs["dcc.autoAccept.list"] = newList;
>+    else
>+        display(getMsg(MSG_DCCACCEPT_DELERR,



>Index: extensions/irc/xul/content/handlers.js
>@@ -2480,28 +2480,76 @@ CIRCUser.prototype.onUnknownCTCP =
> function my_unkctcp (e)
> {
>     this.parent.parent.display (getMsg(MSG_UNKNOWN_CTCP,
>                                        [e.CTCPCode, e.CTCPData,
>                                         e.user.unicodeName]),
>                                 "BAD-CTCP", this, e.server.me);
> }
> 
>+function onDCCAutoAcceptTimeout(o, folder)
>+{
>+    // user may have already accepted or declined
>+    if(o.state.state != DCC_STATE_REQUESTED)
>+        return;
>+
>+    if(o.TYPE == "IRCDCCChat")

Nit: space

>+    {
>+        o.accept();
>+        display(getMsg(MSG_DCCCHAT_ACCEPTED, o._getParams()), "DCC-CHAT");
>+    }
>+    else
>+    {
>+        var dest, leaf, tries = 0;
>+        while(true)

Nit: space

>+        {
>+            leaf = escapeFileName(o.filename);
>+            if(++tries > 1)

Nit: space

>+                leaf = leaf.replace(/(\.[a-z]*\.(gz|bz2|z)|\.[^\.]*|)$/i,
>+                                    "[" + tries + "]$&");

Multiple lines (even though it is only one, wrapped) mean it needs braces.

That's a rather "fun" replace, too; please add a comment explaining what this is doing, and why it is doing it the way it is (the specific matching of "gz", "bz2" and "z"). It might also be worth explaining in what cases this fails, and probably we should still cope.

>+
>+            dest = getFileFromURLSpec(folder);
>+            dest.append(leaf);
>+            if(!dest.exists())
>+                break;
>+        }
>+        o.accept(dest);
>+        display(getMsg(MSG_DCCFILE_ACCEPTED, o._getParams()), "DCC-FILE");
>+    }
>+}
>+


>+
>+    var allowList = this.parent.parent.prefs["dcc.autoAccept.list"];
>+    for(var m = 0; m < allowList.length; ++m)

Nit: space

I'm going to stop mentioning this now, but there's quite a lot of missing spaces before parentheses in ifs. :)

>+    {
>+        if(hostmaskMatches(e.user, getHostmaskParts(allowList[m]),
>+                           this.parent))
>+        {
>+            var acceptDelay = client.prefs["dcc.autoAccept.delay"];
>+            if(acceptDelay == 0) // if delay=0, don't show buttons
>+                cmds = MSG_DCC_ACCEPTING_NOW;
>+            else
>+                cmds += " " + getMsg(MSG_DCC_ACCEPTING, acceptDelay);

Please use a locale string for this; locales may wish to use a different order. I would do a new version of MSG_DCCCHAT_GOT_REQUEST for each of these cases, so we end up with three MSG_DCCCHAT_GOT_REQUEST versions (normal, accepting now, and accepting in a bit). That gives localisers the best chances of formatting it how they like (the accepting now one would just skip the replacement for the cmds).

>+            setTimeout(onDCCAutoAcceptTimeout, acceptDelay * 1000, c);
>+            break;
>+        }
>+    }
>+
>     this.parent.parent.display(getMsg(MSG_DCCCHAT_GOT_REQUEST,
>                                       c._getParams().concat(cmds)),
>                                "DCC-CHAT");
>     client.munger.entries[".inline-buttons"].enabled = false;

If you could move the enabling line to right before the display call, that would be good. The less code between the two the better.

>     client.munger.entries[".inline-buttons"].enabled = true;
>     var cmds = getMsg(MSG_DCC_COMMAND_ACCEPT, "dcc-accept " + f.id) + " " +
>                getMsg(MSG_DCC_COMMAND_DECLINE, "dcc-decline " + f.id);
>+
>+    var allowList = this.parent.parent.prefs["dcc.autoAccept.list"];
>+    for(var m = 0; m < allowList.length; ++m)
>+    {
>+        if(hostmaskMatches(e.user, getHostmaskParts(allowList[m]),
>+                           this.parent))
>+        {
>+            var acceptDelay = client.prefs["dcc.autoAccept.delay"];
>+            if(acceptDelay == 0) // if delay=0, don't show buttons
>+                cmds = MSG_DCC_ACCEPTING_NOW;
>+            else
>+                cmds += " " + getMsg(MSG_DCC_ACCEPTING, acceptDelay);
>+            setTimeout(onDCCAutoAcceptTimeout, acceptDelay * 1000,
>+                       f, this.parent.parent.prefs["dcc.downloadsFolder"]);
>+            break;
>+        }
>+    }

Same comments are with onDCCChat (incidentally, it looks like the function is called my_dccchat, another copy-paste error from my DCC work).


>@@ -2703,11 +2769,15 @@ function my_dccfiledisconnect(e)
>-    this.display(getMsg(MSG_DCCFILE_CLOSED, this._getParams()), "DCC-FILE");
>+    var msg = getMsg(MSG_DCCFILE_CLOSED, this._getParams());
>+    if(this.state.dir == DCC_DIR_GETTING)
>+        msg += " " + getMsg(MSG_DCCFILE_SAVED, this.localPath);

As before, please use two versions of the local string here, rather than appending one to the other.

>+
>+    this.display(msg, "DCC-FILE");
> }
> 


>Index: extensions/irc/xul/content/prefs.js
>+         ["dcc.autoAccept.delay", 10,     "hidden"],

The delays used elsewhere are in MS already, I think it would be better to keep them like that.

>+         ["dcc.downloadsFolder", getURLSpecFromFile(downloadsPath.path), "dcc"],

Comment 4

[Rob Marshall [tH]]

Attachment: nits picked, more strings

Comment 5

[Rob Marshall [tH]]

Attachment: better utils fix

Modified hostmaskMatches() in the same way as the patch for bug 123970, which is better. ;)
No other changes from the previous patch.

Comment 6

[Micah Bucy]

I applied the most recent patch to Chatzilla 0.9.69.3 in Deer Park 20060103 on a NetBSD system, and I was successfully able to do auto-accepting, but two current issues.  The first is the filename was retrieved such that [ was %5b Second, when browsing for the path to set as the default download folder, there is now way to accept the directory ie clicking the Save button does nothing.  The manual typing of the directory in the field did work.

Comment 7

[Micah Bucy]

(In reply to comment #6)
> I applied the most recent patch to Chatzilla 0.9.69.3 in Deer Park 20060103 on
> a NetBSD system, and I was successfully able to do auto-accepting, but two
> current issues.  The first is the filename was retrieved such that [ was %5b
> Second, when browsing for the path to set as the default download folder, there
* is no way to accept the directory ie clicking the Save button does nothing. 
> The manual typing of the directory in the field did work.

A little clarity...as far as [ being %5b, ] was %5d, so I'm guessing it converts all of the characters to url encoding 

Comment 8

[James Ross]

Any characters that we cannot or simply don't gurentee as "safe for filesystems" will be escaped using URL encoding, yes. This is the same as for log files.

Comment 9

[Micah Bucy]

(In reply to comment #8)
> Any characters that we cannot or simply don't gurentee as "safe for
> filesystems" will be escaped using URL encoding, yes. This is the same as for
> log files.
> 

the thing is when accepting dcc files normally by clicking the Accept button, the file name does not use url encoding, i'm guessing due to the Save dialogue that converts the url encoding.  Just wondering is such a conversion is possible with the auto-accept

Comment 10

[James Ross]

The save dialog (well, the Mozilla code that calls it) replaces characters it doesn't like the look of with "_", which is less useful than %-encoding, as you completely and utterly lose the characters. The only possible change that would be considered is escaping the filename using %-encoding before opening the save dialog. There is no way we are removing the %-encoding.

Comment 11

[Rob Marshall [tH]]

The problem with the browse button is bug 322259.

Comment 12

[Samuel Sieb]

Here's the list of characters that can't be used in filenames:
http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsCRT.h#268

Comment 13

[:Gijs (he/him)]

Checked in --> FIXED.

Comment 14

[:Gijs (he/him)]

I've filed bug 326873 for the filenaming issue - it's separate, and several people agreed that we could just check this in and then rethink the filename logic - it works as-is regardless.