WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 6 months ago

#18962 closed defect (bug) (fixed)

Allow duplicate slugs for different content

Reported by: maorb Owned by: wonderboymusic
Milestone: 4.1 Priority: normal
Severity: major Version: 2.9
Component: Permalinks Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Currently, the slug of a post (or any other CPT) must be unique.
If a content is being created and an already existing slug is being assigned to it - WP will add a number for identification (i.e. about-2 etc).

That means, that if one has content from different content types, or in different categories (taxonomies), it still cannot have same slug.

The issue might be very disturbing when working in a multi lingual site (i.e using WPML or any other plugin for that).
Suppose there is an about page - www.mysite.com/about.
This is in the main language of the site.
Than a translation to that content page is being added, let's say it is translation to English.
You would expect url something like www.mysite.com/en/about, but the slug is being changed to about-2, so the url of the about page in English becomes www.mysite.com/en/about-2.
For a site with 10 languages, let's say, it will end with urls like site.com/lang/about-10 etc.. and this is not looking good and may confuse.

As a cms, WP should let the admin/developer/operator - to have same slugs for different content types, or even in the same content type. The $post->ID is the unique identifier of a content, why should be also the slug?

Attachments (5)

wp-includes_post.diff (1.5 KB) - added by mboynes 2 years ago.
Modifying wp_unique_post_slug to allow posts in hierarchical post types to share slugs with posts in other hierarchical post types
18962.diff (2.8 KB) - added by nacin 9 months ago.
Patch by mboynes, I just tweaked it a little and added tests.
18962.2.diff (5.7 KB) - added by dd32 6 months ago.
Includes fix from julien731 in #30339
18962-test.patch (1.4 KB) - added by boonebgorges 6 months ago.
See #30284.
18962.patch (3.4 KB) - added by boonebgorges 6 months ago.

Download all attachments as: .zip

Change History (53)

comment:1 @johnbillion4 years ago

Hmm, I thought you could have the same slug across multiple most types, but I've just tested it and evidently you can't. Maybe this is a regression?

The only case where you can have the same slug for multiple posts is on different levels within hierarchical post types. Eg. example.com/hello and example.com/sub-page/hello. If this is allowed then it's odd that the same slug isn't allowed across post types.

I've built a plugin for WPML which allows duplicate slugs for the same content across languages. If you're interested email me at my username at gmail and I'll send it over.

And just to address your core point, slugs have to remain unique within their post type for obvious reasons, so they are uniquely identifiable at their URL.

Version 0, edited 4 years ago by johnbillion (next)

comment:2 follow-up: @azaozz4 years ago

How about when permalinks are set only to %postname%?

comment:3 in reply to: ↑ 2 @scribu4 years ago

  • Cc scribu added

comment:4 @hearvox3 years ago

You can use the same slug in multiple hierarchical post types, but only for a page that is NOT top-level, AND NOT the same level w/in the same post type.

The function wp_unique_post_slug() disallows identical slugs for the same post_parent. and since all top-level pages of heirarchical pages have a [post_parent] => 0, they can't have the same slug.

Not sure if that's intentional, but it is the result. Been testing it lately, here's the URLs possible:

/test/

/test/test/

/test/test-2/

/test/test/test/

/cpt-name/test-2/

/cpt-name/test-2/test/

All the above "test" slugs have a different [post_parent], so the identical slug is allowed. All the above "test-2" slugs had the same parent as another "test" slug, so the "-2" was added.

In the case of "/cpt-name/test-2/" the post_parent was 0, same as "/test/", so its slug got a "-2."

comment:5 @hearvox3 years ago

  • Cc mbox@… added

comment:6 @nacin3 years ago

  • Keywords needs-unit-tests added

I would love to fix this. To do so, we will need some unit tests.

comment:7 @DH-Shredder3 years ago

  • Cc mike.schroder@… added

comment:8 @thirstcard3 years ago

  • Cc thirstcard added

comment:9 @thirstcard3 years ago

Many of the top Content Management Systems allow the same slug to be used across many different parents. An example of a website that has good information architecture is http://www.riverisland.com.

Considering WordPress has moved away from being a simple blogging tool and into the realm of a powerful CMS, shouldn't we remove the need for sibling slugs to be unique?

comment:10 @scribu3 years ago

  • Milestone changed from Awaiting Review to Future Release

Yes, we should, but it's easier said than done. WP would have to look at the permalink structure to check if the final URL would be ambiguous or not.

