Bug 26505 - Value of file upload forms for non-multipart methods
Summary: Value of file upload forms for non-multipart methods
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Enhancement
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: HTML5Forms
  Show dependency treegraph
 
Reported: 2009-06-17 20:45 PDT by Kent Tamura
Modified: 2009-07-16 21:36 PDT (History)
5 users (show)

See Also:


Attachments
Proposed patch (3.54 KB, patch)
2009-07-06 17:58 PDT, Kent Tamura
eric: review-
Details | Formatted Diff | Diff
Proposed patch (rev.2) (4.43 KB, patch)
2009-07-07 01:59 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.3) (4.38 KB, patch)
2009-07-07 02:05 PDT, Kent Tamura
abarth: review-
Details | Formatted Diff | Diff
Proposed patch (rev.4) (6.16 KB, patch)
2009-07-16 21:25 PDT, Kent Tamura
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2009-06-17 20:45:16 PDT
According to the current HTML5 spec, UAs have to add filenames for non-multipart form submission.
http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#url-encoded-form-data
http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#plain-text-form-data

> If the entry's type is "file", replace its value with the file's filename only.

WebKit sets nothing for type=file at this moment. See HTMLInputElement::appendFormData().
Comment 1 Kent Tamura 2009-07-06 17:58:32 PDT
Created attachment 32336 [details]
Proposed patch
Comment 2 Eric Seidel (no email) 2009-07-07 00:41:42 PDT
Comment on attachment 32336 [details]
Proposed patch

Your ChangeLog has boilerplate code in it.  Please read the boilerplate and act on it.

This needs an automated test.  It's possible to test file drags and file uploads using DumpRenderTree (at least on the mac).  Look at creating an http/ test which uses eventSender.beginDragWithFiles() you could also create a new layoutTestController method which did fileObjectForPath() or similar if you wanted to make test which didn't require drag and drop but still used File objects.
Comment 3 Kent Tamura 2009-07-07 01:59:46 PDT
Created attachment 32368 [details]
Proposed patch (rev.2)

Eric,
Thank you for the advice.  I have updated the ChangeLog, made an automated test, and removed the manual test.
Comment 4 Kent Tamura 2009-07-07 02:05:05 PDT
Created attachment 32369 [details]
Proposed patch (rev.3)

Oops, removed unnecessary JavaScript code.
Comment 5 Eric Seidel (no email) 2009-07-15 16:17:09 PDT
Comment on attachment 32369 [details]
Proposed patch (rev.3)

This looks fine to me.  Ideally Sam would comment before you land.  But I think this is good to go as-is.
Comment 6 Adam Barth 2009-07-16 14:18:59 PDT
Will land.
Comment 7 Adam Barth 2009-07-16 15:02:57 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/forms/get-file-upload-expected.txt
	A	LayoutTests/fast/forms/get-file-upload.html
	M	WebCore/ChangeLog
	M	WebCore/html/HTMLInputElement.cpp
Committed r45987
	M	WebCore/ChangeLog
	M	WebCore/html/HTMLInputElement.cpp
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/forms/get-file-upload.html
	A	LayoutTests/fast/forms/get-file-upload-expected.txt
r45987 = fd7fd50f8f394c89fe0fd7b59661aff6d249ab9d (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/45987
Comment 8 Adam Barth 2009-07-16 17:17:50 PDT
Re-opening.  Tests did not pass on Windows.  Reverted in r45999.
Comment 9 Kent Tamura 2009-07-16 21:00:57 PDT
In Windows, the console had:
  CONSOLE MESSAGE: line 29: TypeError: Result of expression 'eventSender.beginDragWithFiles' [undefined] is not a function.
  FAIL: Timed out waiting for notifyDone to be called

Does Windows have no beginDragWithFiles?
Comment 10 Kent Tamura 2009-07-16 21:18:44 PDT
I have found other tests with beginDragWithFiles() are disabled for non-Mac.
Comment 11 Kent Tamura 2009-07-16 21:25:30 PDT
Created attachment 32914 [details]
Proposed patch (rev.4)

Disables the test for non-Mac.
Comment 12 Adam Barth 2009-07-16 21:30:54 PDT
Comment on attachment 32914 [details]
Proposed patch (rev.4)

Thanks for updating the patch.
Comment 13 Adam Barth 2009-07-16 21:31:26 PDT
Will land.
Comment 14 Adam Barth 2009-07-16 21:36:54 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/forms/get-file-upload-expected.txt
	A	LayoutTests/fast/forms/get-file-upload.html
	M	LayoutTests/platform/gtk/Skipped
	M	LayoutTests/platform/qt/Skipped
	M	LayoutTests/platform/win/Skipped
	M	WebCore/ChangeLog
	M	WebCore/html/HTMLInputElement.cpp
Committed r46006
	M	WebCore/ChangeLog
	M	WebCore/html/HTMLInputElement.cpp
	M	LayoutTests/platform/qt/Skipped
	M	LayoutTests/platform/gtk/Skipped
	M	LayoutTests/platform/win/Skipped
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/forms/get-file-upload.html
	A	LayoutTests/fast/forms/get-file-upload-expected.txt
r46006 = ba2185d36b7a2858015ae99f9dae0a5b13e62071 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46006