285630 - chatzilla does not support resume for dcc downloads

Status: NEW

(Other Applications :: ChatZilla, enhancement)

Description

[aseem behl]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1 StumbleUpon/1.9992
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1 StumbleUpon/1.9992

chatzilla does not support resume for dcc downloads.please provide support
resume  for dcc downloads

Reproducible: Always

Comment 1

[:Gijs (he/him)]

As a clarification, the reporter means DCC resume functions as supported by
mIRC, PIRCH, Virtual IRC and IaIRC. This protocol extension to DCC file
transfers is described here: http://www.mirc.co.uk/help/dccresum.txt

This bug should have Hardware + OS -> All

Comment 2

[Micah Bucy]

don't know if this would help, but check out the dcc.c file from xchat 2 it has a dcc_resume function http://cvs.sourceforge.net/viewcvs.py/xchat/xchat2/src/common/dcc.c?rev=1.79&view=markup
there's also a dcc.h file
http://cvs.sourceforge.net/viewcvs.py/xchat/xchat2/src/common/dcc.h?rev=1.14&view=markup

hoping this might get someone a step toward getting the code for javascript

Comment 3

[James Ross]

Micah, please do not link to other client's source code again. It is very rarely useful. Also, in almost all cases (including this one), the code is licensed directly under the GPL, and not the MPL/GPL/LGPL-tri-license, and hence we cannot use any part of it at all.

Comment 4

[Micah Bucy]

(In reply to comment #3)

Thanks, I'll be sure to keep that in mind.

Comment 5

[Micah Bucy]

Has any work been done on this at all?  Not trying to be pushy, but I can't wait to use chatzilla as my primary irc client, which I can when this issue is taken care of.

Comment 6

[Paul Harvey]

I just want to make a comment that the DCC resume protocol used by mIRC is flawed in a way. It states:

"NOTE: the newer versions of mIRC actually ignore the filename as it is 
redundant since the port uniquely identifies the connection."

This is not necessarily true. Some routers may randomly change the port for security reasons, so the port does not always uniquely identify the connection. For example, this occurs if you route through a Windows machine.

Comment 7

[Stewart]

If some routers randomly change the port for security reasons, they'd have to change the messages to the IRC server as well to keep the access port correct, or file transfer across such routers wouldn't work anyway.

Additionally, the user sending messages can be used, in addition to the port, to uniquely identify any active prospective file transfer... because there's no other ways for the offering system to determine which file is to be transferred at all.

If the router is rewriting the DCC SEND message, but is not similarly adjusting DCC RESUME and DCC ACCEPT messages, resume functionality is simply unavailable, and the client trying to resume will have to recognize that somehow.

Comment 8

[Stewart]

Attachment: Keep the DCC File Transfer progress bar from overflowing the display area

This is a patch for an only tangentially-related, long-standing irritant to me.

Normally, the progress bar's 100% mark might be far to the right of the display, with or without a scroll bar.

This patch seems to prevent this, setting the 100% mark so it fills the available display area, but no more.

Since it IS only tangentially related, I'm attaching it separately from the rest of my modifications enabling DCC RESUME.

Comment 9

[Stewart]

Attachment: DCC RESUME implementation

This is my implementation of DCC RESUME.
It's not perfect.  In particular, the following flaws are known:

1. If the user offering the file has informed the server to block all CTCP messages (usermode T, I believe?), this implementation will take note of the error message resulting from trying to send the DCC RESUME message and the following test PING to that user, and respond appropriately.
However, it will not record that the user in question is blocking CTCP requests, so it'll continue to offer (and attempt, if auto-resume is selected and the user in question is in the dcc-accept-list) the resume option for transfers with that user.
Unfortunately, I haven't been able to find out if there's any way to notice if the user in question turns the usermode in question off, so cacheing the non-acceptance of CTCP messages seems to me to be a bad idea.

2. Users whose clients don't accept or understand DCC RESUME generally won't return any sort of error message, either.  For that matter, bad DCC RESUME messages sent to clients that DO understand DCC RESUME don't seem to respond, either.  My solution for this is the following PING -- if the PING is received, but the DCC ACCEPT isn't, this implementation understands this to mean that the other client doesn't accept DCC RESUME, or at least didn't accept this particular instruction, and responds appropriately.
Again, however, it makes no further note of the failure.  Again, this could probably be done better.

Comment 10

[Glen Mailer]

Its quite difficult to follow a lot of what this patch does in places.

Would you be able to attach a version with larger context and the names of functions included inside the diff? -pU6 should do it, maybe even 9. Alternatively, a public facing link to the hg branch would be good.

There are some coding style guidelines here: http://hacksrus.com/~ginda/pedant.html, which will affect your patch being checked-in, but not necessarily the review process.
As a quick note, we generally don't use the object literal syntax for a complex object. It's preferable to build up the object by assigning properties first, and then pass it in as an argument (in setupResumeFallback).

Unfortunately i'm not familiar enough with the DCC spec to review this properly.

Comment 11

[Stewart]

Attachment: DCC RESUME implementation (v2)

OK, I added more comments, removed a few bits that I probably should've removed before I submitted it the last time (just noticed that I left an empty function definition sitting around...), changed the style to follow your guidelines (or at least they should be closer, now...), and changed setupResumeFallback to (hopefully) be more acceptable.  Plus the changes to the arguments given diff, of course.

Comment 12

[Stewart]

Attachment: DCC RESUME implementation (v3)

It appears that mIRC has good reason to ignore the filename in DCC ACCEPT messages.  I've now encountered a client that returns a different name in a DCC ACCEPT response than it sends in the DCC SEND message... even though I know that I sent the name it sent me in its DCC SEND message in my DCC RESUME request.

The difference between the original (DCC SEND) filename and the responding (DCC ACCEPT) filename was the replacement of (many) underscores with spaces.

This version should handle this particular case properly now.

I also modified my DCC FILE resuming() function so that it will properly indicate a successful resume when f.state == undefined and f.state.state == DCC_STATE_CONNECTED.  I'm not sure why this case ever occurs; I've seen it happen repeatedly now, though.

Comment 13

[:Gijs (he/him)]

Comment on attachment 470678 [details] [diff] [review]
DCC RESUME implementation (v3)

Steven: thanks a lot for this patch. This goes a great way to getting DCC resume working.

I'm afraid James knows a lot more about the DCC implementation than I do (as he wrote it), so I'll punt the actual review. However, here's some general comments:

- Please read the coding style that Glen linked to earlier. In particular:
   * Please use braces around ifs/elses with more than one text line (so irrespective of the number of statements that line represents - comments on separate lines also count as lines :-) ).
   * Please respect the maximum line length of 80 characters wherever reasonably possible - feel free to use temporary variables to help shorten code - it's usually clearer that way, too.

