Page parameter no longer works in wp_list_comments

[smerriman]

wp_list_comments has a page parameter which allows you to display any page of comments. Since WP 4.4 introduced the idea of only querying the comments that are 'needed', this no longer works at all. Whatever you pass in, only the 'current' page of comments gets displayed.

I was previously using this in two ways:

a) Due to the long standing 'bug' of the first page of comments possibly being very empty when comment order is reversed (which was nearly fixed then reverted in recent tickets), I display both the first two pages of comments in comments.php. The second 'page' now duplicates the first.

b) I was using AJAX to display further pages of comments, by calling comments_template and passing in a custom page parameter to wp_list_comments. This also fails.

There should be a solution that make the page parameter work in the way it used to - perhaps if the page being requested is not available, perform another query at that point.

Perhaps additionally, the parameters passed to WP_Comment_Query in the comments_template function should be filterable so you can tell it what pages you actually need if the defaults don't suffice, rather than relying on a single global cpage query variable.

[boonebgorges]

Hi @smerriman - Thanks for the ticket.

It's quite possible that something was missed in 4.4 with respect to custom wp_list_comments() arguments. But I'm having a hard time reproducing it. When I call wp_list_comments() with custom values of 'page'/'per_page', and those params don't match the post URL, the parameters appear to be obeyed. Can you give some complete code that I can use to demonstrate the issue?

[smerriman]

