Changeset 34735

Timestamp:

10/01/2015 05:12:39 AM (11 years ago)

Author:

boonebgorges

Message:

Ensure that comment permalinks reflect pagination.

After [34561], wp_list_comments() no longer passed all of a post's comments
to Walker_Comments. As a result, calls to get_comment_link() occurring
inside the comment loop had insufficient context to determine the proper
'cpage' value to use when generating comment permalinks. This, in turn, caused
comment permalinks to behave erratically.

The current changeset addresses the problem as follows:

• get_comment_link() now accepts a 'cpage' parameter. When present, 'cpage' will be used to build the comment permalink - no automatic calculation will take place.
• When called within the main loop, wp_list_comments() calculates the proper 'cpage' value for comments in the loop, and passes it down to get_comment_link().
• cpage and comment-page-x query vars are generally required in comment permalinks (see #34068), but an exception is made when 'default_comment_page=oldest': the bare post permalink will always be the same as cpage=1, so cpage is excluded in this case.

Props peterwilsoncc for assiduous spreadsheeting.
Fixes #34073.

Location:

trunk

Files:

• src/wp-includes/comment-template.php (modified) (5 diffs)
• tests/phpunit/tests/comment/commentsTemplate.php (modified) (1 diff)
• tests/phpunit/tests/comment/getCommentLink.php (added)

trunk/src/wp-includes/comment-template.php

@@ -660 +660 @@
  *
  * @since 1.5.0
- * @since 4.4.0 Added the ability for `$comment` to also accept a WP_Comment object.
+ * @since 4.4.0 Added the ability for `$comment` to also accept a WP_Comment object. Added `$cpage` argument.
  *
  * @see get_page_of_comment()
@@ -671 +671 @@
  *     An array of optional arguments to override the defaults.
  *
- *     @type string $type      Passed to {@see get_page_of_comment()}.
- *     @type int    $page      Current page of comments, for calculating comment pagination.
- *     @type int    $per_page  Per-page value for comment pagination.
- *     @type int    $max_depth Passed to {@see get_page_of_comment()}.
+ *     @type string     $type      Passed to {@see get_page_of_comment()}.
+ *     @type int        $page      Current page of comments, for calculating comment pagination.
+ *     @type int        $per_page  Per-page value for comment pagination.
+ *     @type int        $max_depth Passed to {@see get_page_of_comment()}.
+ *     @type int|string $cpage     Value to use for the comment's "comment-page" or "cpage" value. If provided, this
+ *                                 value overrides any value calculated from `$page` and `$per_page`.
  * }
  * @return string The permalink to the given comment.
@@ -688 +690 @@
         }
 
-        $defaults = array( 'type' => 'all', 'page' => '', 'per_page' => '', 'max_depth' => '' );
+        $defaults = array(
+                'type'      => 'all',
+                'page'      => '',
+                'per_page'  => '',
+                'max_depth' => '',
+                'cpage'     => null,
+        );
         $args = wp_parse_args( $args, $defaults );
 
