Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for XMLHttpRequest.upload to FakeXMLHttpRequest #334

Merged
merged 4 commits into from Oct 24, 2013

Conversation

bcoe
Copy link

@bcoe bcoe commented Oct 8, 2013

I've added support for HTML 5 upload events to the FakeXMLHttpRequest. Here's an issue requesting/discussing functionality:

#185

Please let me know if there are any changes I can make, to get you pumped about pulling this in 馃憤

Reading

},

uploadProgress: function uploadProgress(progressEventRaw) {
this.upload.dispatchEvent(new ProgressEvent('progress', progressEventRaw));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single quotes. Might want to use double-quotes as in the rest of the class.

@cjohansen
Copy link
Contributor

Lookin' good! @mantoni any input on this before I merge?

@mantoni
Copy link
Member

mantoni commented Oct 22, 2013

Nice work. I didn't know about upload progress yet. Looks good to me - go on and merge.

cjohansen added a commit that referenced this pull request Oct 24, 2013
Adding support for XMLHttpRequest.upload to FakeXMLHttpRequest
@cjohansen cjohansen merged commit bc2b010 into sinonjs:master Oct 24, 2013
@meleyal
Copy link

meleyal commented Jan 8, 2014

UploadProgress.prototype.removeEventListener would also be useful

@mantoni
Copy link
Member

mantoni commented Jan 8, 2014

@meleyal True. Could you put it with a test case into a new pull request?

@codeablehq
Copy link

Nope, I can confirm this as well, here's the error

TypeError: Fake server request processing threw exception: '[object ProgressEventConstructor]' is not a constructor (evaluating 'new ProgressEvent("progress", {loaded: 100, total: 100})')

@bcoe
Copy link
Author

bcoe commented Feb 6, 2014

I can take a look at this today, since it's my doing.

What are the exact steps to reproduce?

@bcoe
Copy link
Author

bcoe commented Feb 6, 2014

I'm not getting an exception with:

npm test

or with,

http://localhost:8080/test/sinon.html

What environment (and/or browser) are you running the test suite within?

@codeablehq
Copy link

I'm running it with PhantomJS, here's the AngularJS service + it's tests, in case you need to reproduce the issue: https://gist.github.com/carmivore/980bb3cf098f686985fd

@cjohansen
Copy link
Contributor

I just released 1.8.2 which fixes this issue. It was a matter of a missing feature test.

@codeablehq
Copy link

Thanks!

drzraf pushed a commit to drzraf/flow.js that referenced this pull request Sep 14, 2020
* get rid of the polyfill need back then.

See:
sinonjs/sinon#185
sinonjs/sinon#334

Note that built-in Sinon FakeXMLHttpRequest returns one more event per 200
 what leds to different event array indexing that got fixed in the corresponding test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants