XHR upload progress event does not reach 100% in FireFox until the beginning of the server response is received

RESOLVED FIXED in Firefox 43

Status

()

defect
--
major
RESOLVED FIXED
4 years ago
2 months ago

People

(Reporter: guohonglin1984, Assigned: mcmanus)

Tracking

31 Branch
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(4 attachments)

Reporter

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
Build ID: 20150624141335

Steps to reproduce:

Steps to reproduce:

1.I am using Dojo Progress Bar to show document upload progress
2.Adding documents in Firefox (file size is 1~2M or smaller,largre size can not reproduce)
3.Progress bar can not reach 100%
4. Debug in firebug and found that in Firefox ESR 24,the progress 100 % event is not fired. In ESR 31 and 38,found that 100% event is fired, but 100% percentage does not render in FireFox browser.
Same code run on Chrome or IE 11, does not find this issue.




Actual results:

Progress bar can not reach 100%



Expected results:

Show 100% progress correctly
Reporter

Updated

4 years ago
Severity: normal → major
Component: Untriaged → Developer Tools
Thank you for the report.  This sounds like an issue in XHR processing, not the DevTools, unless I am misunderstanding.
Component: Developer Tools → DOM
Product: Firefox → Core
Can you please link to a page showing the problem for you?
Flags: needinfo?(guohonglin1984)
Reporter

Comment 3

