Make WordPress Core

Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#10238 closed defect (bug) (fixed)

A failed upload loses its error messages

Reported by: filosofo's profile filosofo Owned by: westi's profile westi
Milestone: 2.8.1 Priority: high
Severity: major Version: 2.8
Component: Media Keywords: media_upload_form_handler has-patch upload
Focuses: Cc:

Description

If there are errors in some uploads, WP fails to report it because the errors are erased. That happens because the $errors variable is initialized at the top of media_upload_form_handler() to an empty array (new behavior in 2.8), but lines like 490 of wp-admin/includes/media.php set $errors to the (sometimes empty) $return variable returned by media_upload_form_handler() when that $return variable is an array.

Prior to 2.8, the behavior was that media_upload_form_handler() would return NULL, so $errors would remain. Now, media_upload_form_handler() returns an empty array, and $errors is set to that empty array.

Patch initializes $errors in media_upload_form_handler() to NULL.

I think this is related to #10153 among others.

Attachments (1)

dont_lose_error_msgs.10238.diff (1.5 KB) - added by filosofo 16 years ago.

Download all attachments as: .zip

Change History (9)

#1 @filosofo
16 years ago

Updated patch to include a fix for a related problem in wp-admin/media-upload.php

The problem is that it redirects too soon before checking for upload errors. You can test this problem by not having a writable uploads directory and trying to upload from the "Add new" page under Media--nothing happens and you get no feedback.

#3 @ryan
16 years ago

Tested patch with browser and flash uploaders from both the edit post page and the new media page.

#4 @ryan
16 years ago

  • Owner set to westi
  • Status changed from new to assigned

#5 follow-up: @ryan
16 years ago

Tested the failure case where the server goes away. With and without the patch produced an I/O error notice. Did you simulate any error conditions?

#6 in reply to: ↑ 5 @filosofo
16 years ago

Replying to ryan:

Did you simulate any error conditions?

Yes, two:

  • Made the uploads directory unwritable
  • got rid of PHP's tmp directory

I was using only the browser uploader for my tests, however. I think that the browser uploader would fail to print messages more often than the Flash / Ajax uploader, because of the redirection issue.

#7 @ryan
16 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [11638]) Don't lose upload error messages. Props filosofo. fixes #10238 for trunk

#8 @ryan
16 years ago

(In [11639]) Don't lose upload error messages. Props filosofo. fixes #10238 for 2.8.1

Note: See TracTickets for help on using tickets.