>diff -ruNpU9 chatzilla.0.9.86.orig/content/chatzilla/commands.js chatzilla.0.9.86.new/content/chatzilla/commands.js
<snip>
>+                    if ((c.TYPE == "IRCDCCFileTransfer") && (c.size > 0))
>+                        cmds = cmds +
>+                               getMsg(MSG_DCC_COMMAND_RESUME,
>+                                      "dcc-resume " + c.id) + " ";

See above for style, and please use cmds += ... (here and below).

In general, why both the accept and the resume button? You seem to have coded up support for showing 'accept' if resume fails to work; in almost all cases, isn't 'resume' the better option? Makes me think we should just have that one rather than both, which would confuse users. Or we could rename [Accept] to [Accept as a new file] or something to make the distinction clearer?

<snip>
>         // Accept the request passed in...
>         var filename = c.filename;
>         var ext = "*";
>         var m = filename.match(/...\.([a-z]+)$/i);
>         if (m)
>             ext = "*." + m[1];

This regex seems a bit strange and the extension part is too strict. Extensions can have numbers and underscores, and maybe dashes. Please check if the DCC code already has such a regex that you could reuse (maybe move it to a static property of CIRCDCCSomething), otherwise use something like \w+. I don't understand why you need the 3 dots at the beginning;

> 
>+        // Prevent auto-accept from starting the download while we pick where
>+        // to put it by setting  c.suppressAuto
>+        c.suppressAuto = true;

Why do you need this? The dialogs used to be modal, which IIRC meant no timeouts would fire in the interim, either. This is a horrible 'feature' of the Mozilla API, so in this sense I guess the flag makes sense for futureproofing - I guess I'm mostly curious if the API changed? :-)

>         var pickerRv = pickSaveAs(getMsg(MSG_DCCFILE_SAVE_TO, filename),
>                                   ["$all", ext], filename);
>+        delete c.suppressAuto;

<snip>

