Basically, the overall behavior of the form is pretty messed up :)

Clicking delete or cancel doesn't take you anywhere (so you never really leave the form but are caught in an endless cycle), and in some cases you can therefore wind up on the deletion form for a media item that you already deleted, and trying to do that and delete it again causes the module to explode. (The root of that last part is that media_load() does not return FALSE for media objects that don't exist, which seems to confuse the menu router system.)

Also, you can wind up on the media deletion form for an item that you aren't able to delete, which is strange - in some cases at least it is simple to detect that beforehand and prevent it, so I'm doing that in this patch also (the larger issue for that is at #877900: Media "Delete" confirmation form should inform user about undeletable items but this at least solves the simple part).

The attached patch is totally untested because it uses file_usage_list() which was just recently added to core and my only local dev setup for the media module right now doesn't have that yet, but hopefully will be able to actually test the patch soon. Basically it just makes is so that when deleting an individual media item, canceling takes you back to the item and a successful deletion takes you to the front page (like when deleting nodes), but when doing a multi-delete you get redirected back to admin/content/media which is where you came from.

CommentFileSizeAuthor
#2 media-delete-892352-3.patch6.55 KBJacobSingh
#1 media-delete-892352-1.patch5.03 KBDavid_Rothstein
media-delete.patch4.26 KBDavid_Rothstein

Comments

David_Rothstein’s picture

David_Rothstein commented
StatusFileSize
new media-delete-892352-1.patch5.03 KB

Here is an updated patch, with a few changes to make it actually work (as well as some slightly more clear variable names).

This one I have now actually tested :)

JacobSingh’s picture

JacobSingh commented
StatusFileSize
new media-delete-892352-3.patch6.55 KB

This code is so atrocious to begin with, it's tempting to fix it properly, but anyway perfect is the enemy of good.

Here's another roll of this patch which also gets rid of the add links.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs review » Reviewed & tested by the community

Yup, I thought about trying to get rid of that two-forms-in-one thing entirely, but then decided to keep the patch to something like the minimal number of changes :)

Those additions look good, and I tested that it works.

JacobSingh’s picture

JacobSingh commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

safetypin’s picture

safetypin
Location Memphis, Tennessee
commented
Version: 7.x-1.x-dev » 7.x-1.0-beta4
Status: Closed (fixed) » Active

I found this issue via #877900: Media "Delete" confirmation form should inform user about undeletable items, so my question is specifically about the message displayed to users when trying to delete media that's currently in use. Has this featured been unintentionally removed? I'm running beta4, and when I delete media that's in use, I only get a message saying it's in use and cannot be deleted. I don't get a list of references to where it's being used. Am I just misunderstanding this thread? It sounds like David Rothstein provided a patch for functionality along these lines back in August, but I don't know enough about the code to understand exactly what his patch is doing.

zigmoo’s picture

zigmoo commented

I could really use this functionality as well...

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented

+1, it would be very useful to know which entities are blocking the deletion.

In fact it's easy to add a single line to media.pages.inc (assuming Devel module is installed) around line 149:

    // The result of file_delete can be an array if the file is in use, or TRUE/FALSE.
    foreach ($results as $fid => $result) {
      if (is_array($result)) {
        dsm($result); // This line is inserted to provide some feedback about the entity blocking the file deletion.
        drupal_set_message(t('The file @title is in use and cannot be deleted.', array('@title' => $files[$fid]->filename)), 'warning');
      }

This will show an array containing entity type (eg. 'node') and entity id.

Unfortunately, turning this data into something that can be shown in the user interface, is a bit harder. It looks like requires some foreach loops, entity_load(), entity_uri() and url() calls before you end up with a decent link. I'm not sure about the best way to do that, especially because a large number of entity_load() calls will probably harm performance.

wmostrey’s picture

wmostrey commented
Priority: Normal » Major

It looks like #1240834: Cannot delete images added via an image field on an entity, what is previously deleted is related to this issue of not being able to remove media files once uploaded. We might want to move the information and discussion there and mark this one as a duplicate.

Leeteq’s picture

Leeteq commented

Subscribing.

amateescu’s picture

amateescu commented
Status: Active » Closed (duplicate)

Let's move further discussions in #1191438: file_usage not updated after removing a file/image field.

David_Rothstein’s picture

David_Rothstein commented
Priority: Major » Normal
Status: Closed (duplicate) » Closed (fixed)

The patch here was committed over a year ago, so I'm marking this as fixed. It seems like the last few comments are unrelated...

The ones before that (i.e. the comments about showing the list of entities that are still using the media item, so you can see what is preventing it from being deleted) could be moved to a separate issue (feature request), if there isn't one filed already.

David_Rothstein’s picture

David_Rothstein commented

Looks like the feature request already exists, actually, and there is a patch in progress: #1274524: More useful warning message, if media isn't deletable.