get_comment_link() should be paged comments aware

[Viper007Bond]

get_comment_link() needs to be made page-aware.

wp_list_comments() should also be made to use get_comment_link() rather than a relative URL anchor to ensure the comment permalink is correct.

I'm working on this, but figuring out what page a comment is on is a bitch. Suggestions welcome.

[Viper007Bond]

Actually, I'm going to leave this to a code ninja master. I can't handle this when threading is enabled.

My idea is we need a Walker function that will walk through all elements looking for the specified element and when it's found, it returns the current page number.

function get_comment_link($comment = null) {
	$comment = get_comment($comment);

	if ( get_option('page_comments') ) {
		/* do the stuff here */
	} else {
		return get_permalink( $comment->comment_post_ID ) . '#comment-' . $comment->comment_ID;
	}
}

[Viper007Bond]

[Fri 01:13:53] <Viper007Bond> or maybe get all top level, page those (as that's the only thing that gets paged), and then walk upwards from the given comment to find the top level parent and what page that top level parent is on

[ryan]

Maybe forget paging when linking to a comment, just link as usual. When handling a comment permalink, show the comment in context of all comments if paging is off. If paging is on show just that comment. This would probably using an actual query arg rather than a fragment, though.

[ryan]

Of course, I'd rather go with your idea if it can be made to work. Avoiding page references in comment permalinks would be nice too. Have it figure out the page based on the comment id.

[Viper007Bond]

Oh, I was thinking of including the page number in the URL.

I like the idea of a query var that redirects to the proper page though.

[Viper007Bond]

Not finished, see comments

[Viper007Bond]

Work when threading is disabled too

[Viper007Bond]

My patch (7956.2.patch) makes get_comment_link() return the post permalink with gotocom=123 tagged onto the end of it.

canonical.php needs to be updated to redirect to /post-name/comment-page-456/#comment-123.

The "456" page number can be determined by passing the gotocom value to the new function this patch introduces, get_page_of_comment(). It will calculate what page a comment will appear on and yes, it is threaded aware.

[Viper007Bond]

If paging is disabled, don't bother with the "gotocom" stuff

[ryan]

Maybe get_page_of_comment() can setup $wp_query->comments in the same way comments_template() does that way comments_template() can avoid doing a second query if it is already set.

[Viper007Bond]

Ditch all of the "gotocom" crap and just go directly to the comment's page

[Viper007Bond]

Replying to ryan:

Maybe get_page_of_comment() can setup $wp_query->comments in the same way comments_template() does that way comments_template() can avoid doing a second query if it is already set.

get_comment_link() and get_page_of_comment() can be used outside of the comments.php template. For example, a "Latest Comments" widget. The comment ID being passed to either of those functions may or may not belong to the post/page that's being displayed.

[Viper007Bond]

Don't add "nofollow" to comment link as it's just a link to another page now

[ryan]

(In [9367]) Make get_comment_link() paging aware. Props Viper007Bond. see #7956

[ryan]

This ise dying in get_page_of_comment(). Maybe an infinite loop?

[Viper007Bond]

Must be the while. That's designed to keep getting the parent comment until it finds the one that has a parent value of 0, i.e. the top level parent.

I'm not sure why it's getting stuck in a loop through for you. It works fine here.

Is there a case where a top level comment's $comment->comment_parent will not be 0?

[Viper007Bond]

Fix while() loops + don't return int if any parent is deleted/unapproved

[ryan]

Still getting a loop that exhausts memory.

Comments for the post are

* Top-level
** Second
*** Third
**** Fourth
* Top

[ryan]

(In [9377]) get_page_of_comment() fixes from Viper007Bond. see #7956

[Viper007Bond]

Properly use get_comments() so it doesn't return the whole DB table + phpdocs

[ryan]

(In [9379]) get_page_of_comment() fixes from Viper007Bond. see #7956

[Viper007Bond]

get_comment_link() is currently using using a heavy MySQL query every single time it's called (due to a call to get_comments() inside of get_page_of_comment()).

Incoming patch (which were all Mark's ideas) ditches the usage of get_comments() inside of get_page_of_comment() and instead just does a simple, lightweight query to find the number of older top-level comments. That's all that's needed as you can then divide that count by the comments per page to get what page that comment is on.

However this patch also allows you to skip even that query by passing a $page value to get_comment_link() if it's known (i.e. if you're in the regular comment loop and not in the sidebar). That way we can recycle the value and don't have to calculate the page for every single get_comment_link() call.

Oh, and the patch also updates the Recent Comments widget to use get_comment_link() (I used it for testing) and fixes a "bug" that resulted in get_comments() returning only the 4 columns that the widget queried for (due to caching).

[Viper007Bond]

Properly intval the $page, props azaozz

[ryan]

That doesn't count orphaned comments as top level though.

[ryan]

Right now orphans are stuck at the end of the comment list rather than in their chrono order. Getting the page for them could be tricky, but at least at the end their not screwing with the page number of other comments by not being counted in the query. Orphans should be rare so maybe we should forget about them and call this fix good enough.

[markjaquith]

(In [9522]) Massive get_comment_link() performance improvements for posts with a lot of comments. props Viper007Bond. fixes #7956

[Viper007Bond]

get_comment_link() is now queryless for inside of the default comment loop due to the above changeset.

However I'm still considering how to further optimize it for outside of the comment loop (i.e. recent comments widget) or for themes that use a callback parameter for wp_list_comments() but no $page parameter when they call get_comment_link().

Now that get_comments() caches, it could potentially be better than that changeset which still results in one query per call always. With get_comments(), you'd only query if you hadn't queried for that comment's parent post before.

Example of "Latest 10 Comments":

Current as of [9522]: 10 queries
With get_comments(): Between 1 (all comments on the same post) and 10 (all comments on different posts) queries

Mark pointed out that get_comments() is a heavy query and that one query is not always better than more than one query (get_page_of_comment()'s current query is fairly light).

I guess we'll need to do some trials.

[Viper007Bond]

Stupid Trac.

Example of "Latest 10 Comments":

• Current as of [9522]: 10 queries
  
• With get_comments(): Between 1 (all comments on the same post) and 10 (all comments on different posts) queries

[Viper007Bond]

RE Above stuff: it's fine as is, can be taken a further look at for future versions. 2.7.0 needs to get out the door.