98928 - nsIXMLHttpRequest needs IE-compatible (JS polymorphic) send method

Status: RESOLVED FIXED

(Core :: XML, enhancement, P2)

Description

[Mike Shaver (:shaver emeritus)]

Lots of people want to be able to call the nsIXMLHttpRequest::send method with
either a DOM document or a JS string.  I'll attach a patch that probably doesn't
work, but shows one (good, IMO) approach to the problem.

Remaining issues:

 - factoring the stream-sending code out of nsXMLHttpRequest::SendDocument and
   calling it from SendDocument and SendStream

 - figuring out where to get the nsJSUtils stuff from, since my path-to-headers
   ugliness can't remain, and we still need the actual symbols

 - moving the polymorphic nsIXMLHttpRequest::send to nsIJSXMLHttpRequest

 - updating C++ callers to use sendDocument/sendString

I don't think I'll get a chance to finish this up myself in the near future, but
someone might be motivated enough by IE compatibility to go the rest of the way.
 Maybe even Heikki, if properly incented. =)

Comment 1

[Mike Shaver (:shaver emeritus)]

Attachment: Untested patch to demonstrate strategy.

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I was actually going to do this as soon as jband was finished with his
nsIVariant work (bug 44675, targeted for 0.9.5, has fix but cleanup remains).
Then we can do just by changing the parameter type to send() to be nsIVariant,
and do not need to add new methods.

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix using nsIVariant

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

John, is this how one is supposed to use nsIVariant?

Comment 5

[John Bandhauer]

That looks reasonable to me. Does it work as expected?

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

It compiles, I get no assertions, the JS call comes into the code I expect and I
send the request so I think it works. I am unable to run my testsuite since
green.mcom.com still needs a static IP address.

I'll probably need to add a couple of checks for return values and once I have
tested this I'll ask for reviews.

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Patch with error checking

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Finally now that green is back up I was able to test my latest patch.

jband, could you review please?

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Harish, r? jst sr? I don't seem to be able to send mail right now which I why I
am asking here...

Comment 10

[John Bandhauer]

Comment on attachment 58803 [details] [diff] [review]
Patch with error checking

r/sr=jband
Heikki, Sorry I forgot about your review request. Looks good to me.

Comment 11

[Johnny Stenback (:jst)]

Space after comma! I.e. |foo, bar|, not |foo,bar|, or at least be consistent.

r=jst if you fix that.

Comment 12

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed.