Alec Koumjian Reporter
	Description • 11 years ago

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.

	Alec Koumjian Reporter
	Comment 1 • 11 years ago

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.

	Josh Matthews [:jdm]
	Comment 2 • 11 years ago

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.

	Boris Zbarsky [:bzbarsky]
	Comment 3 • 11 years ago

Did the preflight succeed?

	Alec Koumjian Reporter
	Comment 4 • 11 years ago

Yes, the preflight succeeds and the POST also successfully uploads the file. It is only the progress event that is not fired.

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 11 years ago

Jonas?

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 6 • 11 years ago

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.

	Alec Koumjian Reporter
	Comment 7 • 11 years ago

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.

	Josh Matthews [:jdm]
	Comment 8 • 11 years ago

Comment 2 explains what I believe is the solution here. We need to loosen up the checks for AllowUploadProgress.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 9 • 11 years ago

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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 10 • 11 years ago

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.

	Hallvord R. M. Steen [:hallvors]
	Comment 11 • 11 years ago

Gecko fails to fire many expected events on the upload object. Test cases here:

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

Tests should cover progress, timeout and abort events - we may be missing tests for the upload.onerror event but that may be broken too.

	Hallvord R. M. Steen [:hallvors]
	Comment 13 • 11 years ago

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)

	thibault.6
	Comment 15 • 10 years ago

Bump this is seriously screwing up my stuff. 

Has anyone come up with a workaround ?

	Alexandre BM (:rednaks)
	Comment 16 • 10 years ago

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.

	Hallvord R. M. Steen [:hallvors]
	Comment 17 • 10 years ago

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.