WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#29615 closed defect (bug) (fixed)

Non-Page Hierarchical Post Type 'Default Permalinks' do not work for child posts

Reported by: loushou Owned by: SergeyBiryukov
Milestone: 4.0.1 Priority: normal
Severity: normal Version: 4.0
Component: Permalinks Keywords: has-patch commit fixed-major
Focuses: Cc:
PR Number:

Description

Intro

I recently helped release a plugin that I have worked on for a while now. Until now, every environment that has used this plugin has been using 'non-default' permalinks, usually something involving %postname%. Yesterday I as doing some testing on a clean, unmodified install of WordPress and found that my post permalinks are not working. After some short investigation this morning I found out why, which I will explain in a second. Also, I did some checking on my various legacy dev environments and have found that this problem has been around since at least 3.2.1. Here is a detailed explanation of the problem and it's roots:

My Setup(s)

I have verified that this problem exists in as early as version 3.2.1 and as late as version 4.0 (official release). My dev environments span the gamut between those versions, and this problem exists in all of them.

Description

The basic problem boils down to one key issue: there is a mismatch in permalink generation and permalink interpretation for 'non-page' hierarchical post types, when using 'default permalinks' (http://example.com/?p=123).

For 'non-page' hierarchical post types, when we generate the permalink for the post, while the 'default permalinks' are selected, we use this format: http://example.com/?post_type=pagename , where 'post_type' is the 'slug name' of the post type itself, and 'pagename' is equal to '$post->post_name'. Nothing seems wrong with this until you look at how the permalink is interpreted by WP_Query. In WP_Query, when looking up a 'non-page' hierarchical post type, we use the 'get_page_by_path()' function to determine the post_id. This function expects that the pagename be something like http://example.com/?my_post_type=parent-post/child-post. In the current format of the permalink (http://example.com/?my_post_type=child-post), that function tries to find a 'parent post' (post_parent = 0) that is named 'child-post', and thus fails, returning 0 as the post_id.

Reproduction Steps

  • Freshly install any version of WordPress 3.2.1 or later.
  • Verify that 'Default' is selected as the permalink structure on 'Settings -> Permalinks'
  • Create a hierarchical post type
  • Create a parent post of that post type
  • Create a child post of that parent post, of that same post type
  • Click the 'view post' link in the edit post page of the child post (or browse to it some other way)
  • Observe a 404 error

Expectations

First, this is only a problem when there is a 'non-page' hierarchical post type and default permalinks, which by definition means that it is only a problem when a plugin or theme is active. None-the-less it is a problem.

Second, when clicking the admin 'view post' link, we should be able to view the post. This logic flaw prevents that in all cases.

Lastly, the get_permalink function (more specifically the get_post_permalink() function) should return a permalink that can be interpreted by WP_Query, no matter what permalink structure is used.

Additional Information

Based on some research, it seems that this whole problem could be solved by moving one of the if statements in the get_post_permalink() up a couple ranges, so that it affects both halves of the primary if statement. To be clearer, in version 4.0, move wp-includes/link-template.php@262-263 above line wp-includes/link-template.php@260.

If this is done, then the else statement on wp-includes/link-template.php@267 will also have a hierarchical value for $slug, and thus will produce proper permalinks. I have tested it any it will still produce valid non-hierarchical permalinks with this code also.

Attachments (3)

link-template.php (93.4 KB) - added by loushou 5 years ago.
Patched version of wp-includes/link-template.php
link-template.php.diff (735 bytes) - added by loushou 5 years ago.
actual patch file for change
29615.patch (1.9 KB) - added by SergeyBiryukov 5 years ago.

Download all attachments as: .zip

Change History (20)

@loushou
5 years ago

Patched version of wp-includes/link-template.php

@loushou
5 years ago

actual patch file for change

#1 @loushou
5 years ago

  • Keywords has-patch added

#2 @loushou
5 years ago

I made the patch, as per instructions http://codex.wordpress.org/Contributing_to_WordPress http://codex.wordpress.org/Reporting_Bugs and http://markjaquith.wordpress.com/2005/11/02/my-wordpress-toolbox/ . According to the Reporting_Bugs page, there should have been an 'accept' button on this ticket page, where I could accept responsibility for the ticket, but I do not see one on my version of the page. Hope just submitting a patch this way is fine.

Loushou

#3 @loushou
5 years ago

  • Summary changed from Hierarchical Post Type 'Default' Permalinks to Non-Page Hierarchical Post Type 'Default Permalinks' do not work for child posts

#4 @netweb
5 years ago

Question for you, was your plugin working with WordPress 3.9.2?

I have a similar issue and before creating a new ticket I wanted to confirm this.

#5 @netweb
5 years ago

  • Keywords reporter-feedback added

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


5 years ago

#7 @SergeyBiryukov
5 years ago

  • Keywords needs-unit-tests added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.1

#8 @SergeyBiryukov
5 years ago

  • Milestone changed from 4.1 to 4.0.1

This appears to be a regression introduced in [28803] as a fix for #28611. I could not reproduce it in 3.9.

With permalinks enabled, you'll get this structure after following the steps 1 to 4 in #28611:

http://trunk.wordpress/handbook/getting-started/
http://trunk.wordpress/handbook/contributing-to-the-wordpress-codex/
http://trunk.wordpress/handbook/contributing-to-the-wordpress-codex/getting-started/

With permalinks disabled, you'll get a conflicting permalink for the first and third post:

http://trunk.wordpress/?handbook=getting-started
http://trunk.wordpress/?handbook=contributing-to-the-wordpress-codex
http://trunk.wordpress/?handbook=getting-started

This part is not a regression, I can reproduce it in 3.0.

Before [28803], when visiting that permalink, you would see both posts in a single post template.

After [28803], you would only see the first (top-level) post. But if there is no top-level post with that slug, you would see no posts at all, so the fix was incomplete.

With link-template.php.diff, the permalinks are correct and work as expected:

http://trunk.wordpress/?handbook=getting-started
http://trunk.wordpress/?handbook=contributing-to-the-wordpress-codex
http://trunk.wordpress/?handbook=contributing-to-the-wordpress-codex/getting-started

Working on a unit test.

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#9 @SergeyBiryukov
5 years ago

  • Keywords commit added; needs-unit-tests removed

29615.patch adds a unit test. Other tests still pass when running the whole suite.

#10 @loushou
5 years ago

@netweb - the answer to your question is, I'm not entirely sure, in practice. as I mentioned before the plugin has been in active use for quite a while, but only on sites that have a pretty permalink structure; however, take a look at the next response below.

@SergeyBiryukov - thanks for taking a look into this. I have tested this directly in version 4.0, 3.9.2, 3.8.1, 3.8.0, 3.6.1, 3.6.0, 3.5.2, 3.5.0, 3.4.2, & 3.2.1. this problem, if reproduced following my outlined list of steps, is present in _all_ of those versions. most of the relevant code across all those versions has not changed.
no major changes (or even minor changes really) in the permalink generation function across all those version ( get_post_permalink() ). there have also been no major changes (or minor changes) to the wp_query->get_posts() function dealing with the pagename query_var across all those versions. the get_page_by_path() however, has several major changes over the course of the mentioned versions.
in all the above versions I can follow these steps and produce one of the two problems we have discovered, the original problem I reported (the 404) and the problem that you identified (wrong post displayed):

  • verify the 'default permalinks'
  • create the post type
  • create a parent post named 'parent-post'
  • create a child post of parent-post, called 'child-post'
  • create a parent post named 'child-post'
  • attempt to browse to the child post called 'child-post'
  • some versions i get a 404, some versions i get the parent post named 'child-post'

I have not tested version 3.9.0, so there may very well have been a 1 or 2 version change that solves it for those versions, but i have tested this issue in all the versions in the above list, and they _all_ have one of the two problems.
Again, thanks for verifying that this is a problem, and that my patch works. Also thanks a bunch for adding the unit test, because that will ensure that this does not continue to slip through the cracks. You have been a great help man.

Loushou

#11 @nacin
5 years ago

#29677 was marked as a duplicate.

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


5 years ago

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


5 years ago

#14 @SergeyBiryukov
5 years ago

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

In 29765:

Create correct permalinks for child posts of hierarchical post types when default permalinks are used.

props loushou.
fixes #29615 for trunk.

#15 @SergeyBiryukov
5 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

#16 @loushou
5 years ago

Thanks :)

#17 @nacin
5 years ago

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

In 30260:

Create correct permalinks for child posts of hierarchical post types when default permalinks are used.

Merges [29765] to the 4.0 branch.

props loushou.
fixes #29615.

Note: See TracTickets for help on using tickets.