Trashed items interfere with page/post slug generation

[Denis-de-Bernardy]

Create a static page called test. Trash it. Create a new static page called test. It'll want to use the slug "test-2" instead of the expected "test".

This is extremely confusing for non-technically oriented users.

[TobiasBg]

Seems like the right behavior to me.

What if the new post would now get "test" and the user untrashs the other page. Then that will get "test-2" and thus would have a different permalink as before (which will be even more confusing).

If the user wants to retain the "test" slug, he should simply not trash the old page, but overwrite/edit its contents.
By doing that, there will never be a "test-2" slug.
And if he later decides that he wants the old content back, it's still in a revision.

[Denis-de-Bernardy]

Replying to TobiasBg:

Seems like the right behavior to me.

I'd have thought so too, until I heard my father cursing about how WP was full of bugs.

What if the new post would now get "test" and the user untrashs the other page. Then that will get "test-2" and thus would have a different permalink as before (which will be even more confusing).

But if you trashed it, the odds are slim that you'll change your mind after recreating the very same page. (His particular issue was, WP created two pages in one go for a reason I've yet to identify.)

[nacin]

Replying to Denis-de-Bernardy:

I'd have thought so too, until I heard my father cursing about how WP was full of bugs.

I don't think we should free up the slug when an item is trashed, but the current behavior isn't good for UX, as you can attest to. Perhaps when we check a permalink slug, if there is a collision with a trashed item, we should in some way present that to the user and offer a solution?

[miqrogroove]

Replying to TobiasBg:

What if the new post would now get "test" and the user untrashs the other page. Then that will get "test-2" and thus would have a different permalink as before

Makes perfect sense to me.

Imagine if Windows refused to create a file called test.txt because there was already a file with that same name in the Recycle Bin. How pissed would you be?

+1 to Denis.

[miqrogroove]

Denis, let's explicitly add to this ticket that the trash should allow multiple trashed posts that have the same slug.

[TobiasBg]

Replying to miqrogroove:

Imagine if Windows refused to create a file called test.txt because there was already a file with that same name in the Recycle Bin. How pissed would you be?

Ok, that's a good example :-) (Although I don't use the Windows Recycle Bin :-) )

But I don't know, if we can compare filenames with permalinks as those generally have a larger impact on others (i.e. search results, dead links, ...)
If I rename a file on my hard drive, nobody will notice or be bothered...

And as we have a manual way to change slugs, I believe that should be enough. I don't know if too many people would ever be noticing this behavior.

[westi]

I think it would be even more confusing if you then went and un-trashed the first post and it ended up as test-2 then its permalink would have changed and that isn't meant to happen.

I wonder if the correct solution to this is to provide better feedback and make it more Poka-yoke so we would provide feedback as to why the best possible permalink wasn't being used (in all cases).

Something small and visual next to the displayed permalink whenever it triggers the duplicate check we a message about the thing that triggered it in a pop-out or somesuch.

[williamsba1]

I think just a message telling the user the permalink is attached to something in the trash is the best approach. Let the user decide what to do at that point. If they really want the permalink they will know to permanently delete the item from trash.

[glenn-om4]

The simplest concept for users to understand would be that once a post is trashed, it is trashed and doesn't affect what they do from there on. And there is a restore from trash option. This matches the commonly understood 'Trash' concept on the Windows and Mac desktop.

If this concept is applied to WP posts and trashed posts, there is no need to check the trash permalinks before creating the new permalink - once posts are trashed, they don't have any influence anymore.

In the much less common situation where a user wants to restore a post from the trash, then WP could check if there is a permalink conflict with active psots. If there is a conflict, only then as per Nacin's suggestion, "we should in some way present that to the user and offer a solution" e.g "The permalink page-2 used by this page is already in use, please correct before restoring". Then the the user could adjust the permalink to allow the restoration of the page.

This way the Trash concept in WP will match the Trash concept elsewhere, and users only need to find out about permalink conflicts if they want to restore a page that conflicts, rather than in the far more common situation where they might be working on a draft, trash it and start again.

[scribu]

completely agree with glenn-om4.

[williamsba1]

Added patch that ignores existing permalinks if they are at a trash status. When restoring a post from trash the permalink is updated if a duplicate exists.

[westi]

This doesn't improve things in a user-friendly way at all.

Now when I rescue a post from the trash I am going to get no warning that the slug change has occurred - this breaks the perma part of permalink.

I would much prefer to see an implementation which provides the feedback to the user when they un-trash a post to show that they is a clash and giving them a way of resolving the clash.

[williamsba1]

I should have noted I'm working on a message patch now to alert the user when this happens. Will post it when completed

[williamsba1]

Does it make sense to put some type of flag in the wp_unique_post_slug() function to determine if a permalink was changed because of a conflict? Or should this check exist in wp_untrash_post() function after wp_insert_post is called?

I'm just not sure the best place to put the check to trigger a message that there was a permalink conflict.

[scribu]

I think it makes more sense to put the check in wp_untrash_post().

[williamsba1]

I was thinking the same. I made some mods to that function you can see here:
​http://pastewp.org/5900

Problem is add_query_arg doesn't work in this case. Trying to figure out how to set the proper message ID in the query string.

[williamsba1]

Patch now includes a warning message to the user that there was a permalink conflict and the untrashed post's permalink was updated. Still not sure if this is the best way to trigger the error message, but wanted to give it a shot!

[williamsba1]

Updated the patch to display the updated post's permalink so there is no confusion on what was changed.

[dd32]

Translation functions can only accept a straight string, you'll need to use printf() w/ %s in the translated string.

[scribu]

Patch working as expected.

[scribu]

use sprintf() for translating string

[scribu]

In conflict.4.patch: use sprintf() for translating string.

[nacin]

Looks good.

[westi]

While we now have a patch which looks good I am nervous of changing this without get some UX feedback on the change.

I'm still in the camp which would prefer the restored post to have the permalink and the new post to get the warning as to why it gets the less pretty one.

If a post is sent to the trash and then restored it should win in the permalink war.

To that end I would rather not change this this close to the release of 3.0 without wider discussion.

