Make WordPress Core

Opened 13 years ago

Closed 7 years ago

Last modified 7 years ago

#18603 closed defect (bug) (invalid)

Comments on pages which exceed paginate settings create erroneous permalinks

Reported by: msagman's profile msagman Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.2.1
Component: Comments Keywords: needs-patch needs-testing good-first-bug
Focuses: Cc:

Description

On my website, clicking on any of the Recent Comments that possess large numbers of comments exceeding paginate settings (50 comments) present erroneous permalinks. Only the posts containing enough comments on page 3 or higher present these improperly constructed links that take users to the wrong page (a page that does not contain the comment in question). The problem does not occur on pages containing a limited number of comments. So far, we have removed our Thesis theme and changed to the standard WP default theme. And we've deactivated all plugins. Yet neither solution appears to resolve the issue. Thanks for your consideration. You may view this issue live at http://www.dogfoodadvisor.com

Attachments (2)

wp_comments_trac_18603.sql (8.8 KB) - added by ambrosiawt 8 years ago.
Dump of comments table that can be used to demonstrate the problem.
18603-partial.diff (444 bytes) - added by ambrosiawt 8 years ago.
Only a partial patch for discussion. Not for live use.

Download all attachments as: .zip

Change History (27)

#1 @SergeyBiryukov
13 years ago

So get_page_of_comment() returns wrong number?

Could not reproduce this on a clean install with 5 pages of comments. Looks more like a support request to me.

#2 @solarissmoke
13 years ago

  • Keywords reporter-feedback close added

From your description it sounds like the issue is with the "Recent Comments" sidebar widget, in which case you need to talk to the author of that plugin. If the issue can be reproduced in a clean installation of Wordpress, without any plugins installed, please give the steps to reproduce.

#3 @solarissmoke
13 years ago

Oops, just realised Recent Comments is part of core, not a plugin. In any case, I can't reproduce either.

#4 @msagman
13 years ago

Thanks for looking. I've left a thread on the support forum. May I ask, is it possible some other file may have been modified by a plugin and have been corrupted (for example, htaccess) that could be causing this? Have absolutely no idea where to look. Thanks.

#5 @SergeyBiryukov
13 years ago

If it was .htaccess, the links would at least look correct when moving cursor over them.

One of the problem links contains comment-page-14/#comment-37218, whereas the proper link for that comment is comment-page-17/#comment-37218. So I suspect get_page_of_comment() returns a wrong number for some reason.

#6 @billerickson
13 years ago

So could this be caused by #8973?

I've been trying to help Mike with this issue. To give you a little more info on it:

  • I cannot reproduce it on a clean WordPress installation
  • When we disable all plugins and switch to default theme, the issue still persists

#7 @msagman
13 years ago

I think we've found the problem. And it does appear to be located in the core function of WP in the get_page_of_comment() lines. You should be able to reproduce the problem. As I understand it, it has to do with the logic of the way WP computes the page number. It appears to have something to do with whether the comments are approved or not. So, it only affects the posts containing the largest number of comments and the longest history.

Here's a copy of what my software technician reported to me. And after testing his changes, the problem disappeared and was fixed:

In /wp-includes/comment.php on line 813:
$oldercoms = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(comment_ID) FROM $wpdb->comments WHERE comment_post_ID = %d AND comment_parent = 0 AND comment_approved = '1' AND comment_date_gmt < '%s'" . $comtypewhere, $comment->comment_post_ID, $comment->comment_date_gmt ) );

In order to determine the correct page number for the most recent comment link you must not include approved comments and parent comments in the query… After changing the query above to the following I was able to determine the correct number of $oldercoms variable, thus affecting the math on line 820, by executing:

$oldercoms = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(comment_ID) FROM $wpdb->comments WHERE comment_post_ID = %d AND comment_date_gmt < '%s'" . $comtypewhere, $comment->comment_post_ID, $comment->comment_date_gmt ) );

Finally, I changed the math logic to:
return round( ( $oldercoms + 1 ) / $argsper_page? );