>+    function resume(c)
>+    {
>+        // FIXME: check if the user in question accepts CTCPs.
>+        //        Resume is impossible without CTCP acceptance for
>+        //        both sides.
>+        //        Should we cache this information, or...?
>+        //         - I'm pretty sure this doesn't show up on the WHO list
>+        //         - Is there any indication of change in this?
>+        // We know that we can receive CTCPs, because we had to
>+        // in order to receive the DCC SEND message!
>+        //     msg: MSG_DCCFILE_ERR_NOCTCP
>+        //   param: c.user.unicodeName

I think there is no reasonable way to tell this. I'm not sure we should even care. Is it that common for CTCP to be off?

>+
>+        // Resume the request passed in...
>+        var filename = c.filename;
>+        var ext = "*";
>+        var m = filename.match(/...\.([a-z]+)$/i);
>+        if (m)
>+            ext = "*." + m[1];
>+
>+        // Prevent auto-accept from starting the download while we pick where
>+        // to put it by setting  c.suppressAuto
>+        c.suppressAuto = true;
>+
>+        // FIXME: This could be done better: limiting the display
>+        //        to files matching the requirements to start with.
>+        //        It'd be nice if the OK button said "Resume" instead
>+        //        of "Open" too, of course...

Is there no way we can do either of these? Feel free to modify the picker wrapper in file-utils.js if the underlying APIs support it...

>+        var pickerRv = pickOpen(getMsg(MSG_DCCFILE_RESUME_TO, filename),
>+                                ["$all", ext], filename);
>+        delete c.suppressAuto;

<snip>

The following logic to find a DCC object based on parameters looks like it's partly repeated a few times between accept/decline/resume. Can you factor it out?


>+
>+    // If there is no nickname specified, use the "last" item.
>+    // This is the last DCC request that arrived.
>+    if (!e.nickname && client.dcc.last &&
>+        client.dcc.last.state.state == DCC_STATE_REQUESTED)
>+    {
>+        if ((new Date() - client.dcc.lastTime) >= 10000)
>+            return resume(client.dcc.last);
>+        return display(MSG_DCCFILE_ERR_RESUME_TIME);
>+    }
>+
>+    var o = client.dcc.findByID(e.nickname);
>+    if (o)
>+        // Direct ID --> object request.
>+        return resume(o);
>+
>+    var list = client.dcc.getMatches(e.nickname, e.file, ["file"],
>+                                     [DCC_DIR_GETTING],
>+                                     [DCC_STATE_REQUESTED]);
>+    // Accept if only one match.
>+    if (list.length == 1)
>+        return resume(list[0]);
>+
>+    // Oops, couldn't figure the user's request out, so give them some help.
Nit: figure out the user's request.

What happens if there are no matches?

>+    display(getMsg(MSG_DCC_PENDING_MATCHES, [list.length]));
>+    display(MSG_DCC_MATCHES_HELP);
>+    return true;
>+}
>+
> function cmdDCCDecline(e)
> {
>     if (!jsenv.HAS_SERVER_SOCKETS)
>         return display(MSG_DCC_NOT_POSSIBLE);
>     if (!client.prefs["dcc.enabled"])
>         return display(MSG_DCC_NOT_ENABLED);
> 
>     function decline(c)
>     {
>@@ -4400,19 +4513,21 @@ function cmdDCCDecline(e)
>     if (o)
>         // Direct ID --> object request.
>         return decline(o);
> 
>     if (!e.type)
>         e.type = ["chat", "file"];
> 
>     // Go ask the DCC code for some matching requets.
>     var list = client.dcc.getMatches(e.nickname, e.file, e.type,
>-                                     [DCC_DIR_GETTING], [DCC_STATE_REQUESTED]);
>+                                     [DCC_DIR_GETTING],
>+                                     [DCC_STATE_REQUESTED,
>+                                      DCC_STATE_RESUMING]);
>     // Decline if only one match.
>     if (list.length == 1)
>         return decline(list[0]);
> 
>     // Oops, couldn't figure the user's requets out, so give them some help.
>     display(getMsg(MSG_DCC_PENDING_MATCHES, [list.length]));
>     display(MSG_DCC_MATCHES_HELP);
>     return true;
> }
>diff -ruNpU9 chatzilla.0.9.86.orig/content/chatzilla/handlers.js chatzilla.0.9.86.new/content/chatzilla/handlers.js

<snip>

Despite my comment about the (un)necessity of checking for CTCP acceptance, this code seems like something we'd like to keep. :-) 