[westi]

Better keyword name (props nacin)

[mikeschinkel]

Replying to miqrogroove:

Imagine if Windows refused to create a file called test.txt because there was already a file with that same name in the Recycle Bin. How pissed would you be?

Or worse, what if any computer wouldn't allow you to create a test.txt in one directory because there's already a test.txt in another directory?

Oh wait, that's the way WordPress currently works with slugs across path branches and across content types. But I digress... Related: #12935

Replying to glenn-om4:

The simplest concept for users to understand would be that once a post is trashed, it is trashed and doesn't affect what they do from there on. And there is a restore from trash option. This matches the commonly understood 'Trash' concept on the Windows and Mac desktop.

+1

[westi]

During this weeks dev chat the general consensus was that as permalinks are meant to be permanent trashed items should win in the slug battle until fully purged from the db.

We think that is would be good to improve the ux with a suitable message as to why about-1234 was being used and to make it easy to empty the post from the trash to get the prettier permalink.

[williamsba1]

Just added my first stab at the reworked patch. Notice is now displayed on the post edit screen when the temp permalink is generated with a link to the trash.

[wpmuguru]

(In [14446]) add warning on slug conflict with trash, props williamsba1, see #11863

[jamescollins]

I've just tried out [14446], and I think it still needs more work.

If you do the following:

1. Create a post with a permalink of test. Publish it.

2. Create another post with a title of test. Let the autosave trigger so that the permalink will try and default to "test".

Result: The permalink is set to "test-2" (as expected), but the display/layout of the warning doesn't look right (see screenshot.png).

4. Edit the permalink, and change it to a slug that doesn't exist. Click OK.

Result: The permalink change is accepted, but the conflict warning message is still displayed. I think it needs to disappear if the permalink is manually changed.

5. Click Save Draft.

Result: Post is saved, and conflicting warning message is removed (as expected).

6. Edit the permalink again, and change it to the original "test" slug.

Result: The permalink is changed to test-2 (as expected), but the conflict warning message isn't displayed. I think the message should be displayed here as well.

I used Firefox 3.6 on Mac.

[westi]

This string needs to have the variable bit put in by sprintf otherwise it is impossible to translate.

[westi]

After playing a while to get this to work better based on the feedback above I am going to back this patch out for now.

We need to cope correctly with different post_types and be congnicent of the rules regarding hierarchical / non hierarchical post types.

I guess we are going to need to leverage the code in wp_unique_post_slug more.

Maybe we need to extend it to let us know when it detects a collision.

[westi]

(In [14462]) Remove the trash warning on permalinks for now as the current implementation doesn't work very well. See #11863

[johnonolan]

UX: Agree that the original permalink should win, this absolutely makes the most sense. The expected result when you click something that says "restore" is that it is indeed restored to exactly how it was before.

[miqrogroove]

The expected result when you click something that says "restore" is that it is indeed restored to exactly how it was before.

... the exact opposite of what Windows does. This wont confuse anyone, guaranteed.

[johnonolan]

Replying to miqrogroove:

The expected result when you click something that says "restore" is that it is indeed restored to exactly how it was before.

... the exact opposite of what Windows does. This wont confuse anyone, guaranteed.

I'm having trouble figuring out whether your sarcasm is meant to be humour, or whether you're just being unhelpful? We can't have a discussion about the issue if you don't explain your views, I wasn't trying to step on anyone's toes :)

I read back over your earlier comments to try and figure out your standpoint, and I actually agree with you that restricting a new slug based on something that's in the trash (or recycle bin) is totally counter intuitive and a less-than-ideal user experience.
Now I'm no developer, so maybe from a technical point of view this isn't feasible - but I would say that once something is in the trash then the slug frees up, but if when restoring a trashed post a duplicate is detected, then the original post takes priority and the newer post gets changed to the "-2" with an appropriate warning dialog.

There is no definite right or wrong here, and it may well get changed based on real user feedback later on.

My apologies if I was unclear or offended you in any way miqrogroove

[miqrogroove]

I think it's the "newer post gets changed" bit that will confuse everyone. In Windows you have to make that change manually or it wont let you restore the item. The only option it provides is to delete the "newer" item. In WordPress lingo, that means trashing the "newer post" while restoring the old one. Then there is no conflict.

[johnonolan]

How will it confuse everyone?

Warning: If you restore this post then post-xx will be moved to post-xx-2. Do you want to continue? [Yes/No]

The most relevant piece of information that we have is that the user wants to restore a post, everything else should revolve around the expectation of what will happen when a button labelled "restore" is clicked.

For the record: Windows really isn't a great benchmark when it comes to user experience - and just because it's most common, doesn't mean it's best. Take Internet Explorer for example.

[mikeschinkel]

Replying to miqrogroove:

I think it's the "newer post gets changed" bit that will confuse everyone. In Windows you have to make that change manually or it wont let you restore the item. The only option it provides is to delete the "newer" item. In WordPress lingo, that means trashing the "newer post" while restoring the old one. Then there is no conflict.

+1. This is the most straightforward UX. What's in trash has been trashed. What's not trashed is live and working on the website. If trash is being restored it makes sense that trash should have to adjust, not what has been live and minding it's own business.

Replying to johnonolan:

The most relevant piece of information that we have is that the user wants to restore a post, everything else should revolve around the expectation of what will happen when a button labelled "restore" is clicked.

Agreed.

For the record: Windows really isn't a great benchmark when it comes to user experience - and just because it's most common, doesn't mean it's best. Take Internet Explorer for example.

I'll say that while I know Windows has had a tremendous amount of professional UX people do usability studies on Windows, probably more than on any other software ever, whether it's the "best" or not isn't what's important. Take "Jacob's Law"[1]; what's important is what the most people are used to and expect, even if it were not the "best."

If "best" were most important then your laptop wouldn't have a qwerty keyboard.

[1] ​http://socialcomputingjournal.com/viewlisting.cfm?id=84

[westi]

As I wrote above we discussed this at length in last weeks dev chat and the vonsensus based decision was that the permalink duplication has to be resolved by the user emptying the post from the trash.

