WordPress.org
Get WordPress

Make WordPress Core

Changeset 34561


Ignore:
Timestamp:
09/25/2015 08:39:18 PM (11 years ago)
Author:
boonebgorges
Message:

Force comment pagination on single posts.

Previously, the 'page_comments' toggle allowed users to disable comment
pagination. This toggle was only superficial, however. Even with
'page_comments' turned on, comments_template() loaded all of a post's
comments into memory, and passed them to wp_list_comments() and
Walker_Comment, the latter of which produced markup for only the
current page of comments. In other words, it was possible to enable
'page_comments', thereby showing only a subset of a post's comments on a given
page, but all comments continued to be loaded in the background. This technique
scaled poorly. Posts with hundreds or thousands of comments would load slowly,
or not at all, even when the 'comments_per_page' setting was set to a
reasonable number.

Recent changesets have addressed this problem through more efficient tree-
walking, better descendant caching, and more selective queries for top-level
post comments. The current changeset completes the project by addressing the
root issue: that loading a post causes all of its comments to be loaded too.

Here's the breakdown:

  • Comment pagination is now forced. Setting 'page_comments' to false leads to evil things when you have many comments. If you want to avoid pagination, set 'comments_per_page' to something high.
  • The 'page_comments' setting has been expunged from options-discussion.php, and from places in the codebase where it was referenced. For plugins relying on 'page_comments', we now force the value to true with a pre_option filter.
  • comments_template() now queries for an appropriately small number of comments. Usually, this means the comments_per_page value.
  • To preserve the current (odd) behavior for comment pagination links, some unholy hacks have been inserted into comments_template(). The ugliness is insulated in this function for backward compatibility and to minimize collateral damage. A side-effect is that, for certain settings of 'default_comments_page', up to 2x the value of comments_per_page might be fetched at a time.
  • In support of these changes, a $format parameter has been added to WP_Comment::get_children(). This param allows you to request a flattened array of comment children, suitable for feeding into Walker_Comment.
  • WP_Query loops are now informed about total available comment counts and comment pages by the WP_Comment_Query (found_comments, max_num_pages), instead of by Walker_Comment.

Aside from radical performance improvements in the case of a post with many
comments, this changeset fixes a bug that caused the first page of comments to
be partial (found_comments % comments_per_page), rather than the last, as
you'd expect.

Props boonebgorges, wonderboymusic.
Fixes #8071.

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-admin/includes/schema.php

    r34529 r34561  
    483483        'thread_comments' => 1,
    484484        'thread_comments_depth' => 5,
    485         'page_comments' => 0,
     485        'page_comments' => 1,
    486486        'comments_per_page' => 50,
    487487        'default_comments_page' => 'newest',

  • trunk/src/wp-admin/options-discussion.php

    r34022 r34561  
    9999?></label>
    100100<br />
    101 <label for="page_comments">
    102 <input name="page_comments" type="checkbox" id="page_comments" value="1" <?php checked('1', get_option('page_comments')); ?> />
    103 <?php
    104 
     101<?php
    105102$default_comments_page = '</label><label for="default_comments_page"><select name="default_comments_page" id="default_comments_page"><option value="newest"';
    106103if ( 'newest' == get_option('default_comments_page') ) $default_comments_page .= ' selected="selected"';
    … …  
    109106$default_comments_page .= '>' . __('first') . '</option></select>';
    110107
    111 printf( __('Break comments into pages with %1$s top level comments per page and the %2$s page displayed by default'), '</label><label for="comments_per_page"><input name="comments_per_page" type="number" step="1" min="0" id="comments_per_page" value="' . esc_attr(get_option('comments_per_page')) . '" class="small-text" />', $default_comments_page );
     108printf( __('Break comments into pages with %1$s top level comments per page and the %2$s page displayed by default'), '<label for="comments_per_page"><input name="comments_per_page" type="number" step="1" min="0" id="comments_per_page" value="' . esc_attr(get_option('comments_per_page')) . '" class="small-text" />', $default_comments_page );
    112109
    113110?></label>

  • trunk/src/wp-admin/options.php

    r34315 r34561  
    8484$whitelist_options = array(
    8585        'general' => array( 'blogname', 'blogdescription', 'gmt_offset', 'date_format', 'time_format', 'start_of_week', 'timezone_string', 'WPLANG' ),
    86         'discussion' => array( 'default_pingback_flag', 'default_ping_status', 'default_comment_status', 'comments_notify', 'moderation_notify', 'comment_moderation', 'require_name_email', 'comment_whitelist', 'comment_max_links', 'moderation_keys', 'blacklist_keys', 'show_avatars', 'avatar_rating', 'avatar_default', 'close_comments_for_old_posts', 'close_comments_days_old', 'thread_comments', 'thread_comments_depth', 'page_comments', 'comments_per_page', 'default_comments_page', 'comment_order', 'comment_registration' ),
     86        'discussion' => array( 'default_pingback_flag', 'default_ping_status', 'default_comment_status', 'comments_notify', 'moderation_notify', 'comment_moderation', 'require_name_email', 'comment_whitelist', 'comment_max_links', 'moderation_keys', 'blacklist_keys', 'show_avatars', 'avatar_rating', 'avatar_default', 'close_comments_for_old_posts', 'close_comments_days_old', 'thread_comments', 'thread_comments_depth', 'comments_per_page', 'default_comments_page', 'comment_order', 'comment_registration' ),
    8787        'media' => array( 'thumbnail_size_w', 'thumbnail_size_h', 'thumbnail_crop', 'medium_size_w', 'medium_size_h', 'large_size_w', 'large_size_h', 'image_default_size', 'image_default_align', 'image_default_link_type' ),
    8888        'reading' => array( 'posts_per_page', 'posts_per_rss', 'rss_use_excerpt', 'show_on_front', 'page_on_front', 'page_for_posts', 'blog_public' ),

  • trunk/src/wp-includes/canonical.php

    r34492 r34561  
    322322                        }
    323323
    324                         if ( get_option('page_comments') && ( ( 'newest' == get_option('default_comments_page') && get_query_var('cpage') > 0 ) || ( 'newest' != get_option('default_comments_page') && get_query_var('cpage') > 1 ) ) ) {
     324                        if ( ( 'newest' == get_option('default_comments_page') && get_query_var('cpage') > 0 ) || ( 'newest' != get_option('default_comments_page') && get_query_var('cpage') > 1 ) ) {
    325325                                $addl_path = ( !empty( $addl_path ) ? trailingslashit($addl_path) : '' ) . user_trailingslashit( $wp_rewrite->comments_pagination_base . '-' . get_query_var('cpage'), 'commentpaged' );
    326326                                $redirect['query'] = remove_query_arg( 'cpage', $redirect['query'] );

  • trunk/src/wp-includes/class-wp-comment-query.php

    r34549 r34561  
    866866                do {
    867867                        $parent_ids = $levels[ $level ];
     868                        if ( ! $parent_ids ) {
     869                                break;
     870                        }
     871
    868872                        $where = 'WHERE ' . implode( ' AND ', $where_clauses ) . ' AND comment_parent IN (' . implode( ',', array_map( 'intval', $parent_ids ) ) . ')';
    869873                        $comment_ids = $wpdb->get_col( "{$this->sql_clauses['select']} {$this->sql_clauses['from']} {$where} {$this->sql_clauses['groupby']}" );

  • trunk/src/wp-includes/class-wp-comment.php

    r34546 r34561  
    228228         * @access public
    229229         *
     230         * @param string $format Return value format. 'tree' for a hierarchical tree, 'flat' for a flattened array.
     231         *                       Default 'tree'.
    230232         * @return array Array of `WP_Comment` objects.
    231233         */
    232         public function get_children() {
     234        public function get_children( $format = 'tree' ) {
    233235                if ( is_null( $this->children ) ) {
    234236                        $this->children = get_comments( array(
    … …  
    238240                }
    239241
    240                 return $this->children;
     242                if ( 'flat' === $format ) {
     243                        $children = array();
     244                        foreach ( $this->children as $child ) {
     245                                $children = array_merge( $children, array( $child ), $child->get_children( 'flat' ) );
     246                        }
     247                } else {
     248                        $children = $this->children;
     249                }
     250
     251                return $children;
    241252        }
    242253

  • trunk/src/wp-includes/comment-functions.php

    r34545 r34561  
    808808                return 0;
    809809
    810         if ( ! get_option( 'page_comments' ) )
    811                 return 1;
    812 
    813810        if ( !isset($per_page) )
    814811                $per_page = (int) get_query_var('comments_per_page');
    … …  
    859856        $args = wp_parse_args( $args, $defaults );
    860857
    861         if ( '' === $args['per_page'] && get_option('page_comments') )
     858        if ( '' === $args['per_page'] )
    862859                $args['per_page'] = get_query_var('comments_per_page');
    863860        if ( empty($args['per_page']) ) {

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

    r34525 r34561  
    684684        $args = wp_parse_args( $args, $defaults );
    685685
    686         if ( '' === $args['per_page'] && get_option('page_comments') )
     686        if ( '' === $args['per_page'] )
    687687                $args['per_page'] = get_option('comments_per_page');
    688688
    … …  
    12151215
    12161216        $comment_args = array(
    1217                 'order'   => 'ASC',
    12181217                'orderby' => 'comment_date_gmt',
    12191218                'status'  => 'approve',
    12201219                'post_id' => $post->ID,
     1220                'hierarchical' => 'threaded',
     1221                'no_found_rows' => false,
    12211222                'update_comment_meta_cache' => false, // We lazy-load comment meta for performance.
    12221223        );
    … …  
    12281229        }
    12291230
    1230         $comments = get_comments( $comment_args );
     1231        $per_page = (int) get_query_var( 'comments_per_page' );
     1232        if ( 0 === $per_page ) {
     1233                $per_page = (int) get_option( 'comments_per_page' );
     1234        }
     1235
     1236        $flip_comment_order = $trim_comments_on_page = false;
     1237        if ( $post->comment_count > $per_page ) {
     1238                $comment_args['number'] = $per_page;
     1239
     1240                /*
     1241                 * For legacy reasons, higher page numbers always mean more recent comments, regardless of sort order.
     1242                 * Since we don't have full pagination info until after the query, we use some tricks to get the
     1243                 * right comments for the current page.
     1244                 *
     1245                 * Abandon all hope, ye who enter here!
     1246                 */
     1247                $page = (int) get_query_var( 'cpage' );
     1248                if ( 'newest' === get_option( 'default_comments_page' ) ) {
     1249                        if ( $page ) {
     1250                                $comment_args['order'] = 'ASC';
     1251
     1252                                /*
     1253                                 * We don't have enough data (namely, the total number of comments) to calculate an
     1254                                 * exact offset. We'll fetch too many comments, and trim them as needed
     1255                                 * after the query.
     1256                                 */
     1257                                $offset = ( $page - 2 ) * $per_page;
     1258                                if ( 0 > $offset ) {
     1259                                        // `WP_Comment_Query` doesn't support negative offsets.
     1260                                        $comment_args['offset'] = 0;
     1261                                } else {
     1262                                        $comment_args['offset'] = $offset;
     1263                                }
     1264
     1265                                // Fetch double the number of comments we need.
     1266                                $comment_args['number'] += $per_page;
     1267                                $trim_comments_on_page = true;
     1268                        } else {
     1269                                $comment_args['order'] = 'DESC';
     1270                                $comment_args['offset'] = 0;
     1271                                $flip_comment_order = true;
     1272                        }
     1273                } else {
     1274                        $comment_args['order'] = 'ASC';
     1275                        if ( $page ) {
     1276                                $comment_args['offset'] = ( $page - 1 ) * $per_page;
     1277                        } else {
     1278                                $comment_args['offset'] = 0;
     1279                        }
     1280                }
     1281        }
     1282
     1283        $comment_query = new WP_Comment_Query( $comment_args );
     1284        $_comments = $comment_query->comments;
     1285
     1286        // Delightful pagination quirk #1: first page of results sometimes needs reordering.
     1287        if ( $flip_comment_order ) {
     1288                $_comments = array_reverse( $_comments );
     1289        }
     1290
     1291        // Delightful pagination quirk #2: reverse chronological order requires page shifting.
     1292        if ( $trim_comments_on_page ) {
     1293                // Correct the value of max_num_pages, which is wrong because we manipulated the per_page 'number'.
     1294                $comment_query->max_num_pages = ceil( $comment_query->found_comments / $per_page );
     1295
     1296                // Identify the number of comments that should appear on page 1.
     1297                $page_1_count = $comment_query->found_comments - ( ( $comment_query->max_num_pages - 1 ) * $per_page );
     1298
     1299                // Use that value to shift the matched comments.
     1300                if ( 1 === $page ) {
     1301                        $_comments = array_slice( $_comments, 0, $page_1_count );
     1302                } else {
     1303                        $_comments = array_slice( $_comments, $page_1_count, $per_page );
     1304                }
     1305        }
     1306
     1307        // Trees must be flattened before they're passed to the walker.
     1308        $comments_flat = array();
     1309        foreach ( $_comments as $_comment ) {
     1310                $comments_flat = array_merge( $comments_flat, array( $_comment ), $_comment->get_children( 'flat' ) );
     1311        }
    12311312
    12321313        /**
    … …  
    12381319         * @param int   $post_ID  Post ID.
    12391320         */
    1240         $wp_query->comments = apply_filters( 'comments_array', $comments, $post->ID );
     1321        $wp_query->comments = apply_filters( 'comments_array', $comments_flat, $post->ID );
    12411322        $comments = &$wp_query->comments;
    12421323        $wp_query->comment_count = count($wp_query->comments);
     1324        $wp_query->max_num_comment_pages = $comment_query->max_num_pages;
    12431325
    12441326        if ( $separate_comments ) {
    … …  
    12501332
    12511333        $overridden_cpage = false;
    1252         if ( '' == get_query_var('cpage') && get_option('page_comments') ) {
     1334        if ( '' == get_query_var('cpage') ) {
    12531335                set_query_var( 'cpage', 'newest' == get_option('default_comments_page') ? get_comment_pages_count() : 1 );
    12541336                $overridden_cpage = true;
    … …  
    18261908                        $_comments = $wp_query->comments;
    18271909                }
    1828         }
    1829 
    1830         if ( '' === $r['per_page'] && get_option('page_comments') )
     1910
     1911                // Pagination is already handled by `WP_Comment_Query`, so we tell Walker not to bother.
     1912                if ( 1 < $wp_query->max_num_comment_pages ) {
     1913                        $r['page'] = 1;
     1914                }
     1915        }
     1916
     1917        if ( '' === $r['per_page'] )
    18311918                $r['per_page'] = get_query_var('comments_per_page');
    18321919
    … …  
    18671954
    18681955        $output = $walker->paged_walk( $_comments, $r['max_depth'], $r['page'], $r['per_page'], $r );
    1869         $wp_query->max_num_comment_pages = $walker->max_pages;
    18701956
    18711957        $in_comment_loop = false;

  • trunk/src/wp-includes/default-filters.php

    r34529 r34561  
    317317add_filter( 'default_option_embed_autourls', '__return_true' );
    318318
     319// This option no longer exists; tell plugins we want comment pagination.
     320add_filter( 'pre_option_page_comments', '__return_true' );
     321
    319322// Default settings for heartbeat
    320323add_filter( 'heartbeat_settings', 'wp_heartbeat_settings' );

  • trunk/src/wp-includes/link-template.php

    r34528 r34561  
    25912591        global $wp_query;
    25922592
    2593         if ( !is_singular() || !get_option('page_comments') )
     2593        if ( ! is_singular() )
    25942594                return;
    25952595
    … …  
    26452645 */
    26462646function get_previous_comments_link( $label = '' ) {
    2647         if ( !is_singular() || !get_option('page_comments') )
     2647        if ( ! is_singular() )
    26482648                return;
    26492649
    … …  
    26932693        global $wp_rewrite;
    26942694
    2695         if ( !is_singular() || !get_option('page_comments') )
     2695        if ( ! is_singular() )
    26962696                return;
    26972697
    … …  
    27382738
    27392739        // Are there comments to navigate through?
    2740         if ( get_comment_pages_count() > 1 && get_option( 'page_comments' ) ) {
     2740        if ( get_comment_pages_count() > 1 ) {
    27412741                $args = wp_parse_args( $args, array(
    27422742                        'prev_text'          => __( 'Older comments' ),

  • trunk/tests/phpunit/tests/comment/getCommentsPagesCount.php

    r31623 r34561  
    6969                $this->assertEquals( 2, get_comment_pages_count( $comments, 10, true ) );
    7070                $this->assertEquals( 4, get_comment_pages_count( $comments, 4, true ) );
    71         }
    72 
    73         /**
    74          * Validate get_comments_pages_count for option page_comments
    75          */
    76         function test_option_page_comments() {
    77                 //setup post and comments
    78                 $post = $this->factory->post->create_and_get( array( 'post_title' => 'comment--post', 'post_type' => 'post' ) );
    79                 $comments = $this->factory->comment->create_post_comments( $post->ID, 15 );
    80 
    81                 // comment paging disabled
    82                 update_option( 'page_comments', false );
    83 
    84                 $this->assertEquals( 1, get_comment_pages_count( $comments, 10, false ) );
    85                 $this->assertEquals( 1, get_comment_pages_count( $comments, 0, true ) );
    86                 $this->assertEquals( 1, get_comment_pages_count( $comments, 2, true ) );
    87 
    88                 // comment paging enabled
    89                 update_option( 'page_comments', true );
    90 
    91                 $this->assertEquals( 2, get_comment_pages_count( $comments, 10, false ) );
    92                 $this->assertEquals( 3, get_comment_pages_count( $comments, 5, false ) );
    9371        }
    9472

  • trunk/tests/phpunit/tests/link/getNextCommentsLink.php

    r31617 r34561  
    1717
    1818        public function test_page_should_respect_value_of_cpage_query_var() {
    19                 update_option( 'page_comments', '1' );
    2019                $p = $this->factory->post->create();
    2120                $this->go_to( get_permalink( $p ) );
    … …  
    3534         */
    3635        public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() {
    37                 update_option( 'page_comments', '1' );
    3836                $p = $this->factory->post->create();
    3937                $this->go_to( get_permalink( $p ) );

  • trunk/tests/phpunit/tests/link/getPreviousCommentsLink.php

    r31617 r34561  
    1717
    1818        public function test_page_should_respect_value_of_cpage_query_var() {
    19                 update_option( 'page_comments', '1' );
    2019                $p = $this->factory->post->create();
    2120                $this->go_to( get_permalink( $p ) );
    … …  
    3231
    3332        public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() {
    34                 update_option( 'page_comments', '1' );
    3533                $p = $this->factory->post->create();
    3634                $this->go_to( get_permalink( $p ) );
Note: See TracChangeset for help on using the changeset viewer.