Bug 26979

Summary: Send form data via XMLHttpRequest
Product: WebKit Reporter: Alfonso Martínez de Lizarrondo <amla70>
Component: XMLAssignee: Jian Li <jianli>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ap, dimich, jianli, rui.coelho, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 35707, 36024    
Bug Blocks:    
Attachments:
Description Flags
Proposed Patch
jianli: commit-queue-
Proposed Patch dimich: review-, jianli: commit-queue-

Description Alfonso Martínez de Lizarrondo 2009-07-05 08:37:36 PDT
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...

--42147134553451418--
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 Alfonso Martínez de Lizarrondo 2009-07-06 07:02:01 PDT
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 rui.coelho 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 Alexey Proskuryakov 2010-02-26 19:42:23 PST
*** Bug 35452 has been marked as a duplicate of this bug. ***
Comment 4 Jian Li 2010-03-01 11:58:25 PST
The requested feature is defined in XMLHttpRequest 2 spec
(http://dev.w3.org/2006/webapi/XMLHttpRequest-2/#formdata).
Comment 5 Jian Li 2010-03-01 12:07:48 PST
Created attachment 49733 [details]
Proposed Patch
Comment 6 Jian Li 2010-03-01 15:55:53 PST
Created attachment 49761 [details]
Proposed Patch
Comment 7 WebKit Review Bot 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 Dmitry Titov 2010-03-02 13:22:16 PST
Comment on attachment 49761 [details]
Proposed Patch

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.

Also:

> 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. 

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

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 Jian Li 2010-03-15 10:38:49 PDT
All related patches are landed. Closed as resolved.