>+CIRCNetwork.prototype.on492 =
>+function my_noCTCP (e)
>+{
>+    var ary;
>+    if (ary = e.params[2].match(/^(\S+) does not accept CTCPs$/i))
>+    {
>+        e.user = new CIRCUser(e.server, null, ary[1]);
>+
>+        // FIXME: Should we record that this user doesn't accept CTCPs,
>+        //        so the DCC Resume option will no longer be offered for
>+        //        DCC File transfers with this user?
>+
>+        if (e.user.resumeAttempts)
>+        {

Ick. Is there no way to use an event hook for this kind of thing? Would be nice not to have the DCC stuff inside this message, which can pop up for any CTCP message (including user-'generated' time/os/ping/version requests). 

>+            // FIXME: This section has never been tested...
>+            //        but it should work.

This is a scary comment. :-)

<snip>


> function onDCCAutoAcceptTimeout(o, folder)
> {
>+        var dest = findAcceptableAutoFilename(o, folder,
>+                                              function (chk)
>+                                              {
>+                                                  if (!chk.exists())
>+                                                      return 1;
>+                                                  return 0;
>+                                              }
>+                                             );

Please declare a normal named function above and reference it by name as an argument. Same below.

<snip>

>diff -ruNpU9 chatzilla.0.9.86.orig/content/chatzilla/lib/js/dcc.js chatzilla.0.9.86.new/content/chatzilla/lib/js/dcc.js
<snip>
>     // Double-quote file names with spaces.
>+    // Use a temporary variable so we don't mess up the filename field for
>+    // later matches (if the targeted user tries to use DCC RESUME)
>     // FIXME: Do we need any better checking?
>-    if (this.filename.match(/ /))
>-        this.filename = '"' + this.filename + '"';
>+    var needQuotes = '';
>+    if (this.filename.match(/\s/))
>+        needQuotes = '"';
Please use .test for a regex where you're not using the result. Also, do people actually use filenames with tabs and other whitespace? If not, you could use indexOf(' ').

<snip>

>+
>+        if ("resumingAt" in this)
>+        {
>+            // Skip the bytes we're to start after
>+            // This would be much easier if we could seek in the file.
>+            while (this.position < this.resumingAt)
>+            {
>+                if (this.position + this.parent.sendChunk > this.resumingAt)
>+                {
>+                    d = this.filestream.readBytes
>+                       (this.resumingAt - this.position);

Method call opening brace on the same line as the method name, please. :-)

(and once you've stuck everything on one line, please get rid of the curlies for the if/else)

>+                }
>+                else
>+                {
>+                    d = this.filestream.readBytes(this.parent.sendChunk);
>+                }
>+                this.position += d.length;
>+            }
>+            this.ackPosition = this.position;
>+            delete this.resumingAt;
>+        }
>+
>         if (this.parent.sendChunk > this.size)
>             d = this.filestream.readBytes(this.size);
>         else
>             d = this.filestream.readBytes(this.parent.sendChunk);
>         this.position += d.length;
>         this.connection.sendData(d);
>     }
>     catch(ex)
>     {


This isn't a particular thorough review (for which, again, I defer to James), but at least you have some comments. Sorry it took so long to get back to you - I've semi-recently started a day job and am still trying to figure out how to balance it out with keeping track of bugzilla and review requests...

Comment 14

[Stewart]

Thanks for the review, Gijs!  I'm afraid I don't understand all of your comments, and I have another revision of this patch in the works right now.  I hope to catch you on the IRC channel in the near future so I can get some clarifications without filling this space. :)


Re: Braces, I've been trying to imitate the coding style I see, and I've actually been over it all again once so far (and I'm checking again now, in fact).  If I've missed one (which is certainly possible...), I'd appreciate being told where it is, though.


Re: 80 character line limit, I actually edit these files in a terminal with a width of 79 characters.  I thought I got all of them, but I'll check again, and I've fixed the ones you specifically noted in my present revision.


Re: +=, I actually didn't know that JavaScript supports that. :O  Fixed now, though -- thanks!


Re: both the accept and resume buttons:
Unfortunately, it's very possible for one server to offer both "This_Directory\archive.rar" and "This_Directory2\archive.rar", and the DCC SEND message doesn't tell us which one it actually is.  As a result, and since I don't have any state information for which file is in what local name, I have to trust the user to sort them out... which also means that Auto Resume has a potential problem. :(

Worse, even for the same directory/filename combination, different clients may offer files of different lengths as well.  And, on top of that, usernames aren't always constant, even for registered names.  (One IRC server I've seen has a client that is always switching between it's "normal" name and that name with a "2" appended; another on that channel alternates with "_" appended, etc..  As a bonus, said clients are sometimes on in both names at the same time -- usually for a short time, though, until one or the other is disconnected by the IRC server on a timeout.)

As far as renaming 'accept'... I can at least change the tooltip in the localization.


Re: "Strange" regexp, that regexp is actually in the original code.  I simply realized that I was using that chunk of code more than once, so I consolidated it into a new function.  Honestly, I'm not quite sure why that regexp was written that way in the first place, so I'm not going to mess with it. :)


Re: suppressAuto, I didn't realize that the timeouts are put on hold while the dialog is up... if, in fact, this is still the case.

Thinking about it, I believe that putting the timeouts on hold while the dialog is up might not be a good idea anyway, so I'll just call this future-proofing.


Re: CTCP being off:
Apparently, there's a usermode for turning CTCP off.  My first encounter with it was when Gryllida, on irc://moznet/chatzilla, suggested that ObiWan could help me with testing my DCC Resume implementation.  My first attempt to send something to him/her -- a CTCP PING, in fact -- revealed that there was a way to turn CTCP off, and I spent the next week or so searching for everything I could find about such a setting.  (I didn't find much information about it, BTW.)

I have no idea how common or uncommon it is, but I added the code to catch the case as well as I could.

(If you're wondering, in theory it should be possible to DCC SEND a file to someone without accepting CTCP messages yourself, thus blocking DCC RESUME.  I'd need a volunteer to test this theory with, however, and I'm honestly not sure if I've actually worked out how to activate this 'no CTCP' usermode.)


Re: the file picker and limiting the choice of files to those which could be used:
As far as I can tell, the underlying APIs simply don't support the kind of limiting needed. :(  If there is, indeed, a way, however, I'd certainly like to hear about it! :)

