Closed Bug 379824 Opened 17 years ago Closed 16 years ago

xmlhttprequest abort method changes readystate to 4

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9beta4

People

(Reporter: someguy1337, Assigned: bent.mozilla)

References

Details

(Keywords: addon-compat)

Attachments

(3 files)

another source code to reproduce this bug
652 bytes, application/octet-stream
Details
Patch
1.99 KB, patch
sicking
: review+
sicking
: superreview+
Details | Diff | Splinter Review
a minimal testcase
503 bytes, application/octet-stream
Details 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3

The abort() method for an XMLHttpRequest object cycles the readystate through 4 the handler for onreadystate change will think the request has completed.

i have tested this on ff3 alpha4

Reproducible: Always

Steps to Reproduce:
1. make XHR request to a script that lags
2. abort request before compelted
Actual Results:  
readystate goes to 4

Expected Results:  
readystate should go straight to 0 and not pass go.

<button onclick="request()">YHEY GO REQUEST!</button>
<div id="readystate"/>
<script type="application/ecmascript">
var XHR = new XMLHttpRequest();
function request()
{
	XHR.open("GET", "lag.php", true);
	XHR.onreadystatechange = change;
	XHR.send(null);
	XHR.abort();
}
function change()
{
	var state = document.createTextNode(XHR.readyState);
	document.getElementById("readystate").appendChild(state);
}
</script>
-> Core::General for triage

I believe this belongs in Core::JavaScript Engine.
Product: Firefox → Core
QA Contact: general → general
Component: General → JavaScript Engine
Version: unspecified → Trunk
Assignee: nobody → general
QA Contact: general → general
XHR is DOM, not JS Engine.
Assignee: general → general
Component: JavaScript Engine → DOM
QA Contact: general → ian
Confirmed for Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5) Gecko/20070713 Firefox/2.0.0.5

Additionally, after the abort the reading of XHR.status produced an error:

[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIXMLHttpRequest.status]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"
Spec: abort() must switch the state to UNSENT (readyState = 0) without dispatching the readystatechange event.

http://www.w3.org/TR/XMLHttpRequest/#abort
http://msdn2.microsoft.com/en-us/library/ms535920.aspx

Mozilla however does send the event and sets state to DONE (readyState = 4).
Then getting the status property throws the exception Teun mentioned. The W3 spec calls for an INVALID_STATE_ERR exception.

Unfortunately "everybody" uses the onreadystatechange handler for callbacks from XHR. This bug makes abort very hard to use. We really should be able to sanely abort XHR for "Web 2.0".

Dear drivers,
IMO this should be Confirmed, Platform All and I'm requesting Blocking 1.9.
Thank you for your time!

confirmed for:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007121305 Minefield/3.0b3pre
Flags: blocking1.9?
Jonas, can you please determine if this should block and set a priority?
This does indeed look bad. Should be low-risk-high-value to fix.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
That's great news, thank you.

This should also be back-ported to Mozilla 1.8 of course. Is it appropriate to request/set blocking 1.8.x and which one would be realistic?
Sorry, this is not appropriate for 1.8 as it's a functional change. We only do crash, security, and regression fixes in stability releases.
I beg to differ! ;) "FUEL is new in Firefox 3 and will be backported to Firefox 2 as well." (http://developer.mozilla.org/en/docs/FUEL) But maybe that doesn't count because it's additional functionality?
Another source code to reproduce this bug:


1.php:

<?
 sleep(4);
 print('');
?>


ajax.htm:

<body>
 <div id="info"></div>
 <script type="text/javascript">

  i=1;t0=+new Date()
  function add_rs(r){
   document.getElementById('info').innerHTML+=(i++)+'. '+(+new Date()-t0)+'ms:'+r+'<br/>'
  }

  r=new XMLHttpRequest()
  r.onreadystatechange=function(){add_rs(r.readyState+' [from onreadystatechange event]')}
  r.open('POST','1.php?'+Math.random(),1)
  r.send('')
  setTimeout(function(){add_rs('[now aborted]');r.abort();add_rs(r.readyState)},1000)

 </script>
</body>




Actual Results:
1.    0ms: 1 [from onreadystatechange event]
2.    0ms: 1 [from onreadystatechange event]
3. 1000ms: [now aborted]
4. 1000ms: 4 [from onreadystatechange event]
5. 1000ms: 0

4) W3C: Switch the state to UNSENT (0). (Do not dispatch the readystatechange event.)


Expected Results:
1.    0ms: 1 [from onreadystatechange event]
2.    0ms: 1 [from onreadystatechange event]
3. 1000ms: [now aborted]
4. 1000ms: 0
Ben, you want this one?
Assignee: general → bent.mozilla
Please change OS from 'Linux' to 'All'.
confirmed for Windows platform:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008013004 Minefield/3.0b3pre
Sure.
Status: NEW → ASSIGNED
OS: Linux → All
QA Contact: ian → general
Attached patch Patch — — Splinter Review 
We were indeed changing the ready state in some cases when we shouldn't have been. This patch follows the spec. Also found that we were needlessly addrefing/releasing listeners in ChangeState, so I fixed that too.

One worry I have is that the old code cleared event listeners on Abort. I couldn't find anywhere in the spec where it says that that should happen. Am I missing something?
Attachment #300519 - Flags: review?(jonas)
Comment on attachment 300519 [details] [diff] [review]
Patch