While the post/page still exists in trashed form in the db it has the potential to be restored and must be restored with the same permalink it always had.

[miqrogroove]

permalink duplication has to be resolved by the user emptying the post from the trash.

/facepalm

[aesqe]

Replying to westi:

As I wrote above we discussed this at length in last weeks dev chat and the vonsensus based decision was that the permalink duplication has to be resolved by the user emptying the post from the trash.

While the post/page still exists in trashed form in the db it has the potential to be restored and must be restored with the same permalink it always had.

Sorry, but that's just wrong :) Trashed items should always have less significance than those that are published (or going to be published).

WordPress should warn the user if they decide to recover something from the trash that has a conflicting slug, not if they want to publish something new.

[Denis-de-Bernardy]

Agreed. Trashed items should get ignored altogether, if only for consistency with the filesystem trash.

Best I'm aware, if you create a test.txt file in a folder, trash it, and recreate a file with the exact same name, neither of Windows nor MacOS will bark. The WP trash should behave in the same way for posts and pages with the same slug.

[westi]

I'm sorry but we had a long discussion on this and the Permalink of an existing post has to win all the time it is in the trash and has the potential to be restored.

Yes this is different from your OS Trash folder.

Intentionally so because of the significance of the Permalink!

[scribu]

This ticket should have been closed right after the meeting (6 months ago).

[westi]

Replying to scribu:

This ticket should have been closed right after the meeting (6 months ago).

Well it would still be nice to have better UX here, even if we aren't going to give the new post permalink priority

[mikeschinkel]

Sorry to pile on, but both @aesque and @Denis-de-Bernardy are right and the previous decision is just wrong.

For example I have a client who I am building a product for which is a layer on top of WordPress (vs. just a website) and they are fanatically focused on user-experience and usability, even more so than Matt with his mom (if you can believe that.) This is one of the things that is causing them consternation with WordPress because the user is left with a frustrating situation that they can't get the slugs the way they want and there's no indication of why or how to change. And even if it did tell them how to fix it would be too many steps to follow for a less advanced user.

But, as you said, it have been decided and prior decisions are immutable.

[nacin]

I agree with westi.

We revisit decisions when we need to. We do not need to revisit this one and we will not. It was made as a user experience decision. A vocal minority on this ticket will not change that.

I'm also really sick of reading the same grandstanding and soapboxing coming from a few individuals on basically any ticket where their opinion is different.

[JohnONolan]

If "about" is in trash, then /about/ redirects to /about-2/ - which as I understand it is the canonical component kicking in.

If this is fixed and the canoncial component doesn't kick in, then the old permalink of /about/ will simply 404.

If the former is true then the permalink preservation is meaningless because the user is served a new (wrong/unexpected) page. If the latter is true then the permalink preservation is meaningless because the user ends up with no page at all, and all search engine rankings + external links die.

Lose/Lose?

I'm not saying the previous decision is wrong (not getting involved) but, based on the above, I don't think the current system is right.

[miqrogroove]

Replying to scribu:

This ticket should have been closed right after the meeting (6 months ago).

Agreeing with that. No change is needed if slugs are broken by design.

[johnbillion]

I'd like to ask that this issue be revisited. Granted, it's not a major issue, but it's a frustrating one for users when it happens.

As Westi pointed out, this was discussed at length by the core devs, however this discussion now took place two years ago. WordPress is in a much stronger position to test UX issues than it was two years ago and maybe in the intervening two years some core devs have noticed this issue and how counter-intuitive it is.

The reason I'd like to revisit the issue is this.

Scenario A: A user trashes a page with slug xyz, attempts to create a new page with slug xyz, ends up with xyz-2 and gets confused. "Damnit, I just deleted the page called xyz, why can't I create it again?"

The reason this happens is because we are reserving the trashed post's slug solely in case the post is subsequently restored from the trash. Due to the "perma"-ness of a permalink, it is considered that a restored post should not have its slug changed under any circumstance.

Reality check: How likely is the following scenario compared to Scenario A?

Scenario B: A user trashes a page with slug xyz, creates a new page with slug xyz, then subsequently un-trashes the original page with slug xyz (and expects it to still be available at its original permalink).

Yes of course it can happen, but in reality the frequency with which that happens is vastly smaller than Scenario A.

The current situation is addressing a situation where trashed posts should have preference over newly-created posts with the same slug just in case it's ever subsequently restored, rather than the far more common situation where a post is trashed and then someone attempts to create a new post with the same slug.

My opinion is that the current behaviour chooses to addresses an edge case rather than addressing a more common scenario, and this should be changed in order to improve UX. Trashed post slugs should not be taken into consideration when creating new posts. In the uncommon scenario where a user attempts to restore a post which shares the same slug as a subsequently created post, they should be notified. In addition, there could be a notice displayed below such posts when viewing the Trashed posts screen.

[DrewAPicture]

Replying to johnbillion:

In the uncommon scenario where a user attempts to restore a post which shares the same slug as a subsequently created post, they should be notified. In addition, there could be a notice displayed below such posts when viewing the Trashed posts screen.

You make some good and I think valid points. Why not just reverse the consideration order when it comes to restoring trashed posts, e.g:

• post(A) created, slug xyz, gets trashed
• post(B) created, slug of xyz (trashed items not considered)
• post(A) is later restored, but with a modified slug of xyz-2 (published items ARE considered)

[ericlewis]

Replying to DrewAPicture:

You make some good and I think valid points. Why not just reverse the consideration order when it comes to restoring trashed posts, e.g:

• post(A) created, slug xyz, gets trashed
• post(B) created, slug of xyz (trashed items not considered)
• post(A) is later restored, but with a modified slug of xyz-2 (published items ARE considered)

As per the previous discussion and this ​IRC dev chat (find: trash) which is referred to in this ticket, consensus was that trashed and restored posts should "win the permalink war."

[JohnONolan]

Here's a crazy idea: why don't we ask some USERS instead of referring to a bunch of ancient conversations between developers?

I know, I know - MADNESS.

[johnbillion]

Replying to ericlewis:

As per the previous discussion and this ​IRC dev chat (find: trash) which is referred to in this ticket, consensus was that trashed and restored posts should "win the permalink war."

We have already established this. Please see my comment above about why, two years later, I think this small but frustrating issue should be revisited.

[ericlewis]

Replying to johnbillion:

Replying to ericlewis:

As per the previous discussion and this ​IRC dev chat (find: trash) which is referred to in this ticket, consensus was that trashed and restored posts should "win the permalink war."

We have already established this. Please see my comment above about why, two years later, I think this small but frustrating issue should be revisited.

Yeah, I agree with your sentiments about the likelihood of cases you described. I think a step in the right direction for this issue would be to issue some error text as has been attempted before.

The last patch williamsba1 attached does not apply cleanly to trunk, I'll attach an updated version. It would need some designer approval, as it removes the CSS 'height' declaration for #edit-slug-box to allow for error messages to increase the height of the div.

[ericlewis]

Updated version of conflict5.patch to apply cleanly to trunk. Added link to current post if the conflicting post is published, leave link to trash if that's where it is.

[JohnONolan]

Provide a screenshot and I'd be happy to take a look

[ericlewis]

11863.patch screenshot

[ericlewis]

11863.patch screenshot 2

[ericlewis]

Note: 11863.patch will only show errors when user manually edits the permalink and a conflict is found. Displaying an error on publishing the post would be a bit more involved since the post_name is appended with '-n' on publish.

[SergeyBiryukov]

Replying to johnbillion:

Trashed post slugs should not be taken into consideration when creating new posts. In the uncommon scenario where a user attempts to restore a post which shares the same slug as a subsequently created post, they should be notified.

Seems that comment:10, comment:37 and comment:46 suggest the same. Makes the most sense to me.

[JohnONolan]

Styling looks fine. Copy needs work.

[ericlewis]

Add error messages when posts are restored from trash; give live post permalink precedence

[ericlewis]

In 11863-1.patch, an error message for each permalink collision is output when restoring posts. Copy is just filler, we could give more specific directions to the user for dealing with the collision.

[DrewAPicture]

Related: #21970 - 404 error when a post has the same slug as with a deleted (trash) page.

[DrewAPicture]

#24817 was marked as a duplicate.

[JoeManna]

I can confirm this behavior happened to me on WP 3.8.1.

An author created a Page with the same slug URL. Later, they realized they intended to create a Post instead. After they trashed the Page's draft, they proceeded to create a Post with the same title/information. The end result was that the post's slug conflicted with the draft that was sitting in the Page's trash.

Upon investigation, I deleted the trashed Page permanently and the issue was resolved immediately.

It's my suggestion that either any future slugs will check for conflicts in the trash and appending a number to the end. Or, to exempt the slugs for any items in the trash. If there was logic for precedence here, I would say the trash is lesser of a priority than published or drafted content, and its slugs should be modified with appended numbers.

-Joe

[williamsba1]

11863-2.patch screenshot

[williamsba1]

11863-2.patch screenshot 2

[williamsba1]

11863-2.patch displays error messages when there is a permalink collision. The View Trash link sends the user to the trash and filters the results with the post title, so should only show the post that is causing the collision.

Outstanding problem: I need to determine how to only show the error message when the post is initially saved. Right now it will only display if a new post is published using the message querystring value, which isn't ideal.

[williamsba1]

#28463 would fix the problem of determining when a post is initially created.

[ericlewis]

attachment:11863.diff​ is a refresh of attachment:11863-2.patch​.

[ericlewis]

I like the UX of this approach. The problem stems from users not understanding how trashed and untrashed posts work with each other. This message provides just enough clarity for the user to take action.

[ericlewis]

The message gets hoisted from the slug editor to the top of the page. Sorta awkward. See attachment:11863.mp4​

[ericlewis]

Working with @williamsba1 on refactoring this patch in attachment:11862.2.diff

Looks good, other than one conditional that needs some consideration:

if ( $orig_permalink != $post_name_abridged && isset( $_GET['message'] ) && $_GET['message'] == '6' )

This is how we're currently figuring out whether the postname is being suffixed with -2 on first post save. There must be a better way to do this.

[paulschreiber]

We just hit this on 4.2.4. +1 to a fix.

[krogsgard]

I'd like to preface by saying the current patch from @ericlewis and @williamsba is by far better than nothing, as old as this issue is. However, I think we could be more user friendly in the approach.

I would really like to see the newest post with a conflicting slug take precedent, if the conflicting counterpart is already in the trash. Then, if someone attempts to restore the trashed post, use the prompt then to resolve the conflict before the trashed post is restored. Basically, the approach would mean new posts that have matching slugs in trash would take the slug, and the trashed item would receive a slug change to add the -2.

This would be a huge win to get into core. So many epically sad -2 slugs out there and basically permanent. (Sidenote, will this work on terms too?) It also relates to being able to use "About" for default content, as was just brought up to me in #34116.

An argument against the more opinionated approach hasn't been made in 5 or 6 years. Yet -2 persists and is terrible. I have a hard time believing a bunch of people are restoring posts from the trash. A ton of people struggle with duplicate slugs though. Can we be more opinionated about making nice slugs on actual new content now?

[ryan]

Related: #15930 - Make trashed posts/pages still readable

#15930 and this ticket would be a nice pair of improvements to trash flow.

[ryan]

After years of being aggravated by this, I'm inclined to suffix untrashed posts. First to publish gets the unsuffixed slug, trashed posts get suffixed. Which is to say, I'm down with @krogsgard's suggestion.

[chriscct7]

I really like @krogsgard's idea

[MikeSchinkel]

+1 for @krogsgard's plan.

[williamsba1]

I really like the idea of first to publish wins. I reset and created the patch 11863.3.diff which allows the first published post to win the slug collision. If a trashed post is restored a suffix will be added just like before.

The next step is to add a message when a post is restored and the slug is changed so the user understands what happened and why.

[ericlewis]

Had a great, serendipitous chat with @coffee2code, @boonebgorges and @williamsba1 about this today at WordCamp US.