As to the "OK" button... (after searching https://developer.mozilla.org/en/nsIFilePicker again)
I don't see any way to make it say "Resume".

Tangentially on this subject... who named that function in 'file-utils.js' "nosepicker"?


Re: DCC object picking, thank you for bringing this up.  I just realized I should have added another test to the 'resume last' case to keep it from trying to apply resume to a chat request.

Also, note that the types of DCC items matched differ between those checks.


Re: if there are no matches, then the offer may have expired.  It'll just list no choices, just like /dcc-accept did before I started modifying.


Re: 492, I didn't see any handlers for on492 already existing, and I'm actually not sure if it's used for anything else... or even if every IRC server uses 492 for this purpose.  Also... "event hook"?  I'll have to ask you about that on IRC the next time I see you there...


Re: scary comment, that's the theoretical case of someone with receiving CTCPs turned off offering a file to someone else.  I'd forgotten about that case, actually... and I think I have the means to test it now (not with a "real" IRC server).  I'll see if I can test it that way sometime this week.  Maybe, after the theoretical tests, ObiWan will be willing to try sending me something a few times (to see if the theoretical case is a concern at all)...


Re: passing functions:
I believe I originally did it with named functions instead of inlined, but then I ran into CIRCServer.prototype.on349 in irc.js (among others)  :)

I'll switch it back again. :)


Re: filenames with spaces, tabs, returns, etc.:
Honestly, I've seen spaces in filenames given from other clients.  I haven't seen tabs or returns yet, though.

Also, I should note that at least one client DOES replace spaces with underscores (_) for DCC SEND messages... but NOT for DCC ACCEPT messages, thus the less-usefulness of the filename component of said messages. :|

Thanks for the tip about .test instead of .match; fixed (and added to my list of JavaScript functions).  Looks like I need to switch it from acting on the string to acting on a regexp? I see a lot more of them in irc.js, by the way.


Re: this.filestream.readBytes ...
If I put that all on one line, it's more than 80 characters wide... unless I sacrifice at least 3 spaces, I believe.
I think the first half of that calculation will fit, though. :)

Comment 15

[Stewart]

Oops, I just noticed that I made a mistake.

Re: "Strange regexp", yes, that one is in the original code, and I don't know why it was done that way.  I didn't consolidate it, though -- that was a different regexp I was thinking of.  I _did_ duplicate it from the original dcc-accept command handler when I built my dcc-resume command handler.


I think I'm almost done putting together the next revision, though I still need to test my changes.  Hopefully sometime this month (also this week) I'll put it here.
For what it's worth, part of the revision is a catch for a file corruption (to a file transfer being received) issue.

Comment 16

[Stewart]

Attachment: DCC RESUME implementation (v4)

