Closed
Bug 307769
Opened 19 years ago
Closed 19 years ago
document the fact that XMLHttpRequest::Send doesn't support streams with headers
Categories
(Core :: XML, defect)
Core
XML
Tracking
()
RESOLVED
FIXED
People
(Reporter: Biesinger, Assigned: darin.moz)
References
()
Details
(Keywords: fixed1.8, regression)
Attachments
(1 file, 1 obsolete file)
2.02 KB,
patch
|
mtschrep
:
approval1.8b5+
|
Details | Diff | Splinter Review |
XMLHttpRequest always passes a content type to SetUploadStream, which means that any possibly existing headers in the input stream will be sent as data. In 1.7.x, that was different: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp&rev=MOZILLA_1_7_BRANCH&cvsroot=/cvsroot&mark=1487#1483 We should maybe still allow streams with headers.
Reporter | ||
Updated•19 years ago
|
Comment 1•19 years ago
|
||
Either that or fix the documentation accordingly... In any case, we need to do one or the other for 1.8, imo. Darin, bsmedberg, thoughts?
Flags: blocking1.8b5?
Comment 2•19 years ago
|
||
How does this affect web authors?
Comment 3•19 years ago
|
||
It doesn't, but it affects extension authors or any other UniversalXPConnect code that's using XMLHttpRequest.
Comment 4•19 years ago
|
||
When we fix this, we should make sure not to cause bug 308484 on the 1.8 branch.
Assignee | ||
Comment 5•19 years ago
|
||
Why should we fix this? Do we know of many extensions that try to set headers at the top of their data stream? If possible, we should drop support for headers in the data stream. It is a messy way of specifying headers. It looks like the change was introduced by the patch for bug 246651.
Reporter | ||
Comment 6•19 years ago
|
||
at least nsIXMLHttpRequest.idl needs to be fixed, which says the headers need to be specified in the stream.
Assignee | ||
Comment 7•19 years ago
|
||
Attachment #196262 -
Flags: superreview?(jst)
Attachment #196262 -
Flags: review?(cbiesinger)
Comment 8•19 years ago
|
||
I'd be fine with us changing the IDL appropriately, sure. As long as our IDL and impl are consistent and we have _a_ way to set headers, I'm happy not to do it in the stream (which I agree is a messy way).
Reporter | ||
Comment 9•19 years ago
|
||
Comment on attachment 196262 [details] [diff] [review] v1 patch: fix comments + * compatible with nsIUploadChannel.setUploadStream, and a I think it should be made clearer that this function expects no headers in the stream - nsIUploadChannel.setUploadStream can behave either way. should it be mentioned that a content-type for the uploaded data can be set using setRequestHeader, or is that obvious?
Comment 10•19 years ago
|
||
Comment on attachment 196262 [details] [diff] [review] v1 patch: fix comments sr=jst
Attachment #196262 -
Flags: superreview?(jst) → superreview+
Updated•19 years ago
|
Flags: blocking1.8b5? → blocking1.8b5+
Assignee | ||
Comment 11•19 years ago
|
||
> I think it should be made clearer that this function expects no headers in the
> stream - nsIUploadChannel.setUploadStream can behave either way.
>
> should it be mentioned that a content-type for the uploaded data can be set
> using setRequestHeader, or is that obvious?
good points. i'll extend the comments to mention those.
Reporter | ||
Comment 12•19 years ago
|
||
Comment on attachment 196262 [details] [diff] [review] v1 patch: fix comments r=biesi with those comment changes.
Attachment #196262 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 13•19 years ago
|
||
OK, here's the version that I checked in. I would like to land this on the 1.8 branch for consistency.
Assignee: xml → darin
Attachment #196262 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #196381 -
Flags: approval1.8b5?
Assignee | ||
Comment 14•19 years ago
|
||
marking FIXED
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Summary: XMLHttpRequest::Send doesn't support streams with headers → document the fact that XMLHttpRequest::Send doesn't support streams with headers
Comment 15•19 years ago
|
||
Could we add a quick blurb about this change linked off http://developer.mozilla.org/docs/Adapting_XUL_Applications_for_Firefox_1.5 ?
Comment 16•19 years ago
|
||
Comment on attachment 196381 [details] [diff] [review] v1.1 patch Can you write a blurb for an update to devmo and send to cbeard/deb?
Attachment #196381 -
Flags: approval1.8b5? → approval1.8b5+
Assignee | ||
Comment 18•19 years ago
|
||
(In reply to comment #15) > Could we add a quick blurb about this change linked off > http://developer.mozilla.org/docs/Adapting_XUL_Applications_for_Firefox_1.5 ? Done
You need to log in
before you can comment on or make changes to this bug.
Description
•