post paginating <!--nextpage--> does not work

[hakre]

When <!--nextpage--> is placed at the very beginning of the post content (first page contains zero-length content) then pagination does not work at all.

Example post content for reproduction:

<!--nextpage-->
page2
<!--nextpage-->
page3

This will result in a single page, not in a three or two page setup.

It's related to strpos returning 0 which is casted into false when if'ed.

Related: #16745

[hakre]

Fix

[nacin]

Going to close this as wontfix. I can see someone using <!--nextpage--> at the start, and having it currently work, but then suddenly having an empty first page. If you want to have an empty first page, use a space or empty line or something.

[toscho]

May I ask to implement Hakre’s patch? The patch is really trivial, and I can see no side effects.

For a user on the other hand, the result is not intuitive. There is also no hint available for the user how she can fix it.

[SergeyBiryukov]

Replying to toscho:

May I ask to implement Hakre’s patch? The patch is really trivial, and I can see no side effects.

One side effect is mentioned in comment:2. I agree that the current result is not intuitive though.

[toscho]

Replying to SergeyBiryukov:

One side effect is mentioned in comment:2. I agree that the current result is not intuitive though.

But that is not true: if <!--nextpage--> is at the first position, paging does not work. You get the content of all pages on one page, because the comparison …

if ( strpos( $content, '<!--nextpage-->' ) )

… is not strict enough. There is even a ​big red warning on the manual for that behavior:

This function may return Boolean FALSE, but may also return a non-Boolean value which evaluates to FALSE. Please read the section on Booleans for more information. Use the === operator for testing the return value of this function.

[nacin]

I did understand the initial report here, and based on my original understanding of the surrounding code, I did not think this patch should be applied. Basically, if someone had <!--nextpage--> at the start of a post, and then later in the post, they'd currently have two pages. With this patch, they would have three — including an empty first page. This is not expected and is fairly lame. While a "page break" usually means to immediately start a new page, <!--nextpage--> just as much marks the start of a page, and if a user was using it to mark the start of page 1 (and it worked) then they should not be punished. I also assumed, knowing the age of this code, that this check existed since b2.

However, in this case, I was wrong. While this code is from b2, ​b2 used a preg_match that *did* obey <!--nextpage--> at the start of a string. It was changed to strpos() in [8754], for performance but it also changed the functionality.

Additionally, this is check is to decide whether there is pagination at all. If the post starts with <!--nextpage-->, strpos() will return 0, and no other <!--nextpage--> will be listened to, and the post will not be paginated. That is clearly incorrect behavior.

While failing to explicitly and strictly check the return value of strpos() is a code smell, it helps when bug reports point out why something smells, as well as any repercussions. In this case, I had felt that the implicit > 0 (needle in haystack, and needle not at start of haystack) was correct.

So — I think this would be best with pieces of both the current and old behaviors: Ignore <!--nextpage--> at the start of the string (so, no empty page), but don't let it prevent pagination.

[nacin]

In 1242/tests:

Move tests/query.php to tests/query/conditionals.php. New query test for setup_postdata() pagination with nextpage. see #16746.

[toscho]

Just an idea: maybe we can remove leading <!--nextpage--> string including surrounding whitespace on save_posts?

[SergeyBiryukov]

Replying to toscho:

Just an idea: maybe we can remove leading <!--nextpage--> string including surrounding whitespace on save_posts?

We don't modify content on save_post for any other reason, so I guess we shouldn't do that in this case either.

[nacin]

In 24599:

Avoid getting tripped up on post content that starts with <!--nextpage-->. props SergeyBiryukov. fixes #16746.

[nacin]

In 1307/tests:

Update post content parsing tests for [24598] and add new tests for [24599]. see #16746.

[nacin]

In 1323/tests:

Simple test for wp_count_posts(). props johnpbloch. see #16746.