LPology / Simple-Ajax-Uploader

Javascript file upload plugin with progress bar support. Works in all major browsers, including IE7+, Chrome, Firefox, Safari, and Opera. No dependencies - use it with or without jQuery.
995 stars 268 forks source link

XHR abort? #24

Closed benkeen closed 11 years ago

benkeen commented 11 years ago

Hi! Really great project.

I'm using this script to upload very large files (hundreds of MBs), and our UI designer has requested that we have an automatic trigger so when the user selects a file, it automatically starts uploading. This is all no problem, but I need a way to allow the user to abort the process (e.g. they selected the wrong file).

Unless I missed it, I don't see a way to do this just yet. If you like I can go ahead and add it and submit a pull request. My one concern is compatibility with older browsers and non-XHR uploads. I don't think you can cancel a form post to an iframe (correct?) so this would only work for XHR. For my use-case, that's an acceptable scenario.

Would this be a useful addition, do you find?

Thanks again!

LPology commented 11 years ago

Hi Ben. Glad the project has been useful to you. This would be a useful addition, I think.

You're right, aborting an upload wouldn't work with the iframe method in IE9 and older. That is definitely what makes it tricky -- somehow like half the internet still uses IE so we're doomed to working around that forever.

One possibility would be to take a route similar to setProgressBar(). So, if we added a function like this:

  setCancelBtn: function(elem) {
    this._cancelBtn = ss.verifyElem(elem);
  },

And inside of _uploadXhr() we could something have this:

if (this._cancelBtn) {
  ss.addEvent(this._cancelBtn, 'click', function(event) {
    xhr.abort();
    // probably some cleanup stuff
    return false;
  });
}

This approach would probably work best in conjunction with the startXHR() callback function. For multiple uploads, cancel buttons would probably need to be dynamically generated just like progress bars. Maybe something like this:

startXHR: function() {
  var btn = document.createElement('button');
  btn.value = 'Cancel';
  this.setCancelBtn(btn);
  // add styles, append it to the DOM somewhere, etc.
}

Using startXHR() would ensure that cancel buttons are only available in browsers that support the abort method.

Anyway, those are just some ideas I had off the top of my head. What were you thinking it should look like?

benkeen commented 11 years ago

Wow, thanks for the speedy response!

Yes, that was pretty much what I had in mind as well. The only addition was that I thought maybe it'd be useful to have the option to trigger the abort from plain JS as well as by clicking on a page element. I think in my situation that may be a requirement because there are other things going on on the page that would need to stop it. So perhaps adding an abort() to the SimpleUpload prototype that would trigger the same code?

I'm actually focusing on this today, so how about I knock it up and send it your way?

Again, thanks for the project. :) I've almost convinced my company to let me generalize and open source what I'm working on (a C# file uploader user control), so fingers crossed I can contribute it back...

LPology commented 11 years ago

I'm actually focusing on this today, so how about I knock it up and send it your way?

That sounds great, can't wait to check it out.

I've almost convinced my company to let me generalize and open source what I'm working on (a C# file uploader user control), so fingers crossed I can contribute it back...

Good luck man, that's awesome. For whatever reason, the project has been getting a little more attention lately. I hope that's a sign that it's picking up steam.

benkeen commented 11 years ago

Hey, sorry I never got back to you on Friday.

After a lot of fidding (mostly with C#... not my language of choice!) my scenario turned out to be really quite custom - so I'm not sure the code will be of any use to you. All I ended up doing with adding a custom abort() function on the SimpleUpload prototype, which I had to dynamically add in the _uploadXHR function, because the xhr var is scoped there and needed to be exposed on the uploader instance.

One other oddity was that when aborting an XHR submission, it fires the onError function like anything else. So to detect that scenario, I ended up passing a fourth param (isAborted boolean) to whatever onError function is defined.

That was really all I added - having a target cancel button didn't really work for my case. If you'd like me to submit the code let me know, but it may well be of no use.

Many thanks for the library, we really appreciate it. :) Great stuff.

Oh - and on the bright side, I got the thumbs up to release the code, so I'll upload that later today [https://github.com/CBCMusic/file-uploader].

LPology commented 11 years ago

Thanks for getting back.

I've been working on the plugin a lot the past couple days, including adding abort functionality, along with some other features. (support for Nginx upload progress module, accepting multiple buttons, some big fixes, etc)

One thing that concerned me is multiple, concurrent uploads and making sure the correct upload gets cancelled. Have you tested that with yours? I think I've been able to solve it, but I'd like to see how someone else has done it. I looked at the way jQuery handles XHR aborting, which is obviously different than what we need, but it was still quite helpful.

It's funny that you mentioned onError, because that's a bug that I found a couple days ago. At first I was going to release a quick fix, but I was able to get a lot more work done than I expected and will be releasing v1.8 very soon, which will include the fix.

Oh, also, I noticed that you're using version 1.6.5 of the plugin. I'm about to push v1.8, which will have quite a few fixes that were made since your version. You may want to switch to the newer version, if you have time.

benkeen commented 11 years ago

Great work, thanks! No, I confess I didn't run into that problem with simultaneous uploads, but it could be because in my case I'm instantiating each uploader separately. It could also be because I haven't tested it enough yet. :)

I'll take a look at the code this weekend and update it to use your new version. Again, great work on that - thanks very much. :)

benkeen commented 11 years ago

Oh - and btw, I do wish I could have just contributed the code back to your project, but C# & Visual Studio are such total beasts, they require a vast number of extra files, folders, etc. it just didn't seem possible. So a separate project seemed in order. If and when I implement this in a different programming language, I'll be sure to contribute it back!

LPology commented 11 years ago

Hey, sorry for the delay getting back to you. Thanks for your input, and if you do ever get the chance to contribute any code, I always appreciate it! Thanks again.