I just installed Twenty Fifteen with a fresh version of WP 4.4 (no plugins), created a post, and added 5 top level comments. Under Settings - Discussion I set it to break into 2 top level comments per page (last displayed by default, older comments at top of page - though changing those settings doesn't alter the result). You should just see comment 5 on the post.

In comments.php I copied the wp_list_comments line and pasted a second copy directly underneath with the page parameter:

wp_list_comments( array(
	'style'       => 'ol',
	'short_ping'  => true,
	'avatar_size' => 56,
	'page'=>2
) );

You should then see comment 5 duplicated on the post - whatever page setting you choose.

wp_list_comments has:

// Pagination is already handled by `WP_Comment_Query`, so we tell Walker not to bother.

and then some code which ignores the page parameter and sets the cpage parameter directly from the query var, so it shouldn't be possible to get any other result.

[boonebgorges]

Thanks, @smerriman. I now see what's happening.

perhaps if the page being requested is not available, perform another query at that point.

I think this is probably right. wp_list_comments() is a bit of a jumble, and it would be nice to rethink it at some point so that it's more easily maintainable down the line. But for the short term, I'd like to propose something like 35175.diff​. It says: if wp_list_comments() contains a 'page' param, and 'page' doesn't match the current cpage query var, then query for all of the post's comments and let Walker do the pagination. This technique misses out on some of the performance wins from 4.4 for standard comment queries, but it's on a par with 4.3 and earlier, and it's far easier to implement for 4.4.1 than a more general overhaul.

Would you mind having a look and letting me know whether it accomplishes what you need it to do? It needs a boatload more unit tests to cover all the permutations of 'default_comment_page' etc, but it should give you an idea of what I have in mind.

[smerriman]

Yep, that's obviously not optimal performance wise as you mentioned, but solely from the perspective of backwards compatibility I think that sort of idea should suffice as a quick fix.

A couple of points:

• I think you'll want is_singular to handle custom post types rather than just posts/pages.
• Now that I think about it, the problem is more widespread than simply the page parameter - anything that affected the query will have issues too. This applies to per_page and reverse_top_level at least, and I think maybe reverse_children as well - even type? max_depth is still handled correctly by the walker I believe.

[boonebgorges]

Thanks for having another look, @smerriman.

I think you're correct about per_page. On a second look, reverse_top_level should be properly respected by enforcing that order of the additional comment query is always ASC, matching the order of the comment query in comments_template(); the logic later in wp_list_comments() plus the walker should handle the rest. Unit tests in 35175.2.diff​ demonstrate both of these.

@smerriman Can you test this against your actual use cases? I have run a number of tests, but there are many possible configurations, and I want some validation that I haven't missed anything.

[boonebgorges]

In 36157:

Ensure that non-default pagination values work in wp_list_comments().

Prior to 4.4, it was possible to pass 'page' and 'per_page' values to
wp_list_comments() that do not match the corresponding global query vars.
This ability was lost in 4.4 with the refactor of how comments_template()
queries for comments; when the main comment query started fetching only the
comments that ought to appear on a page, instead of all of a post's comments,
it became impossible for the comment walker to select comments corresponding to
custom pagination parameters. See #8071.

We restore the previous behavior by (a) detecting when a 'page' or 'per_page'
parameter has been passed to wp_list_comments() that does not match the
corresponding query vars (so that the desired comments will not be found in
$wp_query), and if so, then (b) querying for all of the post's comments and
passing them to the comment walker for pagination, as was the case before 4.4.

Props boonebgorges, smerriman.
Fixes #35175.

[boonebgorges]

Reopening for 4.4.1.

[boonebgorges]

In 36158:

Ensure that non-default pagination values work in wp_list_comments().

Prior to 4.4, it was possible to pass 'page' and 'per_page' values to
wp_list_comments() that do not match the corresponding global query vars.
This ability was lost in 4.4 with the refactor of how comments_template()
queries for comments; when the main comment query started fetching only the
comments that ought to appear on a page, instead of all of a post's comments,
it became impossible for the comment walker to select comments corresponding to
custom pagination parameters. See #8071.

We restore the previous behavior by (a) detecting when a 'page' or 'per_page'
parameter has been passed to wp_list_comments() that does not match the
corresponding query vars (so that the desired comments will not be found in
$wp_query), and if so, then (b) querying for all of the post's comments and
passing them to the comment walker for pagination, as was the case before 4.4.

Merges [36157] to the 4.4 branch.

Props boonebgorges, smerriman.
Fixes #35175.

[boonebgorges]

In 36276:

Always respect $comments array passed to wp_list_comments().

[36157] fixed a bug whereby wp_list_comments() would not properly recognize
custom pagination arguments. See #35175. However, it inadvertently introduced
a bug that caused any $comments array explicitly passed to the function to be
ignored, when that array was accompanied by pagination arguments that differ
from those in $wp_query. We address this bug by moving the logic introduced
in [36157] inside a block that only fires when no $comments array has been
provided to the function.

Props ivankristianto.
Fixes #35356.

[smerriman]

@boonebgorges - sorry for the lack of response before - didn't have as much access over the holidays. The patch does seem to solve all of my use cases, and you were correct about reverse_top_level - I had mistakenly thought it reversed prior to calculating pages, but that still works fine.

There is a change in behaviour if the type variable is passed though - if I take the above example of 5 comments, make the third one a track, and add a second wp_list_comments call passing type=trackback, the trackback would previously appear on page 1; now it appears on page 2.

I'm not entirely sure which behaviour is more logical - initially I thought a wp_list_comments('type=trackback') should list the first x trackbacks, but on the other hand you can't use proper pagination on trackbacks alone to begin with. So I can't decide whether it's a bug.

In either case, should a separate ticket be opened for a more performant fix be opened? I suspect the optimal route will be something similar to pre_get_posts for comments, so you can alter the query without needing to ignore the first one and re-query everything.

[boonebgorges]

There is a change in behaviour if the type variable is passed though

Urgh. This is weird. Like you, I'm not sure if the new behavior counts as a bug. If you decide that it does, please open a separate ticket describing what you've described here :)

In either case, should a separate ticket be opened for a more performant fix be opened? I suspect the optimal route will be something similar to pre_get_posts for comments, so you can alter the query without needing to ignore the first one and re-query everything.

Yes! Here we go: #35432

[dd32]

In 36356:

Comments: Always respect $comments array passed to wp_list_comments().

[36157] fixed a bug whereby wp_list_comments() would not properly recognize
custom pagination arguments. See #35175. However, it inadvertently introduced
a bug that caused any $comments array explicitly passed to the function to be
ignored, when that array was accompanied by pagination arguments that differ
from those in $wp_query. We address this bug by moving the logic introduced
in [36157] inside a block that only fires when no $comments array has been
provided to the function.

Merges [36276] to the 4.4 branch.
Props ivankristianto, boonebgorges.
Fixes #35356.