OK, this took a lot longer than I was hoping for. :)

Changes:
1. Fixed all the style mistakes I found

2. Added handlers for reoffers(offers for apparently already-active requests, from the same user, with the same name and completed file size)

3. Modified /dcc-close so it can abort a file transfer and so file transfer abort properly detaches reoffers, though note that a reoffer provided only after aborting are not really noticed as being related.

4. Reinitialize localFile objects to confirm file sizes are unchanged, making it less likely for other processes acting on the same file to remain unnoticed.  This is apparently required when ChatZilla is running on Seamonkey 1.1.14, and from asking about the problem in the IRC channel, it sounds as if this actually isn't a problem on newer XULRunner versions, at least.

5. Offering a file currently being received to someone else (i.e. I'm receiving a file transfer of "Frogs.rar" from A, and I offer to send the same "Frogs.rar" to B) now provides the full file length of the transfer in progress (from A) to the person being offered a new transfer (B), instead of the size of the file at the moment the offer is made.

6. Created a "stop and resume" command to stop an active transfer in favor of a reoffer.  This is only activated explicitly, by clicking on the [Switch] command provided when a reoffer is noticed, or when it is attached by the /dcc-resume command.

7. Added an error message and a hook in case the file system receiving an incoming file transfer runs out of space.

8. Added catching for trying to resume with a user using the +R usermode (which would only be a problem when the client isn't using a registered nickname).  Both this case, and the +T usermode handler, have now been tested.

9. Added catching for changes of nickname of another user with a DCC User object.  This affects resume and reoffer handling.  It has only actually been tested in the receiving role -- when the user changing nicknames is offering to send (or is sending) a file transfer to the client.

10. Added .7z, .lzma, and .xz as "compression" extensions in findAcceptableAutoFilename.  This affects where [x] is inserted into filenames when searching for an automatic name.

11. Changed the tooltip for the [Accept] command in the resume->accept fallback.

12. Added a hook in onDCCSend for use by plugins (for better handling of XDCC or FServes, or for making some testing easier -- reducing or increasing the default auto-activity delay).

13. Switched some localIP vs. remoteIP parameter retrieval so they actually provide the _other_ user's IP address for receiving (not sending), which makes more sense for identifying a connection.

14. The new "setReoffer" function's conditions have been tested; except the last two, which should work and I couldn't get to happen anyway :)

15. Added auto-abort of impossible outstanding DCC offers (i.e. not-yet-connected offers with the same IP address and port as a newly-received offer).

Comment 17

[Stewart]

Attachment: DCC RESUME implementation (v4.1)

A couple minor changes:

7. The default "out of disk space" handler now also marks the file transfer's tab with the "attention" state.

12. Added the capability for a hook to set a new filename for the DCC SEND being received without changing the CIRCDCCFileTransfer object's filename (and thus causing problems for DCC RESUME).  Also removed the (unused) "action" member from the event's creation.  The new filename will be treated as if it were the original filename when generating the actual filename used locally.

Comment 18

[:Gijs (he/him)]

Comment on attachment 465119 [details] [diff] [review]
Keep the DCC File Transfer progress bar from overflowing the display area

This looks fine, r=me. Thanks for the patch!

Comment 19

[Stewart]

Attachment: DCC RESUME Implementation (v4.2)

Fixed the last bug I could find.  I don't think I'm going to have any time to do anything more here. :|

1. Fixed the 'ignore' response so it no longer results in half-initialized DCC objects, and actually cancels the transfer.

2. I can see that I made other changes as well, but I don't remember what they were. :(

Comment 20

[:Gijs (he/him)]

Comment on attachment 523419 [details] [diff] [review]
DCC RESUME Implementation (v4.2)

Really punting this review to James, as he knows this code much better. :-)

Comment 21

[:Gijs (he/him)]

Attachment: "chatzilla does not support resume for dcc downloads" [

valid hg patch for the progress bar change

Comment 22

[:Gijs (he/him)]

Attachment: "chatzilla does not support resume for dcc downloads" [

So, this is unbitrotted and in hg cset format. I've tested it only to the extent that it's not breaking chatzilla startup.

Comment 23

[Curtis Clauson]

Has there been any progress on this bug? DCC Resume is badly needed in a wide variety of iRC arenas and the lack of it in Chatzilla is one of the reasons people have stated to me that they won't use it.

It has been over nine years since it was reported and over three years since a patch was worked over and sent for review.

I would like to strongly support a higher priority for this bug.