We've decided to change course and base the implementation on @coffee2code's plugin ​No Slug Conflicts with Trash.

A new post (e.g. "about") should take its desired slug. If a trashed post exists with the same slug, it should be modified in the same manner as wp_unique_post_slug() (e.g. renaming to "about-2"), while storing its previous slug "about". If the trashed post "about-2" is ever untrashed, it should try to assume its previous slug "about".

[helen]

Those mentioned above have probably already talked this out a bit, but just wanted to record here that in talking to @boonebgorges I noted that I don't actually think it's important to show a message about it, and that the "view post" link is enough. Chances of a situation in which somebody is untrashing AND there's a slug conflict AND it's critical to have the right URL exactly in that moment are lower than the amount of effort that would go into messaging and UX for further manipulating the slug (already not exactly the greatest UX).

Basically, I think it's reasonable to teach users that when something goes into the trash it "loses"/"forgets" its original slug.

[coffee2code]

It was summed it up concisely by @ericlewis, but anyone a bit more curious about the specific implementation employed in the ​No Slug Conflicts with Trash plugin (which incidentally was released Nov 2013, has 300+ active installations, and favorited 18 times), I ​added a bit more verbose of an overview to the plugin's documentation.

I also just updated the plugin and added documentation for the purpose and expectations of each of its unit tests, so anyone reading the code should have a clearer idea of what's going on.

[ericlewis]

Let's discuss implementation of the aforementioned including @helen's feedback.

1. When creating a post (in wp_insert_post()?), check for slug collisions with the desired post_name in the trash, and change the slug for the trashed post. This is more or less the control flow of ​No Slug Conflicts with Trash.
2. When making a slug unique (in wp_unique_post_slug()), ignore trashed posts. This is what @williamsba1 was moving towards in his last patch, attachment:11863.3.diff​. This means there would be slug collisions between published and trashed posts, until a trashed post is untrashed and assigned a unique slug.
3. Add a marker to a post's post_name when it is trashed, clearing the slug namespace. e.g. about => about%trashed%. If the post is untrashed, remove the marker.

[SergeyBiryukov]

I'm leaning to option 2 or 3, because they both seem to account for the following scenario:

1. Create a post and trash it.
2. Create a second post with the same slug and trash it too.
3. Restore the first post, it should have the original slug.

[coffee2code]

Replying to SergeyBiryukov:

I'm leaning to option 2 or 3, because they both seem to account for the following scenario:

1. Create a post and trash it.
2. Create a second post with the same slug and trash it too.
3. Restore the first post, it should have the original slug.

Just so it doesn't get mischaracterized, option 1 does the same. I have unit tests for the plugin that account for various trash and restore scenarios (including the one you've specified and more complex variations). I referenced an overview upthread that explains what it does.

(The plugin hooks wp_unique_post_slug() and not wp_insert_post() as summarized.)

[coffee2code]

Here's my take. Spoiler: I'm currently leaning toward option 3 (though I still contend option 1 was the best approach for the plugin to take).

Option 1:

As that's the approach I took for my plugin, I won't say much more than I've done so.

• Action on unique slug generation (as for new post):
  Check for conflict with trash. If so, give trashed post the new unique slug (but record its original) and allow use of desired slug by new post.
• Action when post is trashed:
  Nothing (post keeps its slug going into trash).
• Action when trashed post is restored:
  Check if original slug is in use. If not, restore original slug. If so, keep the unique slug it already has. No new unique slug regeneration occurs.

Option 2:

My main concern with this approach is the potential for having simultaneously existing conflicting slugs (a live post + one or more trashed posts). For this specific scope, it may not be an issue, but might there be an issue with other code out there that confidently worked knowing there would never be a slug conflict like that for posts?

• Action on unique slug generation (as for new post):
  Ignore posts in trash when checking for conflicts.
• Action when post is trashed:
  Nothing (post keeps its slug going into trash).
• Action when trashed post is restored:
  Check if original slug is in use. If not, slug can continue to be used. If so, generate a new unique slug via a call to wp_unique_post_slug().

Option 3:

I kinda like this one. It seems more minimal of a change with less unexpected side effects.

• Action on unique slug generation (as for new post):
  Nothing. (wp_unique_post_slug() functions unchanged).
• Action when post is trashed:
  Change slug to append %trashed%. We'd either have to permit trashed posts to be able to have the same slug (which seems reasonable), or have to find a unique variation of the slug at this time (e.g. about%trashed-2%).
• Action when trashed post is restored:
  Remove %trashed% from slug and check if that is in use. If not, the post can be restored with that slug. If so, generate a new unique slug via a call to wp_unique_post_slug().

[MikeSchinkel]

Replying to coffee2code:

Here's my take. Spoiler: I'm currently leaning toward option 3

To consider, $post->post_name is defined to be 200 characters. How many people have post titles that long? Is this an issue? If not, what should happen when a slug + %trashed% is too long?

[SergeyBiryukov]

Replying to MikeSchinkel:

To consider, $post->post_name is defined to be 200 characters. How many people have post titles that long? Is this an issue? If not, what should happen when a slug + %trashed% is too long?

Good catch. Slugs are stored in urlencoded form, so for non-ASCII slugs the limit is 33 characters, see #10483.

[SergeyBiryukov]

Replying to coffee2code:

Just so it doesn't get mischaracterized, option 1 does the same. I have unit tests for the plugin that account for various trash and restore scenarios (including the one you've specified and more complex variations). I referenced an overview upthread that explains what it does.

Thanks, I didn't notice it stores the original slug as well.

I'm leaning to option 1 then, because option 3 would reduce the 33 characters limit for non-ASCII slugs even further, as per my comment above.

[williamsba1]

I agree with @coffee2code and +1 Option 3. I don't think trashed posts should have the ability to hold a slug hostage, since it is trashed. If a post is un-trashed it will try to keep the original slug if there are no new conflicts. This seems like a nice low-impact change that makes complete sense to me.

[ericlewis]

Option 1 is the winner!

