WordPress.org

Make WordPress Core

Opened 10 years ago

Last modified 6 months ago

#11248 assigned defect (bug)

Paged wp_list_comments() should always output something

Reported by: Viper007Bond Owned by: boonebgorges
Milestone: Priority: normal
Severity: normal Version: 2.9
Component: Comments Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Enable comment paging on your blog and visit http://yourblog.com/a/single/post/comment-page-999999/. Note no comments will be displayed.

There's situations where the comment form will redirect you back to the wrong page (for example if you mess with the type parameter and exclude pings). The Walker should detect if there's no comments to be displayed on the requested page and if that's the case, then display the latest page that actually has comments.

Attachments (4)

comment-template.php.patch.13396.txt (613 bytes) - added by edward mindreantre 10 years ago.
11248.patch (6.6 KB) - added by couturefreak 5 years ago.
Displays last page of comments, and fixes the resulting comment pagination links.
11248.2.patch (1.9 KB) - added by tyxla 4 years ago.
In redirect_canonical() redirect comment pages that are larger than the maximum comment page to the post itself.
11248.3.patch (2.1 KB) - added by tyxla 4 years ago.
Updating the previous patch to use a more efficient way of getting the total comment pages (props @boonebgorges for the suggestion)

Download all attachments as: .zip

Change History (22)

#1 @edward mindreantre
10 years ago

  • Cc edward mindreantre added
  • Keywords has-patch added; needs-patch removed

Fixed.

#2 @chriscct7
5 years ago

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

Seems to still be valid in 4.0

#3 @chriscct7
5 years ago

  • Keywords needs-refresh removed

#4 @chriscct7
5 years ago

  • Keywords has-patch added

#5 @chriscct7
5 years ago

Patch is still good ( I misread it for a second there )

@couturefreak
5 years ago

Displays last page of comments, and fixes the resulting comment pagination links.

#7 @rachelbaker
4 years ago

  • Keywords needs-refresh added

@couturefreak Thanks for the patch. Would you mind refreshing it without the extra formatting fixes?

#8 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to wonderboymusic
  • Status changed from new to assigned

#9 @wonderboymusic
4 years ago

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

This should be a fix in canonical

#10 @wonderboymusic
4 years ago

  • Owner wonderboymusic deleted

@tyxla
4 years ago

In redirect_canonical() redirect comment pages that are larger than the maximum comment page to the post itself.

#12 @tyxla
4 years ago

  • Keywords has-patch added; needs-patch removed

11248.2.patch fixes the issue as @wonderboymusic suggested.

Canonical unit test case included.

This will basically redirect the comment page links for pages that are larger than the maximum to the post link.
So for example if there is a maximum of 3 comment pages,

/comment-test/comment-page-4/

will redirect to

/comment-test/.

#13 @boonebgorges
4 years ago

  • Keywords needs-patch added; has-patch removed

Let's not pull up every single comment object belonging to a post to determine the page of comment. I recently wrote something that is a little more lightweight for determining the number of top-level comments on a post - see the top_level_count logic in [34729]. Maybe we can separate this out, and use it to determine the number of comment pages here.

@tyxla
4 years ago

Updating the previous patch to use a more efficient way of getting the total comment pages (props @boonebgorges for the suggestion)

#14 @tyxla
4 years ago

  • Keywords has-patch added; needs-patch removed

Makes sense. I've updated the patch to use the solution @boonebgorges suggested for fetching the number of top level comments. This is used when calculating the total number of comment pages.

#15 @boonebgorges
4 years ago

  • Owner set to boonebgorges

I wrote a set of functions to do the number-of-pages calculation in https://core.trac.wordpress.org/attachment/ticket/8071/8071.8.diff. I'm going to wait to land that before taking care of the current ticket.

#16 @boonebgorges
4 years ago

  • Keywords 4.5-early added
  • Milestone changed from 4.4 to Future Release

#17 @boonebgorges
4 years ago

  • Keywords 4.5-early removed
  • Milestone changed from Future Release to 4.5

#18 @boonebgorges
4 years ago

  • Milestone changed from 4.5 to Future Release
Note: See TracTickets for help on using tickets.