Closed Bug 96459 Opened 23 years ago Closed 23 years ago

Need progress indicators for XMLHttpRequest

Categories

(Core :: XML, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: hjtoi-bugzilla, Assigned: hjtoi-bugzilla)

References

Details

(Whiteboard: [DIGBug][fixinhand])

Attachments

(4 files)

Hopefully "onreadystatechange" and "readyState" members would be enough, as per
the MS IDL.
Priority: -- → P2
Whiteboard: [DIGBug]
Target Milestone: --- → mozilla0.9.4
Hmm... I am not sure how to implement "onreadystatechange" at this point... On
the other hand, I am not sure if that is absolutely required. My limited testing
implies that with IE the function that has been set will only be called once for
INTERACTIVE data, which would seem to indicate that you'd need to set up a
timeout to see if responseText grows. I originally thought INTERACTIVE message
would be gotten several times (once for each chunk of data read from network).
It could be that my testcase never hit a large enough chunk or something (tried
with about 26kB).

The above patch fixes these things:
* Implement readyState member, and allow reading of responseText when readyState
is INTERACTIVE or COMPLETED.
* Throw error if setRequestHeader() is called before open() (same as IE).
* Throw error if send() is called 2nd time without open() (same as IE).

I think this would be the absolute minimum to get going, when used together with
a timeout to check readyState and responseText occasionally.

Greg, would this be sufficient for your purposes? Are some of my
assumptions/observations about IE incorrect?
Status: NEW → ASSIGNED
Whiteboard: [DIGBug] → [DIGBug][fixinhand]
Ok, the latest patch also implements onreadystatechange member. It seems to work
in my testcases. I have no idea if it would be a security risk the way it is
implemented. I have no readystatechange event, I just do some tricks to call the
callback function directly. For onload and onerror I go through the event
system. Thanks to vidur for this trick. 

Mitch, could you take a look at the patch, is it safe?

I don't know if I should fire onreadystatechange with readyState==3
(INTERACTIVE) only once, or for each chunk of data I am reading. With the patch
it is fired for each chunk, but maybe IE is not doing it (so I should change to
fire it only once)? Greg, anyone?

The patch also includes some testcase updates and fix SOAP so that it compiles
(patch for SOAP from peterv).
I don't know whar 'each chunk' means, but would like to be called more than once 
if there is a long download with a lot of data.  I don't know how IE is doing it 
now.
Necko is feeding my code with roughly 4kB 'chunks' of data, and I can fire
onreadystatechange callback for each ~4kB chunk. I know from my testing that IE
called the callback only once for my testcase where the data was about 26kB. It
could be that IE has a 'chunk'/buffer size bigger than that or uses some smart
buffer size and only gave me one when it saw I had fast connection. Or it could
be that IE calls the callback only once, when it starts seeing some data from
the network (besides headers).

Based on your wishes I can leave that part of the code as is, so you will get a
callback for every ~4kB of data. You may still need to do a timer based polling
function for IE, though. Or you could use the same timer polling for both IE and
Mozilla-based browsers.
Do you need this fix for the 0.9.2 branch or would trunk/0.9.4 be enough? If you
need it for 0.9.2, please add topembed keyword.
Well, our group probably won't change our current code to use the new API, so I 
don't think it would be a good idea for me to set the topembed keyword.  Thanks 
for adding it for the future though!
Allrighty! Here is now a proposed fix. 

I noticed one additional problem that is now fixed: in case of an XML parser
error, we were firing the load event too soon. With the fix we store the load
event we get in Load(), and act on it in OnStopRequest() when we can be certain
there is no more data coming in. I noticed the problem with the non-well-formed
XML when playing around with the OASIS XML test suite (Mozilla version by Bob
Clary) at http://green/heikki/oasis/bug57673.html

As it happens, this fix also fixes DIG crasher bug 96307 (aka bugscape 8288).
I'm not sure what's the purpose of JSContextStack. I don't see it being used
anywhere in the function. 

+  nsCOMPtr<nsIJSContextStack> stack;
+  JSContext *cx = nsnull;
+
+  if (mScriptContext) {
+    stack = do_GetService("@mozilla.org/js/xpc/ContextStack;1");
........

Typos:
1) + * compare to Internet Eplorer:
2) +   *   0 UNITIALIZED open() has not been called yet.

Other than that r=harishd
Pushing and popping the JS context that corresponds to the event handlers is a
necessary part of invoking them. The context on the top of the JS context stack
is used by XPConnect for JS invocation. The context is also used by the security
manager to determine subject principals. In fact, the same should be done when
invoking the onreadystatechanged handler. sr=vidur after onreadystatechanged
invocation is fixed.
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Since I 'm not exactly sure how to test this, I'm marking verified based on the
last comments
Status: RESOLVED → VERIFIED
Blocks: 243579
You need to log in before you can comment on or make changes to this bug.