Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Images attached to content are not being cleaned up upon delete. One line patch here fixes it.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | file_reference_count_default.patch | 945 bytes | quicksketch |
| image_del.diff | 527 bytes | moshe weitzman | |
Comments
Comment #1
Dries CreditAttribution: Dries commentedCare to provide a bit more context? That would help the review process.
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedif you delete an article that has an uploaded image, the image is not deleted. the cause is a check in file_get_file_reference_count() that should be true but it is comparing image == file. the attached patch skips that check as it is not needed.
i asked quicksketch to review this.
Comment #4
quicksketchSo this has a bit of a long function chain, but here's the basic idea: Image module depends on File module and pretty much just reuses all File module's hook_field_* functions exactly. Unfortunately File module has a small bias that assumes that you're going to be working with fields of type "file" as opposed to other types (like "image"), causing images to not be deleted when the parent node is deleted.
As a sort of stack trace:
image_field_delete() ->
file_field_delete() ->
file_field_delete_file() ->
file_get_file_reference_count()
Since file_get_file_reference_count() defaults to restricting $field_type == 'file', it doesn't count references of other field types properly. As an alternative to changing the function signature though, we could simply pass NULL into file_get_file_reference_count() from file_field_delete_file().
Comment #5
quicksketchCrosspost. Maybe altering the call in file_field_delete_file() would be a safer change, since it doesn't modify the function signature.
Comment #6
quicksketchUpdates the one call to file_get_file_reference_count() that was working off the assumption of $field_type = 'file'.
Comment #7
moshe weitzman CreditAttribution: moshe weitzman commentedworks for me.
Comment #8
Dries CreditAttribution: Dries commentedThanks for the clarifications. That helps me focus on investigating the surrounding/affected code. Committed to CVS HEAD.