Make WordPress Core

Opened 13 years ago

Closed 10 years ago

#20319 closed defect (bug) (fixed)

$page value not being defaulted to 1 in the absence of cpage query var in get_next_comments_link()/get_previous_comments_link()

Reported by: momdad's profile MomDad Owned by: boonebgorges's profile boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.0
Component: Comments Keywords: has-patch
Focuses: template Cc:

Description

get_next_comments_link() generates incorrect comment pagination links on the first page when used alongside a user-made call to wp_list_comments().

Currently, get_next_comments_link() doesn't take into account instances where there is no 'cpage' query var (which is the case when a user-made call to wp_list_comments() is performed). The intval() of the empty $page variable returns 0, and at this point should then be defaulted to 1 in order for the correct link to page 2 to be generated. Instead, it's left as 0, and the resulting link is self-referencing to page 1.

In wp-includes/link-template.php:

function get_next_comments_link( $label = '', $max_page = 0 ) {
    global $wp_query;

    if ( !is_singular() || !get_option('page_comments') )
        return;

    $page = get_query_var('cpage'); //RETURNS EMPTY IN ABSENCE OF A cpage VALUE ON PAGE 1 OF A USER-MADE CALL TO wp_list_comments()

    $nextpage = intval($page) + 1; //intval() RETURNS 0, 0+1=1, RESULTING IN THE "Newer Comments" LINK SELF-REFERENCING PAGE 1, INSTEAD OF POINTING TO PAGE 2

    ...

Coincidentally, in paginate_comments_links() (wp-includes/link-template.php) this situation is handled correctly, and $page does get correctly defaulted to 1. The same conditional used in paginate_comments_links() can be applied to both get_next_comments_link() and get_previous_comments_link(). The code to properly account for it is:

function get_next_comments_link( $label = '', $max_page = 0 ) {
    global $wp_query;

    if ( !is_singular() || !get_option('page_comments') )
        return;

    $page = get_query_var('cpage');
    if ( !$page ) //******** ADDED
        $page = 1; //******** ADDED

    $nextpage = intval($page) + 1; //intval() RETURNS 0, 0+1=1, RESULTING IN THE "Newer Comments" LINK NOT POINTING TO PAGE 2

    ...

The same addition should be made to get_previous_comments_link() as well, although it's obviously unnoticeable on the first page.

Attachments (2)

2012-03-29-link-template.php.diff (487 bytes) - added by MomDad 13 years ago.
20319.patch (2.3 KB) - added by couturefreak 10 years ago.
Patch refresh.

Download all attachments as: .zip

Change History (10)

#1 @MomDad
13 years ago

  • Keywords has-patch added

#2 @nacin
13 years ago

  • Version changed from 3.4 to 3.0

Looks good, though not a new problem. Likely introduced way before 3.0.

#3 @rachelbaker
10 years ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to Future Release

@couturefreak
10 years ago

Patch refresh.

#4 @rachelbaker
10 years ago

  • Keywords needs-refresh removed
  • Milestone changed from Future Release to 4.2

+1 I have come across this issue as well.

Patch looks good and handles get_previous_comments_link() as well.

Moving to 4.2 milestone.

#5 @couturefreak
10 years ago

Thanks!

Patch refresh aims to fix incorrect comment pagination links that arise from lack of a 'cpage' query var.

Also includes some small cleanup.

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


10 years ago

#7 @DrewAPicture
10 years ago

  • Focuses template added

@couturefreak: Could you possibly regenerate the patch without the extra whitespacing changes?

#8 @boonebgorges
10 years ago

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

In 31617:

In get_next_comments_link(), ensure proper pagination when no 'cpage' query var is found.

The 'cpage' query var is only set when using comments_template() to display
comments. If displaying them in a context where 'cpage' is not yet set, the
default value should be 1, not 0.

Props MomDad, couturefreak.
Fixes #20319.

Note: See TracTickets for help on using tickets.