Summary: | Support using FormData to send a sliced file via XHR. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jian Li <jianli> | ||||||||||||||
Component: | WebCore JavaScript | Assignee: | Jian Li <jianli> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, dimich, eric, gustavo, webkit-ews, webkit.review.bot, xan.lopez | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Bug Depends on: | 36472 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Jian Li
2010-03-26 14:52:02 PDT
Created attachment 51787 [details]
Proposed Patch
This depends on 36472. Before it is landed, the compiling queues will show errors for now.
Created attachment 51788 [details]
Proposed Patch
Removed the change to project.pbxproj accidentally added into the patch.
Attachment 51788 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/html/FormDataList.h:41: More than one command on the same line [whitespace/newline] [4]
Total errors found: 1 in 18 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 51789 [details]
Proposed Patch
Sorry, forgot to add it as patch. Try again.
Attachment 51789 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/html/FormDataList.h:41: More than one command on the same line [whitespace/newline] [4]
Total errors found: 1 in 18 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 51789 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1494001 Attachment 51789 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1495001 Attachment 51789 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/1487004 Attachment 51789 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/1570007 Created attachment 52204 [details]
Patch
Comment on attachment 52204 [details]
Patch
Sorry I submitted a patch to a wrong bug...
Attachment 52204 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1640071 Created attachment 52211 [details]
Proposed Patch
Updated the patch based on the latest sync so that commit-queue check should function now.
Attachment 52211 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/html/FormDataList.h:41: More than one command on the same line [whitespace/newline] [4]
Total errors found: 1 in 18 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 52211 [details] Proposed Patch Look good, couple of small things: > diff --git a/LayoutTests/http/tests/local/send-form-data-with-sliced-file.html b/LayoutTests/http/tests/local/send-form-data-with-sliced-file.html This looks like a typical script-test. Most (if not all) other script tests have their .js files in a subdirectory named 'script-tests' and they also have TEMPLATE.html file there. There is a script in WebKitTools/Script/make-script-test-erappers which then generates the .html files using the template. For the consistency, please consider following the same pattern. If necessary (because of specific template), a http/tests/local/formdata subdirectory can be created to keep FormData-related tests. > diff --git a/WebCore/platform/network/FormData.cpp b/WebCore/platform/network/FormData.cpp > @@ -186,9 +188,20 @@ void FormData::appendDOMFormData(const DOMFormData& domFormData, bool isMultiPar > +#if ENABLE(BLOB_SLICE) > + String fileName; > + if (value.blob()->isFile()) > + fileName = static_cast<File*>(value.blob())->fileName(); > + else { > + // If a blob is sliced from a file, it does not have the filename. In this case, let's produce a unique filename. > + fileName = "Blob" + createCanonicalUUIDString(); > + fileName.replace("-", ""); // For safe, remove '-' in case that certain server does not like it to be in the filename. May be "// For safety, remove '-' from the filename since some servers may not like it." ? > + } > +#else > + String fileName = static_cast<File*>(value.blob())->fileName(); It seems like a good place to have ASSERT(value.blob()->IsFile()); Doing r- since moving files to script-test might benefit from another look. Created attachment 53347 [details]
Proposed Patch
All fixed.
Attachment 53347 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WARNING: Skipping non-existent file: LayoutTests/http/tests/local/resources/send-form-data.js
WARNING: Skipping non-existent file: LayoutTests/http/tests/local/send-form-data-expected.txt
WebCore/html/FormDataList.h:41: More than one command on the same line [whitespace/newline] [4]
WARNING: Skipping non-existent file: LayoutTests/http/tests/local/send-form-data.html
Total errors found: 1 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 53347 [details] Proposed Patch r=me tiny nit: > diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog > + Move common functionalities to test sending FormData into a helper file s/functionalities/functionality/ Also, the style bot complained because the change happens around old code with style violation in it. Perhaps it would be nice to have a separate patch to fix the style of that code? Committed as http://trac.webkit.org/changeset/57695. http://trac.webkit.org/changeset/57695 might have broken Qt Linux Release |