Last Comment Bug 908375 - [XHR2] Does not fire all expected events for XHR upload (progress, timeout, abort)
: [XHR2] Does not fire all expected events for XHR upload (progress, timeout, a...
Status: VERIFIED WORKSFORME
[lang=c++][not a good beginner bug]
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: 23 Branch
: x86 Mac OS X
: -- normal with 2 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Andrew Overholt [:overholt]
Mentors: Josh Matthews [:jdm] (on vacation until Dec 5)
http://w3c-test.org/web-platform-test...
: 641618 (view as bug list)
Depends on:
Blocks: xhr2pass 786953 1103367
  Show dependency treegraph
 
Reported: 2013-08-22 12:47 PDT by Alec Koumjian
Modified: 2014-11-22 06:27 PST (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Alec Koumjian 2013-08-22 12:47:12 PDT
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/28.0.1500.95 Safari/537.36

Steps to reproduce:

On a domain using https:
1. Create a CORS XHR to a different site using https
2. register a callback with the progress event


Actual results:

No progress event is fired.


Expected results:

Progress event should fire.
Comment 1 Alec Koumjian 2013-08-22 14:35:34 PDT
Here is the relevant code snippet:

            var xhr = new XMLHttpRequest();

            xhr.upload.addEventListener('progress', _.bind(this.onProgress, this), false);
            xhr.upload.addEventListener('load', _.bind(this.onComplete, this), false);
            xhr.upload.addEventListener('error', _.bind(this.onError, this), false);
            xhr.upload.addEventListener('abort', _.bind(this.onAbort, this), false);
            xhr.upload.addEventListener('loadstart', _.bind(function () {
                this.eventsController.trigger('uploadQueue:uploadStarted');
                this.setState('uploading');
            }, this), false);

            xhr.open('POST', url);
            xhr.send(uploadData);

            this.listenTo(this.eventsController, 'uploadQueue:cancelCurrent', function() {
                xhr.abort();
            });

Where uploadData is a FormData() object containing an S3 Policy document and a reference to the File object.
Comment 2 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-08-22 18:36:35 PDT
Presumably we're hitting the case at nsXMLHttpRequest::AllowUploadProgress: http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsXMLHttpRequest.cpp#3518 . According to http://www.w3.org/TR/XMLHttpRequest2/#infrastructure-for-the-send-method it looks like we should be allowing upload progress events if the preflight succeeded.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2013-08-22 22:36:23 PDT
Did the preflight succeed?
Comment 4 Alec Koumjian 2013-08-22 22:38:11 PDT
Yes, the preflight succeeds and the POST also successfully uploads the file. It is only the progress event that is not fired.
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2013-08-22 23:50:42 PDT
Jonas?
Comment 6 Jonas Sicking (:sicking) No longer reading bugmail consistently 2013-08-23 00:36:20 PDT
what should happen here is that we should detect that there are upload listeners. this should force a preflight. once the preflight has happened we should be sending progress events as normal when doing the real request.

Not sure that we have tests though, so not surprised if something is broken.
Comment 7 Alec Koumjian 2013-08-28 11:16:23 PDT
Is there anything I can do to help this move forward? Someone in mozilla IRC was able to confirm, but it doesn't look like they set it here.
Comment 8 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-08-28 13:44:58 PDT
Comment 2 explains what I believe is the solution here. We need to loosen up the checks for AllowUploadProgress.
Comment 9 Jonas Sicking (:sicking) No longer reading bugmail consistently 2013-08-28 14:24:10 PDT
So here's what *should* happen. Unfortunately I don't have the bandwidth to investigate why it's not. cc'ing Olli who's been doing a lot of XHR work in the past, but I don't know if he has the bandwidth either.

1. When send() is called we should call CheckChannelForCrossSiteRequest
2. CheckChannelForCrossSiteRequest should detect that mUpload->HasListeners()
   returns true and set XML_HTTP_REQUEST_NEED_AC_PREFLIGHT
3. send() should then call NS_StartCORSPreflight which triggers a preflight
4. Once the real request start we should be getting OnProgress callbacks from necko
5. These callbacks should call MaybeDispatchProgressEvents which in turn should
   call DispatchProgressEvent using mUpload.
6. DispatchProgressEvent should call AllowUploadProgress() which should return true
7. DispatchProgressEvent should then actually dispatch the event.

I don't know where things are going wrong.

Do make sure to do the following two things in your JS code:
A) Make sure to attach the event listeners before calling send() (This is per spec)
B) Make sure that the preflight is successful. If the POST actually is sent to the
   server then the preflight is definitely successful.

Hopefully someone will have the cycles to debug why this is failing. What you could do to help is to set up a simple testcase somewhere. Making the testcase as small as possible will definitely help.
Comment 10 Jonas Sicking (:sicking) No longer reading bugmail consistently 2013-08-28 14:31:06 PDT
jdm: AllowUploadProgress() looks correct to me. It returns true if we didn't do a cross-site request, or if we did a preflighted request.
Comment 12 Hallvord R. M. Steen [:hallvors] 2013-09-20 02:05:49 PDT
*** Bug 786953 has been marked as a duplicate of this bug. ***
Comment 13 Hallvord R. M. Steen [:hallvors] 2013-09-20 02:07:12 PDT
While we don't have a file upload test case in the W3C test suite, we should verify that this fix also handles bug 786953 (upload.onprogress for file upload)
Comment 14 Hallvord R. M. Steen [:hallvors] 2014-05-07 13:15:19 PDT
*** Bug 641618 has been marked as a duplicate of this bug. ***
Comment 15 thibault.6 2014-05-12 12:18:32 PDT
Bump this is seriously screwing up my stuff. 

Has anyone come up with a workaround ?
Comment 16 Alexandre BM (:rednaks) 2014-06-22 04:01:04 PDT
I've just tested the code in attachement 656774 (bug 786953) and it seems that it's woking perfectly for me. I'm running Firefox 30.0

We tried to put the php file on two diffrent domains, let's say domainA and domainB.

There are the different test cases :
localhost -> domainA : Success.
domainB -> domainA : Success.

We also tried different file formats, tar.bz2 and mp3. the progress event was fired until the end.
Comment 17 Hallvord R. M. Steen [:hallvors] 2014-06-23 07:14:21 PDT
URLs on w3c-test.org are now:

http://w3c-test.org/XMLHttpRequest/abort-during-upload.htm
http://w3c-test.org/XMLHttpRequest/event-upload-progress.htm
http://w3c-test.org/XMLHttpRequest/send-response-event-order.htm
http://w3c-test.org/XMLHttpRequest/send-response-upload-event-progress.htm
http://w3c-test.org/XMLHttpRequest/send-timeout-events.htm

This bug originally covered only progress events (tcs 2 and 4 in the above list). I sort of tried to morph it to cover more, but since the progress stuff is now fixed but some of the other stuff isn't that was probably a bad idea. I'll verify this and open a new bug.

I also added a test that should land as 
http://w3c-test.org/XMLHttpRequest/event-upload-progress-crossorigin.sub.htm
to make sure there's a cross-origin variant of the event-upload-progress test already there.

Note You need to log in before you can comment on or make changes to this bug.