Thanks for your help.

#8 @msagman
13 years ago

  • Keywords reporter-feedback close removed

#9 @msagman
13 years ago

Please forgive my lack of understanding of how the review process works, but are you awaiting a response from me? Do I need to do something else? Have not seen any further responses or entries since my last comment. Is this bug still under review? I removed the two tags. Should I have not done that? Please be sure to see my last entry which shows you how to reproduce this same issue in a clean install. You must have sufficient comments on a number of comment pages. WP uses the wrong logic by including both approved and parent comments in the query. Please notice the get_page_of_comment() function is not set up properly. Thanks.

#10 @SergeyBiryukov
13 years ago

Thank you for the report, now it makes sense.

I'll look closer into this and #8973.

#11 @msagman
12 years ago

The problem I describe in this bug report is NOT fixed with the above code I previously submitted. I have figured out what you need to do to reproduce the issue. The problem of defective pagination only occurs when there has been more threaded responses than the number of comments per page set in the paginate feature. Even if WP is set for no threaded responses.

Today in my Wordpress research my technician concluded that the comment page calculation was thrown off when the comment's parent column was set. We tested this together and found that Wordpress sets the comment parent when replying to user's comments as an admin regardless of the threading setting under "Discussions" settings.

To fix this we ran the following SQL query on the comments table:

UPDATE wp_comments SET comment_parent = 0 WHERE comment_parent > 0

#12 @billerickson
12 years ago

  • Keywords dev-feedback added

I think I'm having the same issue on my more popular posts as well. On this post ( http://www.billerickson.net/wordpress-metaboxes ) I have 128 comments, 50 of which are top level comments. In Settings > Discussion I have it set to 50 top level comments per page.

But when I view the comments from the backend and look at their URL (or if I were using the Recent Comments widget), they link to the second page where the comment isn't.

The second to last comment (logesh) has this URL: http://www.billerickson.net/wordpress-metaboxes/comment-page-2/#comment-5608
The last comment (billerickson) has this URL: http://www.billerickson.net/wordpress-metaboxes/comment-page-2/#comment-5623

#13 @ericlewis
12 years ago

This sounds a duplicate of #8973, which @billerickson suggested.

The issue can be replicated by making multiple comments on a post until the comments pagination # is hit, after which the new comment URL will still be returned as comment-page-1. Unless there's a different bug reproduction process which hasn't been realized and detailed yet.

#14 follow-up: @SergeyBiryukov
12 years ago

  • Keywords needs-testing added

Not sure if this is an exact duplicate of #8973. That ticket fixes the issue with including current user's unmoderated comments, while this one has to do with comment threading (I didn't have any comments on the reporter's site and still noticed a problem mentioned in comment:5).

#15 in reply to: ↑ 14 @ericlewis
12 years ago

  • Keywords needs-steps-to-reproduce added

Replying to SergeyBiryukov:

Not sure if this is an exact duplicate of #8973. That ticket fixes the issue with including current user's unmoderated comments, while this one has to do with comment threading (I didn't have any comments on the reporter's site and still noticed a problem mentioned in comment:5).

Yep, you're right.

Could reproduce this after tinkering with some comments. Steps-to-reproduce would help.

Version 0, edited 12 years ago by ericlewis (next)

#16 @jaredatch
11 years ago

  • Cc jared@… added

#17 @jaredatch
11 years ago

Any update on this?

What do we need for a fix? I have a client with this issue so I can do testing/provide feedback if needed.

Basically if someone has pagination and thousands of comments, the Recent Comments widget is worthless as it is now.

#18 @wonderboymusic
10 years ago

  • Keywords needs-patch added; dev-feedback needs-testing needs-steps-to-reproduce removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner Mike Sagman http://www.dogfoodadvisor.com deleted
  • Status changed from new to assigned

#19 @chriscct7
8 years ago

  • Keywords needs-testing good-first-bug added

Need steps to replicate

@ambrosiawt
8 years ago

Dump of comments table that can be used to demonstrate the problem.

