Matthew (lilmatt) Willis Reporter
	Description • 17 years ago

The readyState integers for XMLHttpRequest are not defined as constants but are instead listed in a comment in the IDL.

This leads to code like this:

  this.req = CC[XMLHTTPREQUEST_CONTRACTID].
             createInstance(CI.nsIXMLHttpRequest);
  this.req.QueryInterface(CI.nsIJSXMLHttpRequest);
  this.req.open("GET", aUrl, true);
  var req = this.req;
  this.req.onreadystatechange = function (aEvt) {
    if (req.readyState == 4) {
      try {

We should move the values from the comments into constants for the readyStates.  This makes the code more readable and gets rid of some "magic numbers".

	Boris Zbarsky [:bzbarsky]
	Comment 1 • 17 years ago

How does this align with the W3C spec for this stuff?

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 17 years ago

In particular, I'd rather not introduce constants that would not interoperate with other UAs...

	Matthew (lilmatt) Willis Reporter
	Comment 3 • 17 years ago

Good call.  They're named differently than the original comment in the idl.  I'll make a new patch using the W3C terms.

See http://www.w3.org/TR/XMLHttpRequest/#xmlhttprequest

	Matthew (lilmatt) Willis Reporter
	Comment 5 • 17 years ago

Shortens constant names since "XMLHTTPREQUEST" is included in CI.nsIXMLHttpRequest.

Also makes them unsigned shorts

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 17 years ago

Comment on attachment 274082 [details] [diff] [review]
rev3 - Use exact terms from spec per bz

Thanks!

	Boris Zbarsky [:bzbarsky]
	Comment 9 • 17 years ago

Comment on attachment 274109 [details] [diff] [review]
rev4 - Includes missing implementation

Isn't "short" PRUint16?  Check the generated header from this IDL?

	Matthew (lilmatt) Willis Reporter
	Comment 10 • 17 years ago

Comment on attachment 274109 [details] [diff] [review]
rev4 - Includes missing implementation

Removing review request.  I will try again after I get sleep.

	Anne (:annevk)
	Comment 11 • 17 years ago

Please note that SENT has been renamed to HEADERS_RECEIVED in the editor's draft: http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/Overview.html?content-type=text/html;%20charset=utf-8

	Anne (:annevk)
	Comment 12 • 17 years ago

fantasai told me that OPEN clashed with open() because IDLs are not case-sensitive. I'm not really sure what a better name would be though. READY, maybe?

	Matthew (lilmatt) Willis Reporter
	Comment 13 • 17 years ago

(In reply to comment #12)
Microsoft previously used "LOADING"
http://msdn2.microsoft.com/en-us/library/ms753800.aspx

however they appear to now follow the draft:
http://msdn2.microsoft.com/en-us/library/ms534361.aspx

Perhaps we could simply prepend "READYSTATE_" to the constants?  IMO doing so makes them more readable in code as it explicity indicates these are readyState values.
  (ex: "XMLHttpRequest::READYSTATE_OPEN" vs."XMLHttpRequest::OPEN")

	Anne (:annevk)
	Comment 14 • 17 years ago

I wouldn't trust Microsoft documentation. It is inaccurate. (Microsoft employees admitted as much which is part of the reason that SENT hsa been renamed to HEADERS_RECEIVED for instance.)

Prepending it with READYSTATE_ is not consistent with similar APIs (such as HTMLMediaElement from HTML 5 or various DOM features) so I would rather not do that. The names should be as concise and accurate as possible, imo.

	Jeff Walden [:Waldo]
	Comment 15 • 17 years ago

Given that all the names except LOADING are past-tense, why not OPENED?

	Anne (:annevk)
	Comment 16 • 17 years ago

Cool, UNSENT, OPENED, HEADERS_RECEIVED, LOADING and DONE it is.

	Anne (:annevk)
	Comment 17 • 17 years ago

FWIW, the editor's draft mentioned in comment 11 has been updated to reflect this change.

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 18 • 12 years ago

Was fixed in bug 649133.