Closed Bug 506362 Opened 12 years ago Closed 11 years ago

Use JavaScript to test file uploads before the client sends the POST request


( Graveyard :: Public Pages, enhancement)

Not set


(Not tracked)



(Reporter: mozilla.bugs, Assigned: u278084)


(Whiteboard: [AMO508Testday])


(2 files, 1 obsolete file)

If a user tries to upload a *.tif file or other unsupported format, the file is posted and then rejected.

Would is save time and server load to reject the file with a javascript regular expression before the upload begins?
Whiteboard: [AMO508Testday]
OS: Windows XP → All
Summary: Use Javascript to test file uploads before the client sends the POST request → Use JavaScript to test file uploads before the client sends the POST request
Sounds like a good idea. Can we test the file size too? Probably.
Severity: normal → enhancement
Hardware: x86 → All
Assignee: nobody → cdolivei.bugzilla
Target Milestone: --- → 5.0.9
Attached patch v1 (obsolete) — Splinter Review
Checks for proper file name extension and checks file size (gecko only. If you know of a webkit/trident method, I'd love to hear it!).
Attachment #395245 - Flags: review?(clouserw)
Attachment #395245 - Flags: review?(clouserw) → review-
Comment on attachment 395245 [details] [diff] [review]

Looks pretty good, though:

>+// Returns the largest file you can upload to the server, in bytes.
>+function getMaxFileSize() {
>+    $size = ini_get('upload_max_filesize');
>+    switch (substr($size, -1)) {
>+        case 'M' :
>+            return (int)$size * 1024;
>+        default :
>+            return $size; // assume bytes
>+    }

wouldn't M be $size * 1024 * 1024?
Trident can apparently use ActiveX for this purpose, though I am not sure we want this:

In fact, I'd vote for "no". We decided once upon a time to ask for Gecko+JavaScript in the Dev CP, and try to degrade gracefully where reasonable. Not checking file sizes pre-upload for IE users seems graceful enough to me.
Thanks Wenzel. You are right, it is 1024 * 1024.

I didn't know about the ActiveX method for IE, but for some reason I was surprised. But I agree; dealing with it on the server side is graceful enough for IE, or any other browsers for that matter.
Attachment #395245 - Attachment is obsolete: true
Attachment #395500 - Flags: review?
Attachment #395500 - Flags: review? → review?(fwenzel)
Comment on attachment 395500 [details] [diff] [review]
v2. now more right

I tried it and it wfm, good job.
Attachment #395500 - Flags: review?(fwenzel) → review+
Thanks everyone.  r49610
Closed: 12 years ago
Resolution: --- → FIXED
Can we apply this to the profile user page where a user image is uploaded?  (Since that is what I meant to have this apply to and just didn't make it clear ;) ).  Thank you though for fixing it on the developer page, since I didn't catch it there.
Resolution: FIXED → ---
I'd like QA to check this for 5.0.9 so I'm re-resolving this bug.  Tanner, can you file another bug and put it in the 5.1 milestone for our next push?  (If you can't do the milestone I can after you file).  Thanks.
Closed: 12 years ago11 years ago
Resolution: --- → FIXED
Filed Bug 511932 and set milestone to 5.1.
Verified FIXED; I tried .tif, .doc, .ppt, .gif, .jpeg, .png.
Product: → Graveyard
You need to log in before you can comment on or make changes to this bug.