-        if ( '' === $args['per_page'] )
-                $args['per_page'] = get_option('comments_per_page');
-
-        if ( empty($args['per_page']) ) {
-                $args['per_page'] = 0;
-                $args['page'] = 0;
-        }
-
-        if ( $args['per_page'] ) {
-                if ( '' == $args['page'] )
-                        $args['page'] = ( !empty($in_comment_loop) ) ? get_query_var('cpage') : get_page_of_comment( $comment->comment_ID, $args );
-
-                if ( $wp_rewrite->using_permalinks() )
-                        $link = user_trailingslashit( trailingslashit( get_permalink( $comment->comment_post_ID ) ) . $wp_rewrite->comments_pagination_base . '-' . $args['page'], 'comment' );
-                else
-                        $link = add_query_arg( 'cpage', $args['page'], get_permalink( $comment->comment_post_ID ) );
+        $link = get_permalink( $comment->comment_post_ID );
+
+        // The 'cpage' param takes precedence.
+        if ( ! is_null( $args['cpage'] ) ) {
+                $cpage = $args['cpage'];
+
+        // No 'cpage' is provided, so we calculate one.
         } else {
-                $link = get_permalink( $comment->comment_post_ID );
+                if ( '' === $args['per_page'] ) {
+                        $args['per_page'] = get_option('comments_per_page');
+                }
+
+                if ( empty( $args['per_page'] ) ) {
+                        $args['per_page'] = 0;
+                        $args['page'] = 0;
+                }
+
+                $cpage = $args['page'];
+
+                if ( '' == $cpage ) {
+                        if ( ! empty( $in_comment_loop ) ) {
+                                $cpage = get_query_var( 'cpage' );
+                        } else {
+                                // Requires a database hit, so we only do it when we can't figure out from context.
+                                $cpage = get_page_of_comment( $comment->comment_ID, $args );
+                        }
+                }
+
+                // Drop the 'page' var if we're on the default page.
+                $comment_post = get_post( $comment->comment_post_ID );
+                if ( $args['per_page'] ) {
+                        $total_pages = ceil( $comment_post->comment_count / $args['per_page'] );
+                } else {
+                        $total_pages = 1;
+                }
+
+                /*
+                 * If the default page displays the oldest comments, the permalinks for comments on the default page
+                 * do not need a 'cpage' query var.
+                 */
+                $default_comments_page = get_option( 'default_comments_page' );
+                if ( 'oldest' === get_option( 'default_comments_page' ) && 1 === $cpage ) {
+                        $cpage = '';
+                }
+        }
+
+        if ( $cpage ) {
+                if ( $wp_rewrite->using_permalinks() ) {
+                        if ( $cpage ) {
+                                $link = trailingslashit( $link ) . $wp_rewrite->comments_pagination_base . '-' . $cpage;
+                        }
+
+                        $link = user_trailingslashit( $link, 'comment' );
+                } elseif ( $cpage ) {
+                        $link = add_query_arg( 'cpage', $cpage, $link );
+                }
+
+        }
+
+        if ( $wp_rewrite->using_permalinks() ) {
+                $link = user_trailingslashit( $link, 'comment' );
         }
 
         $link = $link . '#comment-' . $comment->comment_ID;
+
         /**
          * Filter the returned single comment permalink.
          *
          * @since 2.8.0
+         * @since 4.4.0 Added the `$cpage` parameter.
          *
          * @see get_page_of_comment()
@@ -722 +775 @@
          * @param WP_Comment $comment The current comment object.
          * @param array      $args    An array of arguments to override the defaults.
-         */
-        return apply_filters( 'get_comment_link', $link, $comment, $args );
+         * @param int        $cpage   The calculated 'cpage' value.
+         */
+        return apply_filters( 'get_comment_link', $link, $comment, $args, $cpage );
 }
 
@@ -1878 +1932 @@
 
                 // Pagination is already handled by `WP_Comment_Query`, so we tell Walker not to bother.
-                if ( 1 < $wp_query->max_num_comment_pages ) {
-                        $r['page'] = 1;
+                if ( $wp_query->max_num_comment_pages ) {
+                        $default_comments_page = get_option( 'default_comments_page' );
+                        $cpage = get_query_var( 'cpage' );
+                        if ( 'newest' === $default_comments_page ) {
+                                $r['cpage'] = $cpage;
+
+                        // When first page shows oldest comments, post permalink is the same as the comment permalink.
+                        } elseif ( $cpage == 1 ) {
+                                $r['cpage'] = '';
+                        } else {
+                                $r['cpage'] = $cpage;
+                        }
+
+                        $r['page'] = 0;
+                        $r['per_page'] = 0;
                 }
         }

trunk/tests/phpunit/tests/comment/commentsTemplate.php

@@ -410 +410 @@
                 $this->assertSame( array( $comment_1 ), $found_cids );
         }