Option 3 is undesirable because of the hard limit on length of post_name, which is especially limiting for non-ASCII languages where slugs are URL encoded. Option 2 is undesirable because it means modifying an existing API behavior and opening up potential slug collisions — even our own get_page_by_path() doesn't check post_status != 'trash'.

The first patch implementation should:

On unique slug generation (as for new post), check for a slug conflict with trash. If so, give trashed post the new unique slug and allow use of desired slug by new post.

Let's see how that implementation feels, and avoid the untrashing flow for the moment. @helen already stated that flow is an edge case.

[robertwhitis]

One thing to consider as part of this is that items in the trash can still be active elsewhere, such as in menus.

This is something that has always been strange to me.

For instance, when you permanently delete a page, if it was part of a menu, upon permanent deletion, it's removed from the menu. However, simply putting it in the trash will not remove it from the menu.

I agree with the thought that once something is in the trash, the overall intent is for it not to be used - anywhere.

At that point, you could drop any custom slug and revert to it's post ID as the slug.

Upon restoration from trash, the slug based on the title could be reinstated, and if a conflict is found, add -2 to the end of the slug as is the usual approach.

[ericlewis]

In attachment:11863.4.diff​:

• Create a new function _add_suffix_to_post_name_for_trashed_posts() that will add a suffix to the trashed posts' slugs that match a given slug, while noting their "desired" slug in post meta.
• Invoke _add_suffix_to_post_name_for_trashed_posts() when a post is modified/created via wp_insert_post() as well as in get_sample_permalink() because that's where slug uniqueness is calculated after the user enters a title on Edit Post.
• When a post is untrashed, reassign its "desired" slug.
• Unit tests.

[ryan]

I lightly tested 11863.4.diff. Looking good.

• Create "test" draft with "test" slug
• Save draft
• Start another test draft
• Verify new draft shows "test" as its slug rather than test-2
• Publish the second draft
• Verify that the published post has "test" slug
• Edit first draft
• Verify slug is test-2
• Trash published "test" post
• Edit draft post
• Slug is now "test" instead of "test-2"
• Publish draft
• Slug remains "test"
• Restore trashed post
• The post restores directly back to published (because #15930)
• Verify the restored post is "test-2", not "test"

[boonebgorges]

Awesome. Glad to see this moving forward. The flow feels good to me.

A couple thoughts about the patch:

• I think there's a general move away from introducing more underscore-prefixed functions. I think it's fine to prefix them wp_ but maybe mention in the description that it's "used internally" or something like that.
• Related: I think we use @ignore rather than @access private to make the parser ignore functions. @DrewAPicture can verify.
• Running wp_update_post() causes update actions to be fired. '_modify_post_name_on_transition_post_status() runs during 'transition_post_status', which generally only happens during an update; so in this case, the actions will be fired twice. _modify_post_name_on_transition_post_status() runs during get_sample_permalink(), when you wouldn't generally expect your post content to be modified. In both cases, we are technically "updating" a post, but it doesn't seem semantically right somehow. Are there cases where a plugin author might be thrown off as a result of this? Is the alternative (direct database update + manual cache invalidation) liable to be just as confusing?

[ericlewis]

Replying to boonebgorges:

_modify_post_name_on_transition_post_status() runs during transition_post_status, which generally only happens during an update; so in this case, the actions will be fired twice.

Yeah, this recursion did feel awkward when I was writing it. A direct db update + manual cache invalidation like you suggest would make sense to keep it more isolated.

_modify_post_name_on_transition_post_status() runs during get_sample_permalink(), when you wouldn't generally expect your post content to be modified.

In this case, perhaps we could do this higher in the chain of the sample permalink AJAX handler, ​around here?

[ericlewis]

Thinking about this more, I wonder if we should consider a combined approach of what we were calling option 1 and option 3.

When a post is trashed, change post_name to {{slug}}-%trashed%, and also store the value in post meta. This lets us re-apply the original slug on untrashing from the post meta value (avoiding the character limit) and also avoid changing the slug at odd times as @boone mentioned.

[DrewAPicture]

Replying to ericlewis:

Thinking about this more, I wonder if we should consider a combined approach of what we were calling option 1 and option 3.

When a post is trashed, change post_name to {{slug}}-%trashed%, and also store the value in post meta. This lets us re-apply the original slug on untrashing from the post meta value (avoiding the character limit) and also avoid changing the slug at odd times as @boone mentioned.

Had you considered not using post meta at all and simply prefixing/suffixing the slug when trashed? Seems like introducing post meta into the mix might needlessly overcomplicate what we're trying to accommodate here.

I'd think we could rely on core's ability to find a unique post name to revert back to a valid post name on restore. The same would go for a similarly-post-named post slug coming in ... prefix/suffix it and check for a unique slug when trashing.

[ericlewis]

Replying to DrewAPicture:

Had you considered not using post meta at all and simply prefixing/suffixing the slug when trashed? Seems like introducing post meta into the mix might needlessly overcomplicate what we're trying to accommodate here.

I'd think we could rely on core's ability to find a unique post name to revert back to a valid post name on restore. The same would go for a similarly-post-named post slug coming in ... prefix/suffix it and check for a unique slug when trashing.

We have, great minds think alike :) We've turned away from suffixing the slug when trashed because that would severely limit non-ASCII characters in slugs.

[ericlewis]

In attachment:11863.5.diff​:

• Move the postname rejiggering logic into wp_insert_post(), which avoids duplicate calls to `wp_insert_post()`
• Move unit tests to their own file under the post/ component test folder.
• Don't rename postnames within get_sample_permalink(). I originally dropped the logic in here so that, when the AJAX request when creating a new post attempts to decide on a permalink, any post in the trash with the desired slug would be changed. The logic for deciding the slug is embedded in get_sample_permalink(), so extracting that logic would be duplicative higher in the stack in wp_ajax_sample_permalink().
• Relatedly, when a post goes trashed, also add the trashed slug suffix. This should keep the slug namespace clear.

[ericlewis]

In attachment:11863.7.diff​, don't let a-cool-post-%trashed%-%trashed%-%trashed% happen.

[ocean90]