comment:11 follow-up: @caekcraft2 years ago

More tests:
I have IA as such at the moment:

/test1/subtest1/  (works perfectly)
/test2/subtest1/  (works perfectly)
/test2/subtest2/  (works perfectly)

When I try to navigate to
/test1/subtest2/, which does not exist, instead of getting a 404 (which I am expecting), I am instead redirected to
/test2/subtest2/, which, from an IA point of view, detrimental. Also, testing needed if normally I would not have access to /test2/subtest2/, after the redirect, will I get a restricted page, or bypass security and view the contents?

comment:12 in reply to: ↑ 11 @SergeyBiryukov2 years ago

Replying to caekcraft:

When I try to navigate to /test1/subtest2/, which does not exist, instead of getting a 404 (which I am expecting), I am instead redirected to /test2/subtest2/

Related: #18734

comment:13 follow-up: @mboynes2 years ago

  • Cc mboynes@… added

Can someone please explain why wp_unique_post_slug works the way it does with regards to hierarchical custom post types versus non-hierarchical custom post types?

That is, if two custom post types have 'hierarchical' set to false, posts can share the same slug. However, if two custom post types have it set to true, they cannot. This can be seen in http://core.trac.wordpress.org/browser/trunk/wp-includes/post.php#L3036 where the hierarchical post types check:

SELECT post_name FROM $wpdb->posts WHERE post_name = %s AND post_type IN ( '" . implode( "', '", esc_sql( $hierarchical_post_types ) ) . "' ) ...

(where $hierarchical_post_types is an array of all hierarchical post types). By comparison, non-hierarchical post types check:

SELECT post_name FROM $wpdb->posts WHERE post_name = %s AND post_type = %s ...

(where post_type's %s is merely the current post's post type).

I'm sure that I'm missing something, but when I modify this to restrict the hierarchical post types query to just the current post type, everything appears to work just fine. Patch forthcoming.

@mboynes2 years ago

Modifying wp_unique_post_slug to allow posts in hierarchical post types to share slugs with posts in other hierarchical post types

comment:14 in reply to: ↑ 13 ; follow-up: @SergeyBiryukov2 years ago

Replying to mboynes:

Can someone please explain why wp_unique_post_slug works the way it does with regards to hierarchical custom post types versus non-hierarchical custom post types?

Introduced in [11125] as a fix for page/attachment slug conflicts in #6437.

[21845] (#15665) was a more recent fix. Perhaps [11125] can be reverted now.

comment:15 @SergeyBiryukov18 months ago

#26512 was marked as a duplicate.

comment:16 @goto1018 months ago

  • Cc dromsey@… added

comment:17 @SergeyBiryukov17 months ago

#26712 was marked as a duplicate.

comment:18 @jfarthing8417 months ago

  • Cc jeff@… added

Is the patch that I posted in #26712 not sufficient to create parity between hierarchical and non-hierarchical post types? Two years is a long time for this to have been in limbo.

comment:19 @SergeyBiryukov17 months ago

  • Milestone changed from Future Release to 3.9

comment:20 @3flex17 months ago

  • Cc 3flex added

comment:21 @helen15 months ago

  • Keywords has-patch needs-testing added
  • Milestone changed from 3.9 to Awaiting Review

Appears to still need unit tests.

comment:22 @nacin15 months ago

  • Milestone changed from Awaiting Review to Future Release

We definitely want to fix this, so moving to Future.

comment:23 @johnbillion14 months ago

#27458 was marked as a duplicate.

comment:25 in reply to: ↑ 24 @xusht14 months ago

Replying to johnbillion:

Looks like I never posted it on this ticket; here's a plugin which allows duplicate slugs across post types.

plugin doesnt work for me. wont leyt me do>>

CPT1/slug1
CPT2/slug1

not sure if plugin is meant to do that ?

oops it worked on new category slugs not on changing existing - great thanks John :)

Last edited 14 months ago by xusht (previous) (diff)

comment:26 in reply to: ↑ 24 @trymedo11 months ago

Replying to johnbillion:

Looks like I never posted it on this ticket; here's a plugin which allows duplicate slugs across post types.

The plugin only seems to allow you to specify duplicate slugs, but if you have the following permalink structure you only seem to be able to view one post (I presume it's the post with the highest/lowest ID)

/%category%/%postname%/

A site I've recently migrated has multiple posts that have the same slug, but they are all within separate categories so I originally assumed it would be ok.

Is there a way to get this to work? Maybe a hook into the 'template_redirect' action or something?

comment:27 @jpswade10 months ago

The problem:

  1. Add new media called "banana.jpg", the slug is "banana".
  2. Add new post with the tag "banana".
  3. Add "banana" to the "fruit" Custom Post Type.

The result:

The media has a slug called "banana".
The tag has the slug "banana-2".
The fruit Custom Post Type has a slug of "banana-3".

I've read other feedback which says that "Link categories", posts and pages can all conflict too...

What we need to know is: Does the patch resolve this?

comment:28 @jpswade10 months ago

  • Keywords needs-unit-tests removed
  • Severity changed from normal to major
  • Type changed from enhancement to defect (bug)
  • Version set to trunk

Upgrading this to a bug.

As per post.php:

Page slugs must be unique within their own trees. Pages are in a separate
namespace than posts so page slugs are allowed to overlap post slugs.

It's fair to say that media, tags, custom post types, link categories, posts and pages are different "trees".

I've upgraded this to 'major' severity due to the effect it can have on URL structure, which could potentially have a detrimental effect to a site if a slug points to the wrong place.

I've explored the 'tests' looking for the word 'slug', yet this yields no results, therefore the lack of unit test is not one created by this issue and therefore should not be addressed by this issue.

comment:29 @jpswade10 months ago

I tested the 'Allow Duplicate Slugs' plugin by 'John Blackbourn', however, this did not seem to allow duplicate slugs. John suggests it has stopped working since v3.9, but he's yet to explore the problem.

I also tested the 'wp-includes_post.diff' patch to v3.9.1, this did not allow duplicate slugs either. Using the 'quick edit' of my custom taxonomy, it gave the error: The slug “banana” is already in use by another term.

I also tested the patch by 'jfarthing84' instead on v3.9.1, this also did not allow the slug to be changed.

This does seem to be a common problem.

A quick search in Google for "slug" "is already in use by another term" "wordpress" renders about 389,000 results.

comment:30 @johnbillion9 months ago

  • Version changed from trunk to 2.9

comment:31 @JustinSainton9 months ago

FWIW, just applied the patch to 4.0-src, and everything works as expected. Within the same post type, hierarchical or not, I cannot have duplicate slugs. Across post types, hierarchical or not, I can have duplicate slugs. This seems to have always been possible with non-hierarchical post types, but not with hierarchical.

Also, I've noted that the ability to create taxonomy terms of the same name, across hierarchical custom taxonomies, seems to be possible as of this patch.

I'm surely missing something, but this definitely seems to be an improvement. Happy to dig into unit tests for this - if anyone has pointers for what specific assertions we should be testing for, I'm all ears - otherwise, I'll just give it a go.

comment:32 in reply to: ↑ 14 @nacin9 months ago

  • Component changed from Rewrite Rules to Permalinks
  • Keywords commit has-unit-tests added; needs-testing removed
  • Milestone changed from Future Release to 4.1

Replying to SergeyBiryukov:

Replying to mboynes:

Can someone please explain why wp_unique_post_slug works the way it does with regards to hierarchical custom post types versus non-hierarchical custom post types?

Introduced in [11125] as a fix for page/attachment slug conflicts in #6437.

[21845] (#15665) was a more recent fix. Perhaps [11125] can be reverted now.

I've traced this same history and I concur with this finding. It should never have been turned into what it is now.

I'm going to upload a new patch with some basic tests. If someone wants to expand on this, that's fine. Honestly, the only way this can go wrong if two post types share the same rewrite base, and or have their rewrite base removed, and that's going to have problems at the rewrite level, not just this level.

@nacin9 months ago

Patch by mboynes, I just tweaked it a little and added tests.

comment:33 @adisos8 months ago

What about custom taxonomies?

Why I cannot set the same slug for category and tag?

comment:34 @wonderboymusic7 months ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 30158:

All duplicate slugs across different post types.

Adds unit test.

Props mboynes, nacin.
Fixes #18962.

comment:35 @wonderboymusic7 months ago

In 30159:

Allow get_pages(), with child_of passed to it, to work with interrupted hierarchies.

Adds unit test.
Fixes #18962.

comment:36 @wonderboymusic7 months ago

Tagged the wrong ticket in [30159]. (Correct ticket: #14477)

Last edited 6 months ago by dd32 (previous) (diff)

comment:37 @wonderboymusic7 months ago

In 30246:

In get_page_children(), only check $page->ancestors once to avoid duplicates when the function recurses. Adds an argument, $ancestors.

Fixes #18962.

comment:38 @wonderboymusic7 months ago

Oh god - tagged the wrong ticket again (Correct Ticket: #14477)

Last edited 6 months ago by dd32 (previous) (diff)

comment:40 follow-up: @dd326 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@wonderboymusic - How does this change handle attachments?

Attachments need to be unique across all post types (I'm not entirely sure why, but that's what the code says), take this scenario:

  1. Create a page /test/
  2. Upload a file which ends up at /test/example/
  3. Create a page /test/example/

Previously creating the page would be blocked and created as /test/example-2, but it looks like this now allows that, and as a result the post_type check should be changed to post_type IN( $post_type, 'attachment')?

Re-opening pending that and #30339

@dd326 months ago

Includes fix from julien731 in #30339

comment:41 @dd326 months ago

Added a unit test which shows the scenario, and the potential fix. This also includes the change for #30339 and a unit test for it.

comment:42 @johnbillion6 months ago

The test_get_page_by_path_priority() unit test gets broken by these new tests. Now fixing it, then this will go in.

comment:43 @johnbillion6 months ago

Actually, test_get_page_by_path_priority() is broken by the addition of attachment to the query clause in 18962.2.diff.

I'm going to leave that out for beta 2 and circle back to it.

comment:44 @johnbillion6 months ago

In 30480:

Correct an SQL syntax error introduced in r30158. Adds tests.

Fixes #30339
See #18962
Props julien731

comment:45 in reply to: ↑ 40 @boonebgorges6 months ago

Replying to dd32:

@wonderboymusic - How does this change handle attachments?

Attachments need to be unique across all post types (I'm not entirely sure why, but that's what the code says), take this scenario:

  1. Create a page /test/
  2. Upload a file which ends up at /test/example/
  3. Create a page /test/example/

Previously creating the page would be blocked and created as /test/example-2, but it looks like this now allows that, and as a result the post_type check should be changed to post_type IN( $post_type, 'attachment')?

Re-opening pending that and #30339

I can't reproduce this previous behavior. On 4.0, reating that page /test/example/ will successfully create a page with the slug 'example', creating a URL clash with the attachment. The changes in [21845] mean that the page always wins, so that the attachment page is inaccessible. (See #15665.) And, in fact, if what dd32 is saying here were true, I'm not sure how test_get_page_by_path_priority() ever would have passed.

My interpretation is: [21845] didn't really fix the problem. It put the fix in the URL parser (get_page_by_path()) when it should have prevented the duplicate slug to begin with. dd32's suggestion with 18962.2.diff seems like it's the correct fix for the current ticket, and it also seems like a partial fix for #15665. (The "proper" fix for the latter ticket would probably be more specific: when creating an attachment, check that the parent post doesn't have a child with the same slug, and vice versa.)

@boonebgorges6 months ago

See #30284.

comment:46 @dd326 months ago

I can't reproduce this previous behavior.

Neither can I now, although I did test it on an actual installation. So you're right, that's not a regression from 4.0.

It does however seem like a edgecase that should be considered, creating a page that conflicts with a existing image attachment at that url level seems wrong. Especially considering that attachments uploaded after the page is created will avoid that clash.

@boonebgorges6 months ago

comment:47 @boonebgorges6 months ago

  • Keywords commit removed

It does however seem like a edgecase that should be considered, creating a page that conflicts with a existing image attachment at that url level seems wrong. Especially considering that attachments uploaded after the page is created will avoid that clash.

Agreed. 18962.patch is my suggested fix. It implements dd32's suggestion that wp_unique_post_slug() check attachments as well as the same post_type for post_name clashes. As noted, this breaks test_get_page_by_path_priority(), because in the process of setting up fixtures, you can no longer natively create a page and attachment with the 'some-page' slug. So I suggest directly updating the DB in order to replicate the (legacy) situation where you have an attachment/page conflict, so that the get_page_by_path() fix continues to be tested.

comment:48 @johnbillion6 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 30629:

Check attachments as well as the current post type when checking for duplicate post slugs. This avoids creating a page with a slug which conflicts with an existing attachment (the inverse is already handled).

Updates the existing test for pages which should take priority over attachments in cases where an existing clash exists.

Fixes #18962
Props boonebgorges

Note: See TracTickets for help on using tickets.