Bug 26979 - Send form data via XMLHttpRequest
Summary: Send form data via XMLHttpRequest
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Jian Li
URL:
Keywords:
: 35452 (view as bug list)
Depends on: 35707 36024
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-05 08:37 PDT by Alfonso Martínez de Lizarrondo
Modified: 2010-03-15 10:38 PDT (History)
6 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
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.