361222 - [FIX]XMLHttpRequest API documentation for event listeners is bogus

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

The checkin for bug 204545 added the following comment to all the event listener stuff in nsIXMLHttpRequest.idl:

304 heikki   1.19         * After the initial response, all event listeners will be cleared.
305                       * Call open() before setting new event listeners.

Then the checkin for bug 251418 copy/pasted that comment for the onprogress listener, and the same information has made it to both the "Monitoring progress" example in <http://developer.mozilla.org/en/docs/XMLHttpRequest> and the "Remarks" section in <http://developer.mozilla.org/en/docs/nsIJSXMLHttpRequest>.

I don't see why heikki's original comment said what it said (it's quite ok to set event listeners before open(), even with his patch), and for onprogress this comment is just false -- onprogress MUST be set before open() is called, since open() does things differently if it's set.  The example in <http://developer.mozilla.org/en/docs/XMLHttpRequest> just doesn't work as written, as a result.

I'll post a patch to the trunk API docs in a sec.  Do we also want to change the API docs on the branches?

Also, I'm not quite willing to update the devmo docs myself because I can't tell which parts of the skew from the API docs are on purpose....

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Comment fix

Though I suppose we could also make setting onprogress work after open(); we'd just need to twiddle the relevant channel flag in send() instead of doing it in open().  Arguably, we'll need to do this anyway if we're going to comply with the W3C-proposed draft...  Unfortunately, that will mean an incompatible change, but oh, well.  People who need backwards compat will be able to set the listener twice.

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 245968 [details] [diff] [review]
Comment fix

   ACString getResponseHeader(in AUTF8String header);
[...]
+   * been called alread), this is equivalent to calling abort().

s/alread/already/

r+sr=jst

Comment 3

[Boris Zbarsky [:bzbarsky]]

Fixed API docs... need to do something similar for devmo, I guess.

Comment 4

[Eric Shepherd [:sheppy]]

Revising the doc on MDC is on my to-do list.  Hope to get it done today.

Comment 5

[Eric Shepherd [:sheppy]]

Interestingly, I actually tested the code snippet on the XMLHttpRequest page, and it worked fine for me with onprogress being set after the open.  However, I've moved it to before the open on that page now.

Comment 6

[Eric Shepherd [:sheppy]]

Since nobody has commented on the change I made a few months ago, I'm tagging this as dev-doc-complete.