Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11534 closed defect (bug) (duplicate)

WordPress moves post/page to trash instead of deleting it

Reported by: sirzooro Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9
Component: Trash Keywords: has-patch
Focuses: Cc:


I have created Trash Manager plugin. It adds "Delete Permanently" link to post, page and comment lists, so you can delete them permanently without need to trash them first (and you have trash enabled at the same time). These links looks the same as generated by WP on trash pages (I have copied them from WP code), but works incorrectly - WP reports that post/page is deleted permanently, but post/page is moved to trash instead.

Attachments (1)

trash-patch.diff (2.6 KB) - added by sirzooro 4 years ago.
Force delete for action=delete links

Download all attachments as: .zip

Change History (13)

comment:1 sirzooro4 years ago

Introduced in [12377]. I am looking how this can be patched.

comment:2 nacin4 years ago

The issue, I think, is that the action=delete calls wp_delete_post, which then -- if trash is enabled -- tries first to trash the post.

This has been a personal pet peeve of mine and I've tried to raise it in #11470.

comment:3 follow-up: nacin4 years ago

Here's my take on this. Your plugin is a perfectly reasonable example of how plugins can extend the trash feature.

I'm all for wp_delete_post, wp_delete_attachment, and wp_delete_comment relying on a $force_delete parameter to skip trash (if trash is enabled for that component).

That said, action=delete links should be processed always as a force delete, not as a conditional for whether trash is enabled (note how $force = !EMPTY_TRASH_DAYS). There's a reason we have action=trash as well.

Regardless, as you can see in that other ticke,t this is a much larger issue that will require some major decisions from the core devs on the directions of the trash-related API.

comment:4 caesarsgrunt4 years ago

At present you have to trash the post/page/comment first, before it can be deleted permanently. I basically agree that we shouldn't have introduced this restriction, though there are arguments both ways. At any rate, with the restriction there should be a $force parameter...

It was decided that it was too late to change it during the beta period when #11470 was opened. We probably can't fix this now until 3.0, since it's not a blocker-level bug.

comment:5 in reply to: ↑ 3 sirzooro4 years ago

  • Keywords has-patch added; needs-patch removed

Replying to nacin:

That said, action=delete links should be processed always as a force delete, not as a conditional for whether trash is enabled (note how $force = !EMPTY_TRASH_DAYS). There's a reason we have action=trash as well.

I agree with this, and my patch works in this way.

As I checked, for now I can hook actions trashed_post/trashed_comment, check if $_GET['action']=='delete' and delete again (this time post/page/comment will be in trash). This works, but it is an ugly hack - I do not like it.

BTW, I have to check how bulk actions work - I plan to add bulk delete option too.

sirzooro4 years ago

Force delete for action=delete links

comment:6 caesarsgrunt4 years ago

Not saying this isn't a problem, but...

Sizooro; for your particular plugin, why not just set WP_TRASH_DAYS = 0, and then add the javascript AYS back separately? Then you don't need to bother with this.

comment:7 follow-up: nacin4 years ago

Because he wants to keep trash as well, right?

In that case, action=trash calls wp_trash_post, which then calls wp_delete_post when trash is disabled. Too many dependencies.

Patch is a good start.

comment:8 in reply to: ↑ 7 caesarsgrunt4 years ago

Replying to nacin:

Because he wants to keep trash as well, right?

Oh, ok. The plugin description doesn't mention that; it implies that it just disables trash and reinstates AYS.

Either way, this is a duplicate of 11470; let's do all the discussion and patches in one place.

comment:9 caesarsgrunt4 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #11470.

comment:10 nacin4 years ago

Let's make sure this patch makes it to the other ticket.

comment:11 sirzooro4 years ago

Yes, I want to keep trash as well. You are right that plugin description does not say this clearly - I will update it.

I have added my patch to #11470 too.

comment:12 caesarsgrunt4 years ago

  • Milestone 2.9.1 deleted
Note: See TracTickets for help on using tickets.