4 years ago
(In reply to J. Ryan Stinnett [:jryans] (use ni?) (on PTO July 27 - 29) from comment #1)
> Thank you for the report.  This sounds like an issue in XHR processing, not
> the DevTools, unless I am misunderstanding.

yes,it is. Component should be DOM
Reporter

Comment 4

4 years ago
(In reply to Boris Zbarsky [:bz] from comment #2)
> Can you please link to a page showing the problem for you?

it is our product under development,so no page can be linked.

and I found that "load" event of XHR upload is not fired in Firefox ESR 24,31,38.

code is like:

this.xmlHttpRequest.upload.addEventListener("load", myfunction, false);

as load event is fired when the request has successfully completed. it is a work around for 100% progress event. but it is not fired. 

And I found that some one reported this issue in stackoverflow 
http://stackoverflow.com/questions/17932058/progress-event-in-firefox-with-ajax-does-not-reach-100
Flags: needinfo?(guohonglin1984)
> it is our product under development,so no page can be linked.

OK, that makes it rather hard to do anything with this bug report...

> code is like:

So I just tried simple code like that.  See http://web.mit.edu/bzbarsky/www/testcases/xmlhttprequest/simple-upload-load-listener.html which shows us firing the "load" event on the upload object just fine.  Not surprising, since our automated test suite tests this too.

So what's different about your case that causes the load event to not fire?

And is the load event thing really related to the progress event issue this bug was initially filed on?

Anyway, I tried some testcases with progress listeners as well:

http://web.mit.edu/bzbarsky/www/testcases/xmlhttprequest/simple-upload-progress-listener.html
http://web.mit.edu/bzbarsky/www/testcases/xmlhttprequest/medium-data-upload-progress-listener.html
http://web.mit.edu/bzbarsky/www/testcases/xmlhttprequest/large-data-upload-progress-listener.html

And they all work correctly as far as I can tell, in Firefox 39.  Any information on how to actually reproduce the problem you're seeing would be very much appreciated
Flags: needinfo?(guohonglin1984)
Reporter

Comment 6

4 years ago
Flags: needinfo?(guohonglin1984)
Reporter

Comment 9

4 years ago
Hi,

I have created a sample project to reproduce this issue on Firefox.Please take a look and accept this issue as one Firefox defect.

I attached zip/war files of my sample project and the document used to add in to reproduce this issue.

Steps to reproduce:

1. Deploy the project into Tomcat(My environment is Tomcat 6)
2. Access to  http://servername:portnumber/SampleProject/ in browser(My environment is FF ESR 38.2.1)
3. Click "Browse" button and choose the document I attached which size is 3.48M .(you can also use other files which size is several megabytes or smaller,larger size file can not reproduce this issue on Firefox)
4. Click "Upload" button.(Progress bar reach to 97% and STOP.this percentage number may be different for every file and every time.Waiting for 10 seconds,progress bar reached 100%)
5. Function in this sample project is 
       I. added the selected file into one temp folder on server
       II. When adding file is completed, wait for 10 seconds.
       III.Then delete temp file and temp folder.

so correct behavior is progress bar reaches 100% before waiting 10 seconds because the file uploading has completed. Now progress bar on Firefox is waiting there for 10 seconds and then reach to 100%. It is not correct. it seems that 100% progress event is blocked until the action in server side is finished and XHR request is finished. 

6. Do these same steps on Chrome(My environment is version 42), the behavior is correct.

expected result:

Upload progress bar reach to 100% when uploading is completed in Firefox.


if you have any questions,please let me know,Thank you!
Will need to find someone with a Tomcat setup.  :(
Oh, wait.  No, none of that is needed, I expect.

So the issue, basically, is that the last progress event is not fired until the server actually responds.  That's entirely plausible given the way necko sends its notifications, I expect.  Honza, can you check whether necko would fail to send the last upload progress notification until OnStartRequest in this sort of situation?
Flags: needinfo?(honzab.moz)
Summary: XHR upload progress event does not reach 100% in FireFox → XHR upload progress event does not reach 100% in FireFox until the beginning of the server response is received
Reporter

Comment 12

4 years ago
Hi,
Any update for this problem? As our product will be released recently and it's a critical defect for our product. so I'd like to know when it can be fixed and included in Firefox, Thank you!
Patrick, could you help here please?  Comment 11.  Thanks.
Flags: needinfo?(honzab.moz) → needinfo?(mcmanus)
Assignee

Comment 14

4 years ago
this is a fun bug - thanks

the input stream is basically being queried re-entrantly for its status during its output method.. so the counters haven't been updated yet, which makes each status update be one update behind where it should be - and the last one is dropped. (the first one is reported as 0, which is a tell). The progress listener cleans it up when the response arrives.

I'll figure out a fix.
Flags: needinfo?(mcmanus)
Assignee

Comment 20

4 years ago
there is an xhr issue in here too, where it essentially relies on the upload events being short of a full deck and if they are fixed to be correct it suppresses the last notification before the state is changed to ready.
Assignee

Comment 22

4 years ago
re comment 20, testing/web-platform/tests/XMLHttpRequest/event-upload-progress.htm is a test (among many) that breaks if upload.onprogress() is not delivered before readystate changes in OnStartRequest
Assignee

Comment 24

4 years ago
there are some jetpack fails in the comment 23 try run, but I made another run without the patch and they have the same problem

https://treeherder.mozilla.org/#/jobs?repo=try&revision=43ae97d5c38d
Assignee

Updated

4 years ago
Assignee: nobody → mcmanus
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee

Updated

4 years ago
Attachment #8662512 - Flags: review?(hurley)
Assignee

Comment 26

4 years ago
boris, I r?'d you for the XHR portion
Assignee

Comment 27

4 years ago
Comment on attachment 8662512 [details] [diff] [review]
ontransportstatus SENDING_TO should not use request stream re-entrantly

Review of attachment 8662512 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsXMLHttpRequest.cpp
@@ +3497,5 @@
>      mUploadLengthComputable = lengthComputable;
>      mUploadTransferred = loaded;
>      mProgressSinceLastProgressEvent = true;
>  
> +    MaybeDispatchProgressEvents((mUploadTransferred == mUploadTotal));

without this change, XHR always dispatches the last upload progress event either
1] off a timer
or 2] from onstartrequest if necko hasn't sent it proper progress reports yet

Prior to this patch, necko generally didn't send the last upload progress event at all, so #2 was the common case. But upon fixing it things reverted to the timer, which isn't good if you want the upload progress events to preceed the readystate change (which one of the web platform tests checks).

::: netwerk/base/nsTransportUtils.h
@@ -16,5 @@
>   * be delivered.  The progress reported will be that from the second event.
> - * If aCoalesceAllEvents is true, then any undelivered event will be replaced
> - * with the next event if it arrives early enough.  This option should be used
> - * cautiously since it can cause states to be effectively skipped.  Coalescing
> - * events can help prevent a backlog of unprocessed transport events in the

I removed the ability of necko to sometimes coalesce events across state changes. It was a micro-optimization that added a lot of complexit.

Coalescing lots of progress events of the same type makes sense, but the status events are important markers - for instance the devtools tests are basically confused because they cannot count on receiving these. And the status transitions are not the bulk of the data.

Furthermore, the receiver would get very different data streams depending on the state of other components and whether they had registered activity distributors - so behavior was different for xhr depending on whether devtools was open or not. The interface is better as consistent, and I don't think the optimization was a big one.

::: netwerk/protocol/http/nsHttpTransaction.cpp
@@ +346,5 @@
>                                 mReqHeaderBuf.Length());
>      if (NS_FAILED(rv)) return rv;
>  
> +    mHasRequestBody = !!requestBody;
> +    if (mHasRequestBody) {

this is another latent bug. We don't send upsteam onprogress notifications to channels without a request body, but some methods without a body other than GET set the uploadstream from XHR along with a content-length of 0. Because the underlying bug here often dropped the last upload onprogress notification it papered over the fact that the test for mHasRequestBody needs to be more than if just any stream was provided.

Updated

4 years ago
Attachment #8662512 - Flags: review?(hurley) → review+
Comment on attachment 8662512 [details] [diff] [review]
ontransportstatus SENDING_TO should not use request stream re-entrantly

r=me on the XHR part, I think...  That code is finicky.  :(
Attachment #8662512 - Flags: review?(bzbarsky) → review+
Assignee

Comment 29

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a56db89602545da44a7dd7396a683ddf1d695f1
bug 1187239 - ontransportstatus SENDING_TO should not use request stream re-entrantly r=hurley r=bz
https://hg.mozilla.org/mozilla-central/rev/2a56db896025
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Reporter

Comment 31

4 years ago
Hi,
When Firefox 43 can be released with this fix ? and is there plan to include this fix in ESR version ?
The release schedule is at https://wiki.mozilla.org/RapidRelease/Calendar#Future_branch_dates -- Firefox 43 will be released near end of January 2016.

I don't expect the ESR drivers would approve this fix; they typically take only fixes for sufficiently severe security bugs.
Assignee

Comment 33

4 years ago
fwiw I think firefox 43 is in mid december..

anyhow, this is essentially a 10 year old bug so I expect it will ride the trains in the usual way without any backports.
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.