Summary: | Send form data via XMLHttpRequest | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alfonso Martínez de Lizarrondo <amla70> | ||||||
Component: | XML | Assignee: | 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
Alfonso Martínez de Lizarrondo
2009-07-05 08:37:36 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. 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. *** Bug 35452 has been marked as a duplicate of this bug. *** The requested feature is defined in XMLHttpRequest 2 spec (http://dev.w3.org/2006/webapi/XMLHttpRequest-2/#formdata). Created attachment 49733 [details]
Proposed Patch
Created attachment 49761 [details]
Proposed Patch
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 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... All related patches are landed. Closed as resolved. |