wp_unique_post_slug() not used when inserting attachment from media / add new

[Denis-de-Bernardy]

patch attached

[Denis-de-Bernardy]

this is required to sort out #9539

[hakre]

nice find.

[ryan]

(In [11202]) Use wp_unique_slug() in wp_insert_attachment(). Props Denis-de-Bernardy. fixes #9726

[azaozz]

This breaks the gallery as it allows identical slugs for attachments with different parents. Since posts are not hierarchical like pages, some of the rewrite rules depend on all attachments having unique slugs.

Is there any particular need for this patch?

[azaozz]

(In [11461]) Revert [11202], see #9726

[Denis-de-Bernardy]

before the patch, it was causing weirdness on static pages that shared their slug with attachments. #9539 has yet to be committed btw, and it's required concluding patch to make it all work as needed.

can you expand a bit on the issue you're getting so I can look into it?

[azaozz]

Perhaps the proper solution is to check attachment slugs against other attachments and also pages.

To reproduce with the patch: upload a file to a post; upload another file with the same name to different post; visit the attachment post for one of the files. Both attachment posts are displayed (may be theme dependent but happens in the default theme).

[Denis-de-Bernardy]

Replying to azaozz:

Perhaps the proper solution is to check attachment slugs against other attachments and also pages.

That's precisely what wp_unique_post_slug() is supposed to do. :-)

To reproduce with the patch: upload a file to a post; upload another file with the same name to different post; visit the attachment post for one of the files. Both attachment posts are displayed (may be theme dependent but happens in the default theme).

OK. Will give it a shot and nail it down.

[azaozz]

Replying to Denis-de-Bernardy:

That's precisely what wp_unique_post_slug() is supposed to do. :-)

No, it only checks for posts with the same parent. Attachment slugs need to be unique globally.

[Denis-de-Bernardy]

ah, ok. please restore 11202 then, I'll change wp_unique_post_slug accordingly during the afternoon and upload the patch.

[Denis-de-Bernardy]

quick ref so I don't forget:

[11:58] <azaozz> as deleting the parent page will mess things up a lot..
[11:59] <ddebernardy> I believe the slug gets changed in case this results in a conflict
[12:00] <ddebernardy> will test that while I'm at it

[ryan]

How about making unique against all posts/pages/attachments that are not draft or revision?

[Denis-de-Bernardy]

dunno. I think the actual issue is two-fold:

• the check we replaced in the initial patch for wp_unique_post_slug() is actually erroneous. the sql params differ from a check to the next
• the check we replaced in this ticket's patch was yet another sql statement

uploading a new patch in a sec, it's still untested.

[ryan]

Patch makes unique across all post types and states and insulates slugs for published posts and pages from changes to uniqueness rules.

[Denis-de-Bernardy]

yeah, but the trouble with this is, if you're creating a site that is static page oriented, and for some reason you've a slug that is shared between a static page and a post, 9726.4.diff would prevent the page from getting its preferred slug. That's not necessarily desirable. :-|

[ryan]

I like that, but it is a deparature from past behavior.

Shouldn't attachments be unique across all types, though?

[ryan]

.5 is based on .4 but modifies the attachment query to look across all types and statuses and adds some comments.

[ryan]

Correction, .5 is based on .3, not .4.

[Denis-de-Bernardy]

I'll give it a shot tomorrow. Still got 15 plugins in need of a rewrite over here. And finding bugs like #9944 along the way. ;-)

[azaozz]

9726.5.diff is working well: attachments have globally unique slugs, child pages with different parents can have the same slug, and posts are unique among themselves.

[ryan]

(In [11467]) wp_unique_post_slug() fixes. Props Denis-de-Bernardy. fixes #9726