synchronous XMLHttpRequest does not fire "readystatechange" event

RESOLVED FIXED

Status

()

Core
XML
P1
normal
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Alex Lein, Assigned: smaug)

Tracking

Trunk
x86
All
Points:
---
Bug Flags:
blocking1.9.1 -
blocking1.9 -
wanted1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7

When I create an XMLHttpRequest object and use the open() method with syncronious mode flagged, FF 1.0.x will not fire the "readystatechange" event...  FF 1.5b2 simply freezes.

Reproducible: Always

Steps to Reproduce:
1. Visit my testcase http://www.baka.ca/test/sjax.htm
2. Count the number of alert()s that popup.
Actual Results:  
The alert() is shown 5 times (for async mode only).

Expected Results:  
The alret() should be shown 10 times. 5 times for sync mode, and more 5 times for async mode.

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: XMLHttpRequest in syncronious mode does not fire "readystatechange" event. → synchronous XMLHttpRequest does not fire "readystatechange" event
Seems extremely unlikely that this is a Firefox:General bug, as opposed to an XML(HttpRequest) bug.
Assignee: nobody → xml
Component: General → XML
Product: Firefox → Core
QA Contact: general → ashshbhatt
Version: unspecified → Trunk

Updated

12 years ago
OS: Windows 2000 → All

Comment 2

12 years ago
I've found this bug too, and almost filed it as a new bug. 

Anyway I confirmed that it's a bug for Firefox 1.0.6, 1.0.7, Mozilla and Firefox latest nightly builds (as of today), so it must be the core code that caused this bug.  IE, on a separate note, behaves correctly.
The XMLHttpRequest code does:

1849   if ((mState & XML_HTTP_REQUEST_ASYNC) &&
1850       (aState & XML_HTTP_REQUEST_LOADSTATES) && // Broadcast load states only
1851       aBroadcast &&
1852       onReadyStateChangeListener) {

hence this bug.  Does IE really fire onreadystatechanged for sync XMLHttpRequests in all cases? If so, we probably should too...

Comment 4

12 years ago
Yeah, I'm pretty sure IE fires the event on sync-ed requests, and Mozilla family should do that too.  Otherwise, a simple task that I was facing cannot be solved in Mozilla browsers.  The task is onSubmit, first send a sync-ed request to a script checking username/password for DB permission, and if and only if that passes, submit the form data; otherwise give user a warning.  My sync-ed script only worked in IE, not Mozilla browsers.  I tried a few possible workarounds and none worked.  Seems to me that some tasks that need coordination just can't be accomplished by async calls in a reliable way.
Flags: blocking1.9a1?

Comment 5

11 years ago
(In reply to comment #3)

> hence this bug.  Does IE really fire onreadystatechanged for sync
> XMLHttpRequests in all cases? If so, we probably should too...

It's reported in the Korean mozilla user forum that Opera does that, too. I'm not sure of 'in all cases' part, though.
*** Bug 337810 has been marked as a duplicate of this bug. ***

Comment 7

11 years ago
I have taken good time trying to make a synchronous request work, but it really doesn´t. On IE, these control works fine, but at Mozilla it´s broken. The XMLHTTPRequest responseText propertie is returning allways 'undefined', via window.alert simple test case. Plus: it seems to be discarding the 'false' flag at the open method. Aditional info: i´m trying to do this, by posting form data (i.e.  XMLHTTPRequest.open("POST",url,false), XMLHTTPRequest.setRequestHeader('Content-Type','application/x-www-form-urlencoded') and XMLHTTPRequest.send(data)).

Comment 8

11 years ago
I did the same as comment 7 (except using ajax.open("post", "ebridge.php");), but lately ran into this bug. Odd that it used to work...

Comment 9

11 years ago
Created attachment 234988 [details]
Workaround for Synch/Asynch AJAX Calls.

Seen this behavior as well. Here is a workaround. Since Firefox always calls the Firefox-only ONLOAD event handler, you can always bind to it. (Note, IE always calls ONREADYSTATECHANGED correctly).

Comment 10

11 years ago
I make a web page that points a curve fit for that issue. If someone is looking for a solution for that this implementation. The url is http://www.fernandowendt.eti.br/bugzilla/. Maybe help.
Flags: blocking1.9a1? → blocking1.9-
Whiteboard: [wanted-1.9]
*** Bug 353808 has been marked as a duplicate of this bug. ***

Comment 12

10 years ago
http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/Overview.html?content-type=text/html;%20charset=utf-8 should be clear on when the events and state changes are to be done. I also wrote a testcase for synchronous requests which passes in Internet Explorer 7:

  http://tc.labs.opera.com/apis/XMLHttpRequest/readyState/002.htm

Updated

10 years ago
Duplicate of this bug: 383304

Comment 14

10 years ago
As stated in my Bug 383304 report, having the Firebug extension installed fixes this, so it is a solvable problem. My report also includes a link to an example of the bug and an inline workaround for web pages that need this to work as-is. The workaround is future compatible.

Comment 15

9 years ago
I don't know if others have noticed this, but in my case in FF 2.0.0.11
Synchronous works if I place an alert after the send call.

The onreadystatechanged handler is called, but if I don't place an alert
after the send call, the send method returns before the handler 
completes.

But there is something "special" about alert. If I use a spin loop
after the send, that still does not have the same effect as calling
alert. Its almost as if the alert call "flushes" or causes "listeners"
to be invoked. Anyway those are the symptoms I see.

-rick
Flags: wanted1.9+
Whiteboard: [wanted-1.9]

Comment 16

9 years ago
Created attachment 331311 [details]
simple_sjax

Comment 17

9 years ago
confirm on FF 3.0.1


<script>

  r=new XMLHttpRequest()
  r.onreadystatechange=function(){if(r.readyState==4)alert(r.responseText)}
  r.open('GET','1.txt',false)
  r.send(null)

</script>

Updated

9 years ago
Duplicate of this bug: 446210
Duplicate of this bug: 441988

Comment 20

9 years ago
I ran into this also and came up with a simple workaround:

After my onreadystatechange and send methods, I fire my callback
method manually -- when in Firefox and synchronous mode -- inside
this test:

if (!async && navigator.userAgent.indexOf("Firefox") != -1) {
  // call the processing method here
}

A picture is worth 1k words, so here is my ajax lib source listing
where you can find this code near the end.  Hope this helps.

http://ajamyajax.com/ajaMyCore.html

Comment 21

9 years ago
better:

if (!async && navigator.userAgent.indexOf("Gecko") > -1) {
  // call the processing method here
}

Comment 22

9 years ago
That would add Chrome, Safari, etc.  Have you seen this synchronous
issue there also??  Thank you.

Comment 23

9 years ago
Chrome, Safari, Opera, IE fire "readystatechange" event.
Yeah, all parties agree that we're doing the wrong thing here I think. So lets get it fixed for next release.
Assignee: xml → jonas
Flags: blocking1.9.1+
Priority: -- → P2
On second thought this is not a blocker, but a wanted P1.
Flags: blocking1.9.1+ → blocking1.9.1-
Priority: P2 → P1

Updated

8 years ago
Duplicate of this bug: 493565
QA Contact: ashshbhatt → xml
Created attachment 398209 [details] [diff] [review]
Patch v1

This patch removes the restriction against firing onreadystatechange for synchronous XHRs.

It does not pass the OP's testcase, however, because firing an alert within the sync onreadystatechange handler somehow prevents send() from completing.  We think that this behavior might be related to bug 501501.
Attachment #398209 - Flags: review?(Olli.Pettay)
Duplicate of this bug: 503061
(Assignee)

Comment 29

8 years ago
(In reply to comment #27)
> because firing an alert within the
> sync onreadystatechange handler somehow prevents send() from completing.
I can't reproduce this problem. I tried on Linux and OSX.
Which testcase were you using?
(In reply to comment #29)

Just to be clear, I meant in comment #27 that if I load http://www.baka.ca/test/sjax.htm in Firefox with my patch, it never begins the asynchronous XHR, because the first XHR doesn't complete its send().  If I don't fire any alerts in the sync onreadystatechange handler, then the sync's send() does complete.
(Assignee)

Comment 31

8 years ago
Comment on attachment 398209 [details] [diff] [review]
Patch v1

Unfortunately this isn't enough. The problem is that alert() starts a new syncloop already before XHR's syncloop and when it exists, XHR has already been loaded, but XHR syncloop is just started.

I'll post a bit different patch.
Attachment #398209 - Flags: review?(Olli.Pettay) → review-
(Assignee)

Comment 32

8 years ago
Created attachment 398450 [details] [diff] [review]
patch

Justin, want to try this one?
Attachment #398209 - Attachment is obsolete: true
Attachment #398450 - Flags: review?(jlebar)
Comment on attachment 398450 [details] [diff] [review]
patch

Looks good to me.

I noticed that the spec calls for sync XHRs to dispatch events differently than async XHRs.  Does this bring us more in line with the spec, or should we work on getting it changed to match our (and everyone else's) behavior?
Attachment #398450 - Flags: review?(jlebar) → review+

Updated

8 years ago
Attachment #398450 - Flags: superreview+
(Assignee)

Comment 34

8 years ago
Yeah, our readystatechange handling isn't quite what the spec says.
Both async and sync behave a bit differently.
I think a followup bug would make sense. When changing the readystatechange
handling, have to make sure that the XHR2 draft specifies things properly.

Comment 35

8 years ago
FWIW, the specification requires that you do not dispatch events during synchronous requests.
(Assignee)

Comment 36

8 years ago
oh, even readystatechange events?
I thought only progress events.
Things have changed again...
(Assignee)

Comment 37

8 years ago
Anyway, if that is the case, then this bug is invalid.
(And I agree, events shouldn't be dispatched.)

Comment 38

8 years ago
I told you when I changed them and you told me you were happy ;-)
As far as I can tell, everyone (Chrome 2, Safari 4, IE 8, Opera 10) dispatches
onreadystatechange events on sync XHR.  Based on the number of duplicates of
this bug, I think it's fair to say that FF's behavior is confusing a lot of web
developers.  Is that sufficient reason to change the spec and/or our behavior?
(Assignee)

Comment 40

8 years ago
(In reply to comment #38)
> I told you when I changed them and you told me you were happy ;-)
Apparently I forgot the readystatechange part, and remembered only progress events.
And seems  like readystatechange is still dispatched with sync, at least in some cases, like after open().

But it is great if we don't have to dispatch those events.
readystatechange events are legacy. New code should use progress events.
My feelings are this:

On one hand sync XHR is crappy, and so I don't think adding features for people using it is that high on my list. And it's nice to be consistent in not firing *any* events during sync XHR.

On the other hand, if every other UA is firing readystatechange during sync XHR, it seems easier to standardize on that. Getting every other implementation to remove a feature is a lot of work. Especially a feature that people are clearly using.


So: All in all I think I'd be in favor of changing the spec and fixing this bug.
So to make it clear: The only reason for dispatching any events during sync requests is for compat with other UAs, and existing content. From that point of view it doesn't (IMHO) make sense to dispatch progress events.

Comment 43

8 years ago
This really be discussed on the list, but to clarify, some readystatechange events are still dispatched. After open(), but also after a successful request. See "switch to the DONE state". Just not during a request as the event loop is busy at that time with the request.
(Assignee)

Comment 44

8 years ago
(In reply to comment #43)
> After open(), but also after a successful request.
> See "switch to the DONE state". Just not during a request as the event loop is
> busy at that time with the request.

Right, this makes sense. I think almost all the cases when I've seen readystatechange to be used with sync XHR, it has been used to to detect readyState 4.
I'll modify the patch a bit so that only 1 and 4 are reported when xhr is sync.
Does that sound ok to you Jonas?


Thanks Anne for the clarification!
That makes sense. I think the most important thing here is to align all UAs to a particular behavior. What that behavior is feels less important to me.
(Assignee)

Comment 46

8 years ago
Created attachment 399734 [details] [diff] [review]
dispatch readystatechange when readyState == 1 or 4

This is what the XHR2 spec says for normal loads.

Error and abort handling need some work, but those are separate cases.
When we take the patch (in some form), I'll file followups to fix error and
abort.
We also need add support for loadend (Bug 461066), which the draft does specify
nowadays.
Attachment #399734 - Flags: review?(jonas)
Comment on attachment 399734 [details] [diff] [review]
dispatch readystatechange when readyState == 1 or 4

Would be good to have tests that do the evil things that would have broken Justin's initial patch. I take it it's stuff like calling .abort/.open from within the readystatechange handler.
Attachment #399734 - Flags: review?(jonas) → review+
(Assignee)

Comment 48

8 years ago
(In reply to comment #47)
> Would be good to have tests that do the evil things that would have broken
> Justin's initial patch.
The testcase in the patch does do that; starting a new "event loop"
in a readystatechange listener.
Ooh, I thought the concern was when the call to the eventhandler would change the existing XHR objects state in unexpected ways.
(Assignee)

Updated

8 years ago
Assignee: jonas → Olli.Pettay
(Assignee)

Comment 50

8 years ago
http://hg.mozilla.org/mozilla-central/rev/36a582d5c645
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 51

8 years ago
Backed out to see if this caused memory usage regression on linux.
http://hg.mozilla.org/mozilla-central/rev/834a55bc458b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Olli, any update on this bug? Given by bug 503061 this is a regression?
(Assignee)

Comment 53

8 years ago
I can't see when this would have regressed.

Anyway, I haven't figured out why memory usage goes up on linux.
The memory leak was only on linux or also other platforms?
Status: REOPENED → ASSIGNED
(Assignee)

Comment 55

8 years ago
Linux only, and it wasn't a leak, but memory usage going up.
(Assignee)

Comment 56

8 years ago
IIRC, I did retry this on tryserver with the same results.
I could try again.
When you do this which tool we could use to track the memory consumption under Linux? Valgrind? Which steps someone would have to use to simulate this excessive memory consumption? Just running the testcase from the patch?

Would be great if you can push it again.
(Assignee)

Comment 58

8 years ago
(In reply to comment #57)
> Which steps someone would have to use to simulate this
> excessive memory consumption? 
Run tp4 and measure the memory usage the same way as talos does.
I haven't done that locally. I have no idea how to run talos locally.
(Assignee)

Comment 59

8 years ago
I posted this to tryserver yesterday and there linux tp4 shows again peak in
tp4_pbytes and tp4_rss
What is tp4? Would be great if you could point me to a documentation. Does it run all the automated tests?
(Assignee)

Comment 61

7 years ago
I posted this again to tryserver.
(Assignee)

Comment 62

7 years ago
Created attachment 475896 [details] [diff] [review]
up-to-date
(Assignee)

Comment 63

7 years ago
Comment on attachment 475896 [details] [diff] [review]
up-to-date

So this time tryserver didn't show tp4 tp4_pbytes/tp4_rss regressions.
Have we fixed something else? Don't know why there was regressions earlier, 
but we could try to get this in.
Attachment #475896 - Flags: approval2.0?
Attachment #475896 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 64

7 years ago
http://hg.mozilla.org/mozilla-central/rev/268ef4ccb5ff

Waiting for the test results before marking this fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 65

7 years ago
This time tp4_pbytes and tp4_rss look ok.

Marking this fixed, but I'll reopen/back-out if some other
test shows regression.

Comment 66

6 years ago
Hi,
Will this be backported to FF 3.6 branch?
As of now I don't see it in _tip_: http://hg.mozilla.org/releases/mozilla-1.9.2/log/tip/content/base/src/nsXMLHttpRequest.cpp

Thanks.
(Assignee)

Comment 67

6 years ago
(In reply to comment #66)
> Hi,
> Will this be backported to FF 3.6 branch?
No.

3.6 is in stability-and-security-fixes only mode.
You need to log in before you can comment on or make changes to this bug.