+
+        /**
+         * @ticket 34073
+         */
+        public function test_comment_permalinks_should_be_correct_when_using_default_display_callback_with_default_comment_page_oldest() {
+                $now = time();
+                $p = $this->factory->post->create();
+                $comment_1 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '1',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 100 ),
+                ) );
+                $comment_2 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '2',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 200 ),
+                ) );
+                $comment_3 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '3',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 300 ),
+                ) );
+                $comment_4 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '4',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 400 ),
+                ) );
+
+                update_option( 'comment_order', 'desc' );
+                update_option( 'default_comments_page', 'oldest' );
+
+                $link_p1 = add_query_arg( array(
+                        'comments_per_page' => 2,
+                ), get_permalink( $p ) );
+
+                $this->go_to( $link_p1 );
+
+                $found_p1 = get_echo( 'comments_template' );
+
+                // Find the comment permalinks.
+                preg_match_all( '|href="(.*?#comment-([0-9]+))|', $found_p1, $matches );
+
+                // This is the main post page, so we don't expect any cpage param.
+                foreach ( $matches[1] as $m ) {
+                        $this->assertNotContains( 'cpage', $m );
+                }
+
+                $link_p2 = add_query_arg( array(
+                        'cpage' => 2,
+                        'comments_per_page' => 2,
+                ), get_permalink( $p ) );
+
+                $this->go_to( $link_p2 );
+
+                $found_p2 = get_echo( 'comments_template' );
+
+                // Find the comment permalinks.
+                preg_match_all( '|href="(.*?#comment-([0-9]+))|', $found_p2, $matches );
+
+                // They should all be on page 2.
+                foreach ( $matches[1] as $m ) {
+                        $this->assertContains( 'cpage=2', $m );
+                }
+        }
+
+        /**
+         * @ticket 34073
+         */
+        public function test_comment_permalinks_should_be_correct_when_using_default_display_callback_with_default_comment_page_newest() {
+                $now = time();
+                $p = $this->factory->post->create();
+                $comment_1 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '1',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 100 ),
+                ) );
+                $comment_2 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '2',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 200 ),
+                ) );
+                $comment_3 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '3',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 300 ),
+                ) );
+                $comment_4 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '4',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 400 ),
+                ) );
+                $comment_5 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '4',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 500 ),
+                ) );
+                $comment_6 = $this->factory->comment->create( array(
+                        'comment_post_ID' => $p,
+                        'comment_content' => '4',
+                        'comment_date_gmt' => date( 'Y-m-d H:i:s', $now - 600 ),
+                ) );
+
+                update_option( 'comment_order', 'desc' );
+                update_option( 'default_comments_page', 'newest' );
+
+                $link_p0 = add_query_arg( array(
+                        'comments_per_page' => 2,
+                ), get_permalink( $p ) );
+
+                $this->go_to( $link_p0 );
+
+                $found_p0 = get_echo( 'comments_template' );
+
+                // Find the comment permalinks.
+                preg_match_all( '|href="(.*?#comment-([0-9]+))|', $found_p0, $matches );
+
+                foreach ( $matches[1] as $m ) {
+                        $this->assertContains( 'cpage=3', $m );
+                }
+
+                $link_p2 = add_query_arg( array(
+                        'cpage' => 2,
+                        'comments_per_page' => 2,
+                ), get_permalink( $p ) );
+
+                $this->go_to( $link_p2 );
+
+                $found_p2 = get_echo( 'comments_template' );
+
+                // Find the comment permalinks.
+                preg_match_all( '|href="(.*?#comment-([0-9]+))|', $found_p2, $matches );
+
+                // They should all be on page 2.
+                foreach ( $matches[1] as $m ) {
+                        $this->assertContains( 'cpage=2', $m );
+                }
+
+                // p1 is the last page (neat!).
+                $link_p1 = add_query_arg( array(
+                        'cpage' => 1,
+                        'comments_per_page' => 2,
+                ), get_permalink( $p ) );
+
+                $this->go_to( $link_p1 );
+
+                $found_p1 = get_echo( 'comments_template' );
+
+                // Find the comment permalinks.
+                preg_match_all( '|href="(.*?#comment-([0-9]+))|', $found_p1, $matches );
+
+                // They should all be on page 2.
+                foreach ( $matches[1] as $m ) {
+                        $this->assertContains( 'cpage=1', $m );
+                }
+        }
 }