Bug 26979 - Send form data via XMLHttpRequest
: Send form data via XMLHttpRequest
: WebKit
: 528+ (Nightly build)
: All All
: P2 Enhancement
Assigned To:
: 35707 36024
  Show dependency treegraph
Reported: 2009-07-05 08:37 PST by
Modified: 2010-03-15 10:38 PST (History)

Proposed Patch (51.24 KB, patch)
2010-03-01 12:07 PST, Jian Li
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Proposed Patch (62.54 KB, patch)
2010-03-01 15:55 PST, Jian Li
dimich: review-
jianli: commit‑queue-
Review Patch | Details | Formatted Diff | Diff


You need to log in before you can comment on or make changes to this bug.

Description From 2009-07-05 08:37:36 PST
Webkit does allow to send a file from an <input type="file"> with XmlHttpRequest.send, but when it does so, the contents of the body sent to the server are just the contents of the file, without the form boundaries like any other form does.

In Firefox it's possible to call file.getAsBinary() to get the contents as a string and manually construct the full form data and then send it with XmlHttpRequest.sendAsBinary

The current behavior means that special coding is required at the server end to accept this special uploads, it's not possible to change only the client side to get an ajax upload as it's possible with the Firefox system.

This problem might need to be fixed before multiple files upload (https://bugs.webkit.org/show_bug.cgi?id=25923) is possible, or all the files would be sent one after another.

Ideally, when sending a file, the 
Content-Type: multipart/form-data; boundary=--42147134553451418--

header should be added, and in the body the boundary should be created also, that would make it very easy to use. The missing part is the field name...

Content-Disposition: form-data; name="NewFile"; filename="myPic.jpg"
Content-Type: application/octet-stream

But the important part is to have some method to properly send the file as a normal form would do, without any change to the server code to parse "raw" data. (it does lack even the file name!)
------- Comment #1 From 2009-07-06 07:02:01 PST -------
After thinking about this issue, I think that the most straightforward and friendly solution would be to allow using the <form> element in the xhr.send() method, so the browser takes care of everything like in a normal submit, but now being able to keep track of the progress and getting the response from the server.
------- Comment #2 From 2010-02-10 07:46:49 PST -------
If XHR file upload is to replace flash/plugin solutions when we need progress bars or drag-drop then this bug is important. Otherwise we need to change the web server-side code to handle regular form posts (in multipart/form-data) and this uploads.
------- Comment #3 From 2010-02-26 19:42:23 PST -------
*** Bug 35452 has been marked as a duplicate of this bug. ***
------- Comment #4 From 2010-03-01 11:58:25 PST -------
The requested feature is defined in XMLHttpRequest 2 spec
------- Comment #5 From 2010-03-01 12:07:48 PST -------
Created an attachment (id=49733) [details]
Proposed Patch
------- Comment #6 From 2010-03-01 15:55:53 PST -------
Created an attachment (id=49761) [details]
Proposed Patch
------- Comment #7 From 2010-03-01 16:00:04 PST -------
Attachment 49761 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/bindings/v8/custom/V8XMLHttpRequestCustom.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMFormDataCustom.cpp:36:  "JSDOMFormData.h" already included at WebCore/bindings/js/JSDOMFormDataCustom.cpp:32  [build/include] [4]
WebCore/platform/network/FormData.cpp:198:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/platform/network/FormData.cpp:201:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/bindings/v8/V8Index.cpp:73:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/xml/DOMFormData.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/xml/DOMFormData.cpp:48:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/html/HTMLFormElement.cpp:217:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 8 in 46 files

If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #8 From 2010-03-02 13:22:16 PST -------
(From update of attachment 49761 [details])
Woohoo! FormData!

I would recommend to split the patch: 
1. implementation of DOMFormData (all internals, no tests yet)
2. IDL + JSC Bindings + tests
3. V8 Bindings.
#2 and #3 can be landed together but they probably will be reviewed by different people.


> diff --git a/WebCore/platform/network/FormData.cpp b/WebCore/platform/network/FormData.cpp
> +void FormData::appendFormDataListAsMultiPartForm(const FormDataList& list, FormDataBuilder& formDataBuilder, const TextEncoding& encoding, const CString& boundary, Document* document) {

Looking at the name and parameters of this method I think we should refactor this code to make things simpler.

I see what you are doing but I think it can be even cleaner. Since we'll soon need a method to create a DOMFormData from a From element (for "new FormData(nyFormElement)"), it may be better to have a method on HTMLFormElement like: "PassRefPtr<DOMFormData> createDOMFormData()".

Then on FormData, it'd be FormData::create(const DOMFormData&), instead of append...() method. The appending of all the items could be done inside that create() method.

This could avoid passing FormBuilder for example - it can be a local variable of FormData::create. 

It's possible to do this:
var formData = new FormData(myFormElement);
formData.append("another_file", file);

I realize this patch does not yet address this, but please consider how this would affect the shape of this code, to hopefully avoid significant changes in near future...
------- Comment #9 From 2010-03-15 10:38:49 PST -------
All related patches are landed. Closed as resolved.