Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#35402 closed defect (bug) (fixed)

per_page parameter no longer works in wp_list_comments

Reported by: ninos-ego's profile Ninos Ego Owned by: boonebgorges's profile boonebgorges
Milestone: 4.4.2 Priority: normal
Severity: major Version: 4.4.1
Component: Comments Keywords: fixed-major
Focuses: template Cc:

Description

The per_page & page parameters are not working anymore in the wp_list_comments()-function since 4.4. For the singular page you fixed that bug (see #35175), in the loop the bug still exists.

This bug affects if $wp_query->max_num_comment_pages is >= 1 (page & per_page parameters will be changed to 0).

https://core.trac.wordpress.org/browser/tags/4.4.1/src/wp-includes/comment-template.php#L1976

I think it's because the WP_Comment_Query uses the param 'comments_per_page' from admin panel (options) instead of the per_page parameter. Because of that $wp_query->max_num_comment_pages returns a wrong value. Changing 'comments_per_page' to 2 solves my problem.

Following function is not working as expected (I get all comments):
<?php wp_list_comments( array(

'page' => $cpaged,
'per_page' => 2

) ); ?>

Attachments (1)

35402.diff (1.6 KB) - added by boonebgorges 9 years ago.

Download all attachments as: .zip

Change History (20)

#1 @Ninos Ego
9 years ago

  • Severity changed from normal to critical

#2 @nphaskins
9 years ago

I can also confirm that this is no longer working. When the per_page param is filled out it shows no comments. With it removed it respects what's in discussion settings.

#3 follow-up: @boonebgorges
9 years ago

  • Milestone changed from Awaiting Review to 4.4.2
  • Severity changed from critical to major

Hi @Ninos Ego and @nphaskins - Thanks for jumping in. Can you share some sample code that demonstrates the issue? When you say "in the loop", do you mean that you're using wp_list_comments() within a WP_Query loop? Are you passing a $comments array to the function? See [36276] #35356 for what may be a related issue.

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

Replying to boonebgorges:

Hi @Ninos Ego and @nphaskins - Thanks for jumping in. Can you share some sample code that demonstrates the issue? When you say "in the loop", do you mean that you're using wp_list_comments() within a WP_Query loop? Are you passing a $comments array to the function? See [36276] #35356 for what may be a related issue.

I am not a coder, but here is a link to one of my many articles that have 1000's of comments -- I have it set to 75 comments per page -- whatever is supposed to make that happen is broken, and has been since the move to version 4.4.

https://www.navycs.com/blogs/2010/05/23/moral-waivers-for-enlistment

#5 @boonebgorges
9 years ago

@navycs Hello - It's likely that this issue is unrelated to what's being discussed in the current ticket. It's possible, however, that what you're seeing is related to #35419; if you once had comment threading enabled on your site, but then turned it off, then this is almost certainly what's happening in your case, and 4.4.2 should fix your problem.

If you find evidence that your issue is directly link to the use of custom pagination parameters in wp_list_comments(), please do provide details.

#6 @Ninos Ego
9 years ago

@boonebgorges sure, I'm using the function in the post content template file, inside the post loop, so inside of:

<?php while ( have_posts() ) : the_post(); ?>

The loop so is called everywhere a loop-post content is included (archives, author, category, search, ...). My used code is following:

<?php
wp_list_comments( array(
        'page'     => $cpaged, // Variable defined before, default: 1
        'per_page' => 2
) );
Last edited 9 years ago by Ninos Ego (previous) (diff)

#7 @nphaskins
9 years ago

@boonebgorges sure thing. I've had comment threading on the entire time and have not turned it off. We use the comment tables in WordPress as a "questions and answers" feature.

This is the simplest version of what I show as failing. I am not using this within the loop, but instead it's being called outside in a different area. Both get_comments and wp_comment_query produce the same results.

$comments = get_comments( array(  'status' => 'approve' ) );
wp_list_comments( array( 'per_page' => 5 ), $comments );

With the per_page param set, only 1 comment is displayed, however the comment object itself is correct with showing all comments. When I remove the per_page param, it then respects what's set under discussion settings.

#8 @boonebgorges
9 years ago

@nphaskins Thanks for this. I'm fairly certain that what you're describing is due to #35356, not the current ticket. Maybe you could test to see whether the problem is still occurring with the latest trunk?

@Ninos Ego Thanks for the details. I'll see what I can do to reproduce.

#9 @boonebgorges
9 years ago

Ninos Ego - I'm having a hard time reproducing what you've described:

$posts = array( 1, 2, 3 ); // post IDs with comments
$q = new WP_Query( array(
    'post__in' => $posts,
) );

while ( $q->have_posts() ) {
    $q->the_post();

    wp_list_comments( array(
        'page' => 2,
        'per_page' => 5,
    ) );
}

wp_list_comments() does nothing here, either in 4.3 or 4.4. This is because $wp_query->comments is empty, and you're not passing a $comments array to wp_list_comments(). The only way I can imagine making this work is by calling comments_template() within the loop. Is that what you're doing? Any more info you can provide on how to reproduce would be great.

#10 follow-up: @Ninos Ego
9 years ago

Oh I'm sorry. Inside this loop I'm calling:

<?php
get_template_part( 'templates/content', 'comment' );

The template file templates/content-comment.php has following content:

<?php if ( ! defined( 'ABSPATH' ) ) {
        exit;
} ?>

<?php global $withcomments; ?>
<?php $withcomments = 1; ?>
<?php add_filter( 'comments_template', '__return_true' ); ?>

<?php if ( comments_open() || get_comments_number() ) : ?>
        <div class="entry-comment">
                <?php comments_template( '/comments-content.php' ); ?>
        </div>
<?php endif; ?>

And comments-content.php has following part:

<?php
wp_list_comments( array(
        'page'     => $cpaged, // Variable defined before, default: 1
        'per_page' => 2
) );
?>
Last edited 9 years ago by Ninos Ego (previous) (diff)

#11 in reply to: ↑ 10 @boonebgorges
9 years ago

Replying to Ninos Ego:

Oh I'm sorry. Inside this loop I'm calling:

Ah great, this makes more sense.

I guess the problem here is the is_singular() check here: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/comment-template.php?marks=1931#L1926 I originally put that check in, thinking that a singular page would be the only place where get_query_var() would return meaningful values for 'cpage' and 'comments_per_page'. But I guess maybe that restriction doesn't matter, because if meaningful values are not found, then a new query will be forced in any case.

Can you verify that is_singular() is the culprit? Change the line in question to the following:

if ($r['page'] || $r['per_page'] ) {

#12 @Ninos Ego
9 years ago

Hmm now I do not get any comments if I use param page or per_page. I more think that the problem is in the $wp_query->max_num_comment_pages, because here the results of WP_Query for the maximum comment pages is not the same as in the wp_list_comments().

#13 @boonebgorges
9 years ago

Ah, but you shouldn't be hitting the max_num_comment_pages block, after [36276] #35356. Are you able to test against the latest trunk?

#14 @Ninos Ego
9 years ago

Hmm with latest trunk + your patch I still get no comments

@boonebgorges
9 years ago

#15 @boonebgorges
9 years ago

OK, I've had a chance to reproduce.

My is_singular() guess was only half right. When working on #8071, I made some assumptions about where and how comments_template() is called that obviously were not correct - namely, that it's only used in the context of single posts. That's why the is_singular() check was there. And it's why the query added in [36157] used get_queried_object_id().

So the solution is to use get_the_ID() instead, which will always be sensitive to the current $wp_query loop - whether on a singular page/post or not - and, perhaps more importantly, which more closely mirrors the query in comments_template() (which uses the $post global to get the 'post_id' for the comment query).

Ninos Ego - Could you please try 35402.diff to see if it's doing the trick for you?

#16 @Ninos Ego
9 years ago

@boonebgorges thx for explanation. It's working now :-)

#17 @boonebgorges
9 years ago

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

In 36324:

Respect custom pagination params when using wp_list_comments() in a query loop.

[36157] fixed a problem, introduced in 4.4, that caused custom pagination
parameters passed to wp_list_comments(). However, the fix introduced in that
changeset was limited to the is_singular() context, so that the bug remained
when wp_list_comments() is used within a non-singular WP_Query loop. We
fix this by removing the is_singular() check and using the more general
get_the_ID() to identify the correct post_id to use for the secondary
comment query.

Fixes #35402.

#18 @boonebgorges
9 years ago

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

Reopening for 4.4.2.

#19 @dd32
9 years ago

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

In 36360:

Comments: Respect custom pagination params when using wp_list_comments() in a query loop.

[36157] fixed a problem, introduced in 4.4, that caused custom pagination
parameters passed to wp_list_comments(). However, the fix introduced in that
changeset was limited to the is_singular() context, so that the bug remained
when wp_list_comments() is used within a non-singular WP_Query loop. We
fix this by removing the is_singular() check and using the more general
get_the_ID() to identify the correct post_id to use for the secondary
comment query.

Merges [36324] to the 4.4 branch.
Props boonebgorges.
Fixes #35402.

Note: See TracTickets for help on using tickets.