11863.8.diff​: Added test_trashed_post_slugs_that_were_moved_should_not_be_reassigned_after_untrashing_if_post_with_the_same_slug_exists() and moved the delete_post_meta() call into the condition.

[ericlewis]

In attachment:11863.11.diff​,

• limit the scope of what the tests cover
• actually add the tests

[ericlewis]

In attachment:11863.12.diff​, mind coding standards.

[ericlewis]

In 36607:

Posts: Non-trashed posts should take slug priority over trashed posts.

When determining a unique post slug, trashed posts are taken into account. Previously, new posts would add suffixes to their slugs (e.g. about-2) when a post in the trash had the desired slug (e.g. about).

To avoid this behavior, when a post is trashed its slug (i.e. post_name) is now suffixed with -%trashed%. The post's pre-trash slug is stored as post meta, and if the post is restored from trash, its desired slug is reapplied.

For existing trashed posts which don't have the -%trashed% suffix, the suffix is added when a post with its desired slug is created.

Props ocean90, boonebgorges, ryan, SergeyBiryukov, coffee2code, helen, williamsba1.
See #11863.

[jorbin]

CLosing as fixed. If any issues are discovered during 4.5 beta testing, we can reopen.

[DrewAPicture]

In 36726:

Docs: Standardize summaries for two new internal functions used to handle suffixing trashed posts.

Also adds a notation of private access to each.

See #11863. See #32246.

[DrewAPicture]

In 36727:

Docs: Use the correct variable name for the $post_ID parameter in the DocBlock for wp_add_trashed_suffix_to_post_name_for_trashed_posts().

See #11863. See #32246.

[netweb]

Reopening as this currently breaks bbPress, with bbPress trashed post types (topics and replies) can still be viewed, edited, manipulated etc on the front end and are not limited to the "trash" section of the post types admin UI back end in /wp-admin.

To reproduce:

1. Install bbPress
2. Create a forum with title 11863
3. Create a topic in the 11863 titled deleted-topic and content also deleted-topic
4. Verify the topic exists, e.g http://src.wordpress-develop.dev/forums/topic/deleted-topic/
5. Now "trash" that topic my clicking on the "trash" link in the topic admin links
6. The topic is only "trashed", the post has not been "deleted" as such it should still be viewable to those with appropriate roles/caps at http://src.wordpress-develop.dev/forums/topic/deleted-topic-%trashed%

The result above returns an error 400 Bad Request because % percentage symbols in URL's are ​reserved characters.

Workaround is to replace the % characters in -%trashed% from the change in r36607 with characters that are valid for URL usage:

src/wp-includes/post.php

@@ -6126 +6126 @@
 
         $post = get_post( $post );
 