#20 follow-up: @ambrosiawt
8 years ago

Steps to replicate. During Contributor Day in Philadelphia today, I spent some time on this ticket. I was able to recreate what I think the situation is, although I'm recreating as a situation where nested comments were on, and then later turned off. I think there is still an aspect of this that involves a situation where nested comments were off to begin with. I will continue looking at that. I started looking at a possible solution, and I think I have part of the issue identified, but not all of it.

To recreate issue:

Clean WordPress install
Set "Enable threaded (nested) comments" to ON
Created 11 new top-level comments (12 total on Hello World)
Created 9 2nd level comments on the first top-level comment I created
Created 1 2nd level comment on a different top-level comment I created
Created 1 2nd level comment on the last top-level comment I created
Changed "Break comments into pages with" to 6 entries
Set "Enable threaded (nested) comment"s to OFF


Result:

The link to the very last 2nd level comment is wrong... says page 2. If you click on it, you don't actually see the comment on the page.
If you click on the post, the comment shows on the page just fine.
The link on the comment itself is correct (page-2/comments-23), but the link in Recent Comments says page 3 (which is incorrect at this point.
Since Nested is off, seems like there should really be 4 total pages of comments, not 2, in this case.

I attached a dump (https://core.trac.wordpress.org/attachment/ticket/18603/wp_comments_trac_18603.sql) of the comments table which you can, on a fresh install, drop the existing table and then import this one and then change "Break comments into pages with..." to 6, and turn nested comments off.

@ambrosiawt
8 years ago

Only a partial patch for discussion. Not for live use.

#21 @ambrosiawt
8 years ago

I uploaded a patch to demonstrate what I thought would solve at least part of the problem, but I've changed my mind about this... I was thinking that it looked pretty simple and the purpose of what I was removing was not necessary due to code before it. Now I think that it's not so simple. :)

I do want to explain where my head is at, however, since I was after a specific purpose here. The situation that we're having here is certainly that in some cases we can have values in the parent column, for example if you have threaded comments on, and after a while decide to turn them off. When the setting is changed, none of the rows with a value in the parent column are changed, just the behavior of how the comments are treated (all threaded items become top level).

I don't think it makes sense to change those rows. For example, if someone turns threading off, and then realized they didn't mean to make that change, turning it back on should retain the original threading data, which means the parent column in those rows needs to be retained. If others think differently, please comment.

That said, it seems like for this piece of code we could just drop the parent=0 from the query, but it doesn't solve the whole problem and seems to be problematic still. I may try another patch shortly.

#22 @dossy
8 years ago

I'm hoping to fix this bug as part of the work I'm doing on #8973, as until that issue is addressed and fixed, this issue will always be broken under specific conditions.

Once I get #8973 fully fixed, then I'll look more closely at the STR for this issue - thanks for confirming that the issue still exists.

#23 @lukecavanagh
7 years ago

Looks like the current version of comment.php in core uses.

https://github.com/WordPress/WordPress/blob/master/wp-includes/comment.php#L1026-L1037

Since $oldercoms is replaced with $older_comment_count.

#24 in reply to: ↑ 20 @FolioVision
7 years ago

  • Resolution set to invalid
  • Status changed from assigned to closed

Replying to ambrosiawt:

Steps to replicate.

I tried these steps but I wasn't able to reproduce the issue.

The link on the comment itself is correct (page-2/comments-23), but the link in Recent Comments says page 3 (which is incorrect at this point.

The link which I got for the last comment was: http://src.wordpress-develop.dev/?p=1&cpage=2#comment-23

I also checked how it appeared in the Recent Comments widget and it was correct as well.

The comment 22 was linked to the page 1 though: http://src.wordpress-develop.dev/?p=1&cpage=1#comment-22 But it's actually correct - the reason for that it's a reply for an older comments which shows up on page 1: http://src.wordpress-develop.dev/?p=1&cpage=1#comment-13

So I think this is fixed and the ticket should be closed.

#25 @netweb
7 years ago

  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.