Images are still visible if trashed or if their parent post is unpublished

[aldenta]

If a post thumbnail is assigned, and then the image is trashed, the post thumbnail still appears since the _thumbnail_id meta key exists.

I see a couple of different ideas that might provide a solution. I'm happy to write the patch but I need some direction on the best option.

1. Update get_post_image_id() in post-image-template.php to check if the image has been trashed before returning the id. This adds a little bit of extra overhead but would keep the post thumbnail in place if they restore the image. One possible downside is that if the user later restores the image from the Media page then they may be surprised that the post thumbnail has also returned.

2. Delete the _thumbnail_id when the photo is trashed. This would mean that if the user clicks Undo then the photo thumbnail is no longer attached. It may be nice to delete the _thumbnail_id when a photo is deleted anyway to keep the table a little more tidy.

Any thoughts on the best approach to this?

Thanks.

John

[scribu]

The problem with option 2 is that it deletes the purpose of the trash functionality by adding a modal window. So I would go with 1.

[caesarsgrunt]

All trashed images still appear in all locations until they are permanently deleted (manually or 30 days after trashing).

It is not possible (sensibly) to automate the removal of images from all posts when they are trashed, and this does not currently happen for deleting images, either. Therefore, I see no reason to automate the removal of trashed images from post thumbnails either.

[caesarsgrunt]

I can't find post thumbnails in the UI anymore; are they still there?

[nacin]

Your theme needs to support it, and support was pulled from the default theme in [12180]

[caesarsgrunt]

Oh; pity. Why on earth was it removed?

I'll look for a theme which has it to test this...

[nacin]

Replying to caesarsgrunt:

Oh; pity. Why on earth was it removed?

Good question. For testing, I've simply patched the default theme back up with what was reverted, [12133]. Diff: ​http://core.trac.wordpress.org/changeset/12133?format=diff&new=12133.

You can enable it in the backend, if that's all you're looking for, w/ add_theme_support( 'post-thumbnails' ); in the theme functions.php.

[westi]

Maybe we should be using get_post_status() to check that the image is visible as well as check post_status on the individual attachment post related to the image.

[caesarsgrunt]

What makes this a blocker?

Trashed images are also still visible in all other places. Why is this one so much more important than the others?

[westi]

Replying to caesarsgrunt:

What makes this a blocker?

Trashed images are also still visible in all other places. Why is this one so much more important than the others?

It is not just Trashed images that would be visible but ones whos parent is private or draft could end up visible here - I don't think that would normally happen but we need to think about it.

[caesarsgrunt]

Still, the same applies to images within the post, and always has done.

Also, I don't think it can be fixed for images in the post, except by renaming the image file.

[westi]

Note: If you trash a media item it disappears from the gallery on a post

[westi]

For consistent behaviour I think we should be check 'post_status' == 'inherit' on the id stored in post_meta for the post image.

[caesarsgrunt]

Interesting. I'm not familiar with galleries, but presumably they are generated on the fly and check the image status, rather than having static code in the post.

This would be a good way to fix this bug and make images more easily editable overall - instead of inserting static html into the post when an image is added, just add a shortcode such as [image id=99 align=right]. The handler for this shortcode would then check the image status, and return an empty string if the status is wrong.

[westi]

Looks like that it is only gallerys that exhibit the correct behaviour.

Now wondering if we should disable Trash for Media.

[caesarsgrunt]

Depends what you mean by "correct".
Firstly, note that the problem here is basically that you can still see the image if you know the URL. But, this also applies to posts.
Secondly, note that the behaviour when you delete an image (pre 2.9) is not "correct" either. The code for the image is still in the post, it's just that the image file doesn't exist any more.

I strongly oppose the idea of disabling Trash for Media.

I propose taking the two actions described below :

1. Check whether post_status == 'inherit' before showing the post thumbnail. Do this before 2.9. This would leave us with Galleries and Thumbnails behaving in an optimal way, and images in posts behaving more or less as they currently do.

2. Change the system for adding images to posts, so that it uses a shortcode rather than inserting direct html. Do this in 3.0. This allows images to be somewhat dynamic, with the ability for plugins to alter their display etc - and enabling us to only show them if post_status == 'inherit'.

[nacin]

Replying to caesarsgrunt:

Firstly, note that the problem here is basically that you can still see the image if you know the URL. But, this also applies to posts.

No; trashed posts return a not found, same as if it was a draft, future, etc.

1. Check whether post_status == 'inherit' before showing the post thumbnail. Do this before 2.9. This would leave us with Galleries and Thumbnails behaving in an optimal way, and images in posts behaving more or less as they currently do.

This would work, except that images in posts would not behave as they currently do. When you delete an image, the HTML remains, but the file is gone.

On the other hand, when you trash an image, the HTML and the file both remain, so it will still display. Big problem.

This was extensively discussed in the dev meetup for today. ​https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2009-12-10&sort=asc. The decision the core devs decided on -- and, FWIW, most participants agreed to -- would be to disable Trash for Media, but keep the code in place and allow a plugin to turn it back on. Thus, Trash would function as if it was turned off, and fall back to the "Delete Permanently" links.

There were many alternatives discussed, but nothing presented itself as a viable solution, especially with 2.9 so close.

2. Change the system for adding images to posts, so that it uses a shortcode rather than inserting direct html. Do this in 3.0. This allows images to be somewhat dynamic

It seems there was traction for an [image] shortcode for 3.0; I imagine that will be the basis of Trash for Media.

[caesarsgrunt]

OK, well since I didn't participate in the dev chat I'll obviously just have to put up with the ultimate decision of the core devs. I would first like to add my vote for another solution, though, which was touched upon in the dev chat :

Move trashed images into a "trashed" folder, or add a ".trashed" suffix to their names. Then when untrashing them, this change could be reverted. This would "solve" the direct linking problem, since trashed images would then behave in the same way as deleted ones currently do.

That said, and having thought the matter over, I don't think Media is one of the most important places for Trash. I've never deleted an image and then wanted it back... The main purpose of Trash for media, in my eyes, is for consistency with Posts and Pages.

[nacin]

westi, do you want someone to patch this?

Replying to caesarsgrunt:

Move trashed images into a "trashed" folder, or add a ".trashed" suffix to their names. Then when untrashing them, this change could be reverted. This would "solve" the direct linking problem, since trashed images would then behave in the same way as deleted ones currently do.

The problem there was pointed out by markjaquith, in that "For file-moving schemes, it would have to be unpredictable." prettyboymp made a good point that we could make it unpredictable then store in in post meta, I don't think it caught on because it's too much to rush into 2.9 and there are probably better ways to tackle this in 3.0.

[azaozz]

(In [12377]) Disable trash for attachments for now pending improvements in 3.0, fixes #11149

[azaozz]

Can be enabled by adding define('MEDIA_TRASH', true); in wp-settings.php. If there are any attachments in the trash, it will still show on the Media Library screen until they are restored or deleted.

[azaozz]

Meant to say "define('MEDIA_TRASH', true); in wp-config.php"...

[nacin]

wp-admin.css could use a version bump.

[westi]

(In [12386]) Bump the version number on the wp-admin.css files for [12377]. See #11149 props nacin.