-        if ( strpos( $post->post_name, '-%trashed%' ) ) {
+        if ( strpos( $post->post_name, '-yolo-trashed-yolo' ) ) {
                 return $post->post_name;
         }
         add_post_meta( $post->ID, '_wp_desired_post_slug', $post->post_name );
-        $post_name = _truncate_post_slug( $post->post_name, 190 ) . '-%trashed%';
+        $post_name = _truncate_post_slug( $post->post_name, 190 ) . '-yolo-trashed-yolo';
         $wpdb->update( $wpdb->posts, array( 'post_name' => $post_name ), array( 'ID' => $post->ID ) );
         clean_post_cache( $post->ID );
         return $post_name;

The result of applying that patch is after restoring the trashed topic, then applying the patch and trashing the topic once again is that the topic is now viewable once again on the front end via the URL http://src.wordpress-develop.dev/forums/topic/deleted-topic-yolo-trashed-yolo/?view=all

Any suggestions apart from yolo to replace the % characters?

[ericlewis]

Repurposing some familiar namespace-syntax we use in PHP and JavaScript, we could use -_trashed?

[netweb]

Replying to ericlewis:

Repurposing some familiar namespace-syntax we use in PHP and JavaScript, we could use -_trashed?

As long as there's reasonable consensus that -_trashed wouldn't cause conflicts then yes, that works as expected

http://src.wordpress-develop.dev/forums/topic/hypen-underscore-trashed-_trashed/?view=all

[netweb]

Added a patch, tested and looks good for bbPress

[ocean90]

This was introduced 6 weeks ago and so was shipped in 4.5 beta and RC. Do we have to provide an upgrade routine for those installs?

@netweb Your patch doesn't update the unit tests which were introduced in [36607].

[netweb]

Replying to ocean90:

This was introduced 6 weeks ago and so was shipped in 4.5 beta and RC. Do we have to provide an upgrade routine for those installs?

Possibly, currently 4.5 beta/rc will not be able to restore any trashed items with the original pre-trash slug

Replying to ocean90:

@netweb Your patch doesn't update the unit tests which were introduced in [36607].

Updated patch 11863.14.diff​ updates the tests.

[ericlewis]

In attachment:11863.15.diff​, we now only need to truncate 191 characters in the _truncate_post_slug() invocation.

[azaozz]

Looking back at this:

Option 1:
As that's the approach I took for my plugin, I won't say much more than I've done so.

• Action on unique slug generation (as for new post): Check for conflict with trash. If so, give trashed post the new unique slug (but record its original) and allow use of desired slug by new post.
• Action when post is trashed: Nothing (post keeps its slug going into trash).
• Action when trashed post is restored: Check if original slug is in use. If not, restore original slug. If so, keep the unique slug it already has. No new unique slug regeneration occurs.

And then:

Option 1 is the winner!

Not sure if this is the best UX. Example:

• Write a post about... bananas called "What the...".
• Publish, popularize it, etc.
• In a month, trash that post and write a post about politics with the same or very similar title.

As the first post is in the trash, the second will take over the slug, and all links pointing to the bananas post will now point to the political post. This can pan out really badly depending on context. Of course the same can happen when the second post is published few months after the first has been deleted.

IMHO the user should be alerted when reusing old slugs, always. In case the old post is still in the trash, that alert can contain a "Delete permanently" button.

[ocean90]

@ericlewis 11863.15.diff​ looks good, but let's go with post-slug__trashed as decided in the dev chat.

[ericlewis]

attachment:11863.16.diff​ is updated in light of feedback.

[boonebgorges]

11863.16.diff​ looks good to me.

[kirasong]

Great, if this is ready, @boonebgorges or @ocean90 can one of the two of you commit this, please?

[boonebgorges]

Really sorry, but I'm going to withdraw my +1 for the time being - the strpos check looks too loose to me. See 11863.17.diff​ and the new unit test.

[kirasong]

After chatting with @boonebgorges, re-adding commit keyword for second review.

[ocean90]

11863.17.diff​ looks good to me, @boonebgorges please commit.

[boonebgorges]

In 37165:

Use __trashed suffix rather than -%trashed% for trashed post slugs.

Percent signs are reserved characters in URIs. As such, it was impossible for
plugins to route requests to trashed posts, as happens in bbPress. The new
__trashed suffix should be sufficiently unique.

Also adds a test that demonstrates that the __trashed suffix can be
appended to slugs that contain the suffix somewhere other than the end of
the string.

Props netweb, ericlewis.
Fixes #11863.

[pavelevap]

When trashed post is restored, postmeta _wp_old_slug with value postname__trashed is created. Should I reopen or create new ticket?

[ericlewis]

Replying to pavelevap:

When trashed post is restored, postmeta _wp_old_slug with value postname__trashed is created. Should I reopen or create new ticket?

Thank you for the bug report. I think we can deal with it on this ticket. Patch fix incoming.

[ericlewis]

In attachment:11863.18.diff​, we should not store the old slug for untrashed posts (i.e. "abouttrashed").

Old slugs are used for redirecting in case a user changes the slug. Thinking about this a little further...I wonder if this is useful in some cases, like BBPress, where trashed posts are visible on the front-end? In case a post is untrashed?

[netweb]

Will take a look and test 11863.18.diff​ in a couple of hours.

[ericlewis]

Thinking about this a little further...I wonder if this is useful in some cases, like BBPress, where trashed posts are visible on the front-end

I stand by this logic and think this is desirable behavior. Closing this, please open new tickets for any bugs.

[netweb]

Replying to ericlewis:

Thinking about this a little further...I wonder if this is useful in some cases, like BBPress, where trashed posts are visible on the front-end

I stand by this logic and think this is desirable behavior. Closing this, please open new tickets for any bugs.

Yes, all good here from a bbPress perspective :+1

And, yes, it is actually useful from a forum moderation perspective, if a topic or reply was trashed and you're using the trashed slug, e.g. .../forums/topic/my-topic__trashed/?view=all and another moderator or admin restores the topic, thanks to _wp_old_slug you get an automatic redirect to .../forums/topic/my-topic/?view=all

[ericlewis]

Replying to netweb:

you get an automatic redirect to .../forums/topic/my-topic__/?view=all

Did you mean a redirect to .../forums/topic/my-topic/?view=all? The underscores should be removed when untrashed.

[pavelevap]

Yes, when you publish post "Trash" (slug trash) and move it to Trash, post_name will be changed to trash__trashed.

And when you restore it, post_name is changed back to trash and _wp_old_slug with value trash__trashed is created.

The only feature of _wp_old_slug in this case is possible redirection from trash__trashed to trash? But I do not understand why? No user knows (for default Posts and Pages) this temporary trashed slug and there is no need to save this redirect. And if it is possible problem on bbPress side, it should be solved there because it is not WP core issue? For most of WP users it has only downsizes, I guess, that means bloated database with not usable values :-(

[netweb]

Replying to ericlewis:

Replying to netweb:

you get an automatic redirect to .../forums/topic/my-topic__/?view=all

Did you mean a redirect to .../forums/topic/my-topic/?view=all? The underscores should be removed when untrashed.

Yes, my bad, edited and fixed copy pasta.

[netweb]

Replying to pavelevap:

Yes, when you publish post "Trash" (slug trash) and move it to Trash, post_name will be changed to trash__trashed.

Correct

Replying to pavelevap:

And when you restore it, post_name is changed back to trash and _wp_old_slug with value trash__trashed is created.

Correct

Replying to pavelevap:

The only feature of _wp_old_slug in this case is possible redirection from trash__trashed to trash? But I do not understand why?

Correct, the 11863.18.diff​ would have bypassed saving _wp_old_slug

Replying to pavelevap:

No user knows (for default Posts and Pages) this temporary trashed slug and there is no need to save this redirect.

For default post types yes, for custom post types like bbPress' use case knowing and using the trashed slug is required.

Replying to pavelevap:

And if it is possible problem on bbPress side, it should be solved there because it is not WP core issue?

There is no longer any issue on bbPress side, it also couldn't be handled by bbPress alone, this is WP core functionality that changed that bbPress itself could not change.

Replying to pavelevap:

For most of WP users it has only downsizes, I guess, that means bloated database with not usable values :-(

Yes, but again this would be pretty limited numbers for most users

[jadpm]

I know this tikets is closed, fixed and shipped with WodPress 4.5 already. Just wanted to voice here, too late, that changing the slug of a post when it gets trashed might not have been the best idea when thinking on broader usages.

Consider a private post type used for non-standard purposes, like shop orders or content blocks. Those items might be referenced somewhere by ID, or by slug, storing the reference, say, on a postmeta, in the idea that slugs are, like IDs, unique (as the existence of functions like wp_unique_post_slug suggest...). When you have to query for references containing the item slug, if the item has been trashed, all references get broken as they do not include the new __trashed suffix.

Also, I do not get how we jumped from comment 100: https://core.trac.wordpress.org/ticket/11863#comment:100

Option 1 is the winner!

which states

Action when post is trashed: Nothing (post keeps its slug going into trash).

to comment 102: https://core.trac.wordpress.org/ticket/11863#comment:102 which directly starts implementing the changes to sligs for trahed items. I assume discussion happened somewhere else, but it woudl be nice to knwo why and how the decision was made?

I am not discussing whether there was a usability issue to fix, which is obvious. But thinking about usages out of the box might have prevented breaking such references, that do happen in the wild, believe me :-)