Make WordPress Core

Opened 9 years ago

Last modified 8 years ago

#35562 new defect (bug)

Single post pagination redirect back to page one with nextpage tag - WordPress 4.4.1

Reported by: pietergoosen's profile pietergoosen Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.4
Component: Canonical Keywords: needs-patch
Focuses: Cc:

Description

When a single post post content, $post->post_content, does not contain the <!--nextpage--> tag, and we append more pages to the single post through the content_pagination filter, pagination works as expected

Sample code to test

add_filter( 'content_pagination', function ( $pages )
{
    $pages[] = 'This is another page we want to append';
	
    return $pages;
});

However, when we add the <!--nextpage--> tag inside the post content ($post->post_content}}} and run the same sample code, any appended page simply redirects back to page one as a 404 header is set on appended pages.

I tracked this down to the following block of code (lines 623 - 628) inside the WP class (wp-includes/class-wp.php which seems to be introduced in either v4.4.0 or 4.4.1

// check for paged content that exceeds the max number of pages
$next = '<!--nextpage-->';
if ( $p && false !== strpos( $p->post_content, $next ) && ! empty( $this->query_vars['page'] ) ) {
	$page = trim( $this->query_vars['page'], '/' );
	$success = (int) $page <= ( substr_count( $p->post_content, $next ) + 1 );
}

Removing those lines fixes the issue.

What I recommend is that instead of simply checking for the <!nextpage--> and returning a 404 according to that, we must rather have a check that acts on the amount of pages we have counted immediately after the content_pagination filter and then return a 404 if the current page exceeds the amount of pages after the content_pagination filter

Change History (5)

#1 @Milamber
9 years ago

#35544 was marked as a duplicate.

#2 @dd32
9 years ago

  • Component changed from General to Canonical
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 4.4.1 to 4.4

Caused by #11694 / [34494] because it wasn't updated to respect [35285] / #9911

@chriscct7 @DrewAPicture @wonderboymusic Any chance one of you could take a peak at making it work?

Last edited 9 years ago by dd32 (previous) (diff)

#3 follow-up: @birgire
9 years ago

The problem is that WP::handle_404() is executed long before WP_Query::setup_postdata() where the content_pagination filter currently lives.

I don't think we want to remove a restriction in handle_404() that makes sure the current page number doesn't exceed the maximum number of content pages.

If we want to keep it, I suggest we try to move the content pagination logic (including the content_pagination filter) from the setup_postdata() up to the handle_404() method.

The current content pagination isn't supposed to work for secondary loops anyways, as far as I understand it, even though the content_pagination filter is available to all loops through setup_postdata().

To support such a restriction for secondary loops, I think the corresponding post ID would need to be part of the GET request.

... or do you see any other way ?

Last edited 9 years ago by birgire (previous) (diff)

#4 in reply to: ↑ 3 @pietergoosen
9 years ago

Replying to birgire:

The problem is that WP::handle_404() is executed long before WP_Query::setup_postdata() where the content_pagination filter currently lives.

I don't think we want to remove a restriction in handle_404() that makes sure the current page number doesn't exceed the maximum number of content pages.

If we want to keep it, I suggest we try to move the content pagination logic (including the content_pagination filter) from the setup_postdata() up to the handle_404() method.

The current content pagination isn't supposed to work for secondary loops anyways, as far as I understand it, even though the content_pagination filter is available to all loops through setup_postdata().

To support such a restriction for secondary loops, I think the corresponding post ID would need to be part of the GET request.

... or do you see any other way ?

What bothers me about this whole issue, and why I suggested to remove that section of code for now is that we can successfully pages to any added pages (through the content_pagination filter) when a <!--nextpage--> tag is not set in the post content. Why can we page past page 1 and any other page after that when we do not have the <!--nextpage--> tag. Is that because there is nothing concretely set to test for a 404 when we page a single post. From what I make from all relevant code is that there is nothing that catches or set a 404 page when we page a single post. Paging a single post is definitely not something new. I really did not have time to test anything after the submission of this ticket due to a massive power failure, so I'm stuck on a mobile device for now.

I think that we will need to look at the whole process of paging a single post and when to set a 404. We should not just look at the <!--nextpage--> tag.

#5 @Milamber
8 years ago

Any update on a fix for this?

Note: See TracTickets for help on using tickets.