Make WordPress Core

Opened 9 years ago

Last modified 12 months ago

#8973 reviewing defect (bug)

Function get_page_of_comment() returns incorrect page numbers

Reported by: GregMulhauser Owned by: chriscct7
Milestone: Future Release Priority: normal
Severity: normal Version: 2.7
Component: Comments Keywords: dev-feedback needs-refresh
Focuses: Cc:


The function get_page_of_comment(), defined in /wp-includes/comment.php for WordPress 2.7, returns wildly incorrect results in cases where the database includes spam trackbacks or pingbacks. The reason is that the DB query on line 595 does not include "comment_approved = 1" and therefore incorrectly counts unapproved trackback and pingback spam (not to mention non-spam comments in moderation). As far as I can tell from browsing my DB, although spam comments have their comment_type changed to 'spam' in the database, trackbacks and pingbacks still retain their original comment_type.

As a newbie to the WordPress Trac, I'm afraid I don't know how to provide a diff, but the full correct query is as follows:

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

The upshot is that without the above fix, comment URLs in several places (e.g., RSS feeds) may point to non-existent pages.

All the best,

Attachments (3)

8973.diff (2.2 KB) - added by solarissmoke 7 years ago.
Include current user's unmoderated comments when finding comment page
8973-1.diff (3.3 KB) - added by dossy 2 years ago.
Test and updated patch to implement this for 4.4-4.5
attachments-ticket-34391.zip (86.7 KB) - added by renggo888 16 months ago.

Download all attachments as: .zip

Change History (26)

#1 @GregMulhauser
9 years ago

  • Component changed from General to Comments
  • Owner anonymous deleted

#2 @DD32
9 years ago

see also: #8946: Redirection after commenting leads to an empty page [not confirmed] (Which i suspect is due to this)

#3 @ryan
9 years ago

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

(In [10449]) Include only approved comments when determining page of comment. Props GregMulhauser. fixes #8973 for trunk

#4 @ryan
9 years ago

(In [10450]) Include only approved comments when determining page of comment. Props GregMulhauser. fixes #8973 for 2.7

#5 @mrmist
9 years ago

  • Resolution fixed deleted
  • Severity changed from major to normal
  • Status changed from closed to reopened

Unfortunately I do not think that this is fixed.

Or rather, one issue is fixed but another is created.

If someone has a comment awaiting moderation, and they add a further comment, which happens to be a page-turning comment, then the refreshed page after submitting will

a) not be on the new page and b) not link to the new page.

In other words, the new comment does not immediately appear as awaiting moderation. Once the article page is (manually) revisited, the new comment is visable (awaiting mod.)

Possibly you may find this behaviour less of an issue than whatever the other issue was.

#6 @ryan
9 years ago

  • Milestone changed from 2.7.1 to 2.8

We'll probably have to use queries similar to those in comment_template(). I'd rather leave that for 2.8 and go with comment_approved = 1 for 2.7.1 as it seems the lesser of the two problems.

#7 @GregMulhauser
9 years ago

I think ryan and mrmist are both right.

Related to this (not sure whether this merits a separate submission?), I think it would be helpful if those functions which call on get_page_of_comments could get an indication back as to whether a value of 1 indicates the same page of comments as that included at the post's permalink or whether it indicates a completely different page of comments (as will happen when there is more than one page, and comments are displayed in reverse order).

As the function operates at the moment, folks with reverse-ordered comments wind up with a lot of extraneous "comment-page-1" URLs for individual comments when they could just have links to the main post instead.

All the best,

#8 @Denis-de-Bernardy
9 years ago

  • Keywords needs-patch added; comments paging removed

itching to punt this to future, from lack of traction...

#9 @hakre
9 years ago

  • Summary changed from Function get_page_of_comment() returns incorrect page numbers -- fix included to Function get_page_of_comment() returns incorrect page numbers

#10 @Denis-de-Bernardy
9 years ago

  • Milestone changed from 2.8 to Future Release

#11 @Denis-de-Bernardy
9 years ago

  • Milestone changed from Future Release to 2.9

#12 @hakre
9 years ago

Looks like thise foremost is a documentation problem with the get_page_of_comment() function. For what I see are arguments already supported (comments, trackback, and pingsbacks).

The documentation says nothing about wether or not comments must be approved. It does not say wether or not the function should be used on the blogs front or within the backend.

#13 @azaozz
8 years ago

  • Milestone changed from 2.9 to Future Release

No patch.

#14 @solarissmoke
7 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

This patch uses logic similar to comments_template(). There are some assumptions here regarding what this information is going to be used for - i.e., to get the right page number for *the current user* (who can he his/her own awaiting-moderation comments). If, for example, a plugin tries to use it for something else, then weirdness can be expected.

7 years ago

Include current user's unmoderated comments when finding comment page

#15 @billerickson
7 years ago

Related: #18603 - get_page_of_comment() adds approved comments and parent comments, which results in an inaccurate count of comments and page number.

#16 @ericlewis
6 years ago

8973.diff solves the issue for me, makes sense.

For reproduction of bug:

  1. Lower comment pagination break from 50 to 3 in Settings > Discussion.
  2. With a user account that requires moderation of their comments, add 4 comments, and don't moderate.
  3. On the 4th comment, you should be sent to comment-page-1, where your comment does not exist.

#17 @SergeyBiryukov
6 years ago

8973.diff still applies and fixes the issue outlined in comment:5 and comment:16.

#18 @chriscct7
3 years ago

  • Keywords needs-refresh added; has-patch removed

2 years ago

Test and updated patch to implement this for 4.4-4.5

#19 @dossy
2 years ago

I wrote up a test case to demonstrate the issue (a commenter's unapproved comments not being included in the count) and a patch to address it (see 8973-1.diff). However, code that calls get_page_of_comment() will need to be updated to be aware of this in order for the user experience to Do The Right Thing, but I'm hoping this at least is a start in the right direction.

Last edited 2 years ago by dossy (previous) (diff)

#20 @chriscct7
2 years ago

  • Owner set to chriscct7
  • Status changed from reopened to reviewing

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

2 years ago

#22 @lukecavanagh
17 months ago


Patch 8973-1.diff applies cleanly to WP 4.7 beta2.

#23 @jdorner
12 months ago

Tried to reproduce, but need more info (steps)

Note: See TracTickets for help on using tickets.