Closed Bug 524421 Opened 10 years ago Closed 10 years ago

input[type=file] has a file even if empty


(Core :: DOM: Core & HTML, defect)

Not set



Tracking Status
status1.9.2 --- final-fixed


(Reporter: darktrojan, Assigned: darktrojan)




(1 file, 2 obsolete files)

Attached patch proposed fix (obsolete) — Splinter Review
A file input control contains one file, even if none have been chosen. Presumably this was to clear the textbox of the control, but it's unnecessary. It also results in:
* (javascript) target.files.length = 1
* an empty file being sent over HTTP (although IE and Webkit also do this)
Attachment #408333 - Flags: review?(jonas)
Comment on attachment 408333 [details] [diff] [review]
proposed fix

>+  void ClearFileNames() {
>+    nsAutoTArray<nsString, 0> fileNames;
>+    SetFileNames(fileNames);
>+  }

Simply use |nsTArray<nsString> fileNames| instead.

And can you add DEBUG-only code to SetFileNames that asserts that none of the file names is an empty string.

Also, can you add a testcase as well? Like modify test_bug523771.html to test that input.files.length == 0 before any files are filled, and after .value is set to "".
Attached patch v2Splinter Review
Attachment #408333 - Attachment is obsolete: true
Attachment #408350 - Flags: review?(jonas)
Attachment #408333 - Flags: review?(jonas)
Attachment #408350 - Flags: superreview?(Olli.Pettay)
Attachment #408350 - Flags: superreview?(Olli.Pettay) → superreview+
Keywords: checkin-needed
Landed on trunk. Thanks for the patch!
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Attached patch Branch patch (obsolete) — Splinter Review
Branch-patch also containing fix for bug 524421
Assignee: geoff → jonas
Attachment #408724 - Flags: approval1.9.2?
Comment on attachment 408724 [details] [diff] [review]
Branch patch

Oops, wrong bug
Attachment #408724 - Attachment is obsolete: true
Attachment #408724 - Flags: approval1.9.2?
Is this needed/wanted on 1.9.2?
Assignee: jonas → geoff
Flags: wanted1.9.2?
Oh, crap, yes! I thought it already had landed there
Flags: wanted1.9.2? → wanted1.9.2+
Comment on attachment 408350 [details] [diff] [review]

Nevermind, this is fixed on 1.9.2. I think I included it in the initial landing of the multiple feature there.
Attachment #408350 - Flags: approval1.9.2?
So I'll just set the status flag to final-fixed. This will help verify that this was indeed fixed on 1.9.2.
Keywords: verifyme
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.