Oh, and we weren't clearing the response body like the spec says we should. Fixed that too.
Target Milestone: --- → mozilla1.9beta4
Attachment #300519 - Flags: superreview+
Attachment #300519 - Flags: review?(jonas)
Attachment #300519 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: late-compat
Resolution: --- → FIXED
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021904 Minefield/3.0b4pre

Not fixed!
Still on abort dispatch the readystatechange event (wrong behaviour) and then readyState=4.
lunter: Could you attach a minimal testcase showing the behaviour you think is wrong and describe what you think the right behaviour should be.
I have also tested this again (with my original testcase) using the same build as lunter and with the same result as lunter.

This was in violation of the specs (and generally retarded in my opinion) when this bug was launched.

However http://www.w3.org/TR/XMLHttpRequest/
W3C Working Draft 26 October 2007

Seems the specs have been changed...

1. ...
2. ...

3. If the state is UNSENT, OPENED and the send() flag is "false", or DONE go to the next step.

Otherwise, switch the state to DONE, set the send() flag to "false" and synchronously dispatch a readystatechange event on the object.

4. Switch the state to UNSENT. (Do not dispatch the readystatechange event.)


In the test case the connection is already open and indeed send() has been called before the abort() method. So i guess the original bug is now in alignment with the current working specs from w3c.
(In reply to comment #21)
> So i guess the original bug is now in alignment with the current
> working specs from w3c.

I'm not really following your comment. If you think that we're not following the spec properly can you please attach a testcase or explain further?
When the bug was launched the w3c said to not dispatch the readystate event and set state straight to 0. That functionality being the opposite of what gecko did.

The working draft however _now_ states to do exactly what gecko does.

Now ironically opera and others are incorrect in their implementation.
@lunter: Could you attach a minimal testcase showing the behaviour you think is
wrong and describe what you think the right behaviour should be.

Yes, I can.
https://bugzilla.mozilla.org/attachment.cgi?id=300331

WC3 says that:
http://www.w3.org/TR/XMLHttpRequest/#abort
"4. Switch the state to UNSENT. (Do not dispatch the readystatechange event.)"

Now gecko calls readystatechange event on abort(). (Opera and Safari do not dispatch).

IMHO abort() must switch the state to UNSENT (readyState = 0) without
dispatching the readystatechange event.


1.php:

<?
 sleep(4);
 print('');
?>


ajax.htm:

<body>
 <div id="info"></div>
 <script type="text/javascript">

  i=1;t0=+new Date()
  function add_rs(r){
   document.getElementById('info').innerHTML+=(i++)+'. '+(+new
Date()-t0)+'ms:'+r+'<br/>'
  }

  r=new XMLHttpRequest()
  r.onreadystatechange=function(){add_rs(r.readyState+' [from
onreadystatechange event]')}
  r.open('POST','1.php?'+Math.random(),1)
  r.send('')
  setTimeout(function(){add_rs('[now
aborted]');r.abort();add_rs(r.readyState)},1000)

 </script>
</body>




Actual Results:
1.    0ms: 1 [from onreadystatechange event]
2.    0ms: 1 [from onreadystatechange event]
3. 1000ms: [now aborted]
4. 1000ms: 4 [from onreadystatechange event]
5. 1000ms: 0

4) W3C: Switch the state to UNSENT (0). (Do not dispatch the readystatechange
event.)


Expected Results:
1.    0ms: 1 [from onreadystatechange event]
2.    0ms: 1 [from onreadystatechange event]
3. 1000ms: [now aborted]
4. 1000ms: 0




PS. abort() method in future will be dispach onabort event, http://www.w3.org/TR/XMLHttpRequest/#abort-err
Attached file a minimal testcase — 

    
    

    
  
lunter, see my comment above

"If the state is UNSENT, OPENED and the send() flag is "false", or DONE go to the next step.

_Otherwise_, switch the state to DONE, set the send() flag to "false" and synchronously dispatch a readystatechange event on the object."


in this case
state = OPENED && send() = true
therefore switch state to done and dispatch readystate event



    
    

    
  
a minimal testcase:
https://bugzilla.mozilla.org/attachment.cgi?id=304723

Now on abort() script dispatches onreadystatechange and readyState = 4 and r.status = 2147746065 (???)

Should be: on abort() script does not dispatch onreadystatechange [and readyState = 0]

    
    

    
  
"If the state is UNSENT, OPENED and the send() flag is "false", or DONE go to the next step."

Yes ...but on abort() the state is HEADERS_RECEIVED or LOADING !!

    
    

    
  
Ok, my mistake :/

...but what is r.status = 2147746065 on abort() ???

    
    

    
  
(In reply to comment #30)
> Ok, my mistake :/
> 
> ...but what is r.status = 2147746065 on abort() ???
> 
2147746065 is NS_ERROR_NOT_AVAILABLE
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpcom/base/nsError.h&rev=1.52&mark=200#198

Don't know why it is returned in ::GetStatus
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsXMLHttpRequest.cpp&rev=1.221&mark=1075#1059

But see also comment https://bugzilla.mozilla.org/show_bug.cgi?id=304980&mark=29#c29
Component: DOM → DOM: Core & HTML

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: