Make WordPress Core

Opened 14 years ago

Closed 12 years ago

Last modified 11 years ago

#15665 closed defect (bug) (fixed)

Page name won't work if the parent page has an attachment with the same name

Reported by: designsimply's profile designsimply Owned by: nacin's profile nacin
Milestone: 3.5 Priority: high
Severity: normal Version: 3.1
Component: Permalinks Keywords: has-patch needs-testing early
Focuses: Cc:

Description

It's possible to create a page name that has the same name as an attachment page with the same parent. If you do that, the attachment page takes precedent and it's not possible to display the page with the matching name.

To reproduce:

  1. Create a page, attach some images to it, save and publish
  2. Create a child page with the same name as one of images attached to the parent page

---> Result: a page name and an attachment page name that are the same and it's not possible to view the page at http://example.com/parent_page_name/page_name/

Tested with r16708

Making the attachment post type hierarchical fixes it this my tests. Is there any reason attachment post types should not be hierarchical?

Attachments (5)

post.diff (517 bytes) - added by designsimply 14 years ago.
garyc40.15665.diff (1.0 KB) - added by garyc40 14 years ago.
fixed get_page_by_path()
15665.patch (1013 bytes) - added by SergeyBiryukov 12 years ago.
15665-unit.patch (2.4 KB) - added by devesine 12 years ago.
Here's a unit test which tests this bug. Comments gratefully accepted - particularly as this is the first WP test I've written.
15665-unit-2.patch (2.5 KB) - added by devesine 12 years ago.

Download all attachments as: .zip

Change History (34)

@designsimply
14 years ago

#1 @nacin
14 years ago

This is definitely duplicating another ticket somewhere. But I think this is the first time I've seen it well-explained and with a patch. :)

#2 @scribu
14 years ago

  • Keywords needs-patch added; has-patch needs-testing removed
  • Milestone changed from Awaiting Review to Future Release

Making the attachment post type hierarchical fixes it this my tests. Is there any reason attachment post types should not be hierarchical?

Yes, attachments are not hierarchical because it doesn't make sense to link an attachment to another attachment.

We should find a better way to fix this.

#3 follow-up: @westi
14 years ago

I suspect the real fix here is to find out why we match attachment slugs when looking up root level pages

#4 @westi
14 years ago

  • Cc westi added

#5 @designsimply
14 years ago

Found two additional cases where it's possible to add pages that conflict with auto-generated pages:

  • author/EXISTING_BLOG_USER
  • category/EXISTING_CATEGORY

This is in addition to the case originally reported where it's possible to add a page that conflicts with existing attachment pages names for the same parent page:

  • PARENT_PAGE/ATTACHMENT_PAGE

@garyc40
14 years ago

fixed get_page_by_path()

#6 @garyc40
14 years ago

  • Keywords has-patch needs-testing 3.2-early added; needs-patch removed

#7 @garyc40
14 years ago

Can't reproduce your problem with author and category though. More detailed steps?

#8 @SergeyBiryukov
13 years ago

  • Keywords needs-refresh added

Closed #16438 as a duplicate.

#9 @Ipstenu
13 years ago

  • Cc ipstenu@… added

#10 in reply to: ↑ 3 ; follow-up: @duck_
13 years ago

#19683 was closed as a duplicate, apparently root level pages were not susceptible to collisions prior to 3.3. This is might due to changes in get_page_by_path(), see #16687.

Replying to westi:

I suspect the real fix here is to find out why we match attachment slugs when looking up root level pages

get_page_by_path() queries for attachment as well as the specified post type.

Related #17170.

#11 in reply to: ↑ 10 @duck_
13 years ago

Replying to duck_:

apparently root level pages were not susceptible to collisions prior to 3.3.

Sorry for repeating that without double checking. It's wrong.

#12 @SergeyBiryukov
12 years ago

Closed #20612 as a duplicate.

#13 @Viper007Bond
12 years ago

  • Component changed from General to Permalinks
  • Priority changed from normal to high

This is super annoying. We should resolve this.

#14 @SergeyBiryukov
12 years ago

Closed #21008 as a duplicate.

#16 @SergeyBiryukov
12 years ago

  • Keywords early added; 3.2-early needs-refresh removed
  • Milestone changed from Future Release to 3.5

15665.patch compares found post type with the requested one before breaking the loop, to see if we can find an item that meets all other requirements and also has the same post type.

#17 @nacin
12 years ago

  • Keywords needs-unit-tests added

+1 for 3.5.

@devesine
12 years ago

Here's a unit test which tests this bug. Comments gratefully accepted - particularly as this is the first WP test I've written.

#18 @devesine
12 years ago

15665-unit.patch executes the reproduction steps described in the bug; currently fails, passes with patch applied.

#19 @trepmal
12 years ago

Just came across a little issue which I believe is related to this.

If I have a page "Rooms," for example, with child page "Amenities" as well as a CPT with 'rooms' as the rewrite slug (below), the child page /rooms/amenties/ will 404 as 'amenties' is expected to be part of the CPT

register_post_type('room', array(
	//...
	'rewrite' => array('slug' => 'rooms')
	//...
));

If unrelated, let me know, I'll check again for an existing ticket or open a new one.

#20 follow-up: @nacin
12 years ago

I think we can simply call wp_insert_attachment() rather than actually upload a file for 15665-unit.patch.

trepmal, that does not appear related.

#21 in reply to: ↑ 20 @devesine
12 years ago

Replying to nacin:

simply call wp_insert_attachment()

Looks like that works fine; updated in 15665-unit-2.patch.

#22 @SergeyBiryukov
12 years ago

Closed #21216 as a duplicate.

#23 @SergeyBiryukov
12 years ago

Closed #21501 as a duplicate.

#24 @nacin
12 years ago

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

In [21845]:

In get_page_by_path(), attempt to return an item that has the same post type before returning an attachment with the same name. props SergeyBiryukov. tests in [UT1021]. fixes #15665.

#25 @nacin
12 years ago

  • Keywords needs-unit-tests removed

#26 @SergeyBiryukov
11 years ago

#24612 was marked as a duplicate.

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


11 years ago

#28 follow-up: @aaroncampbell
11 years ago

The same problem occurs with posts if you're using the popular /%postname%/ permalink structure. If you upload an image and then create a post that gets the same slug as the image (different post type, so there is no collision protection here), the front end will display the attachment rather than the post at site.com/some-shared-slug

#29 in reply to: ↑ 28 @SergeyBiryukov
11 years ago

Replying to aaroncampbell:

The same problem occurs with posts if you're using the popular /%postname%/ permalink structure. If you upload an image and then create a post that gets the same slug as the image (different post type, so there is no collision protection here), the front end will display the attachment rather than the post at site.com/some-shared-slug

Thanks, I've reopened #24612 (which I thought was a duplicate, but this ticket only covered pages). See also #13459.

Note: See TracTickets for help on using tickets.