Make WordPress Core

Opened 10 years ago

Last modified 3 years ago

#29462 reopened defect (bug)

comment pagination in reverse order should display a full number of the latest comments

Reported by: mark-k's profile mark-k Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.9
Component: Comments Keywords: needs-patch needs-testing 2nd-opinion
Focuses: Cc:

Description

set the following discussion setting:
break comment into pages with 5 top level comments per page and the last page displayed by default
Comments should be displayed with the newer comments at the top of each page

have a post with 6 comments

only the last comment made is displayed by default instead of the expected 5 last comments.

Change History (32)

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


9 years ago

#2 @jorbin
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Confirmed that this is still an issue. This requires a patch for us to move forward with fixing it.

#3 @jorbin
9 years ago

  • Owner set to obenland
  • Status changed from new to assigned

#4 follow-up: @peterwilsoncc
9 years ago

I suspect this is by design but I can't find anything on trac.

Currently, each page of comments has a permalink - a link to comment page two will always show comments 6 through 10, regardless of the order set in the discussion settings.

A comment permalink on http://example.com/?p=149 is then http://example.com/?p=149&cpage=3#comment-55

Comment permalinks are important. Making a change to show a full set of comments when displaying the last page first has a two of possible effects:

  • Comment permalinks are not, as they may change cpage, or,
  • Pagination is broken. Clicking older comments would repeat some comments the visitor has already seen.

Up for discussion as to what is the least sub-optimal.

#5 @arikg
9 years ago

I am looking for a solution to this design flaw for months. Even paid a developer to make me a plugin that resolves this issue ( but not in a clean manner unfortunatly)
If there is anything I can do to help promoting this ticket please tell me.

#6 in reply to: ↑ 4 @arikg
9 years ago

Replying to peterwilsoncc:

I suspect this is by design but I can't find anything on trac.

Currently, each page of comments has a permalink - a link to comment page two will always show comments 6 through 10, regardless of the order set in the discussion settings.

A comment permalink on http://example.com/?p=149 is then http://example.com/?p=149&cpage=3#comment-55

Comment permalinks are important. Making a change to show a full set of comments when displaying the last page first has a two of possible effects:

  • Comment permalinks are not, as they may change cpage, or,
  • Pagination is broken. Clicking older comments would repeat some comments the visitor has already seen.

Up for discussion as to what is the least sub-optimal.

BTW, if you don't care about permalinks for some reason than the solution is quite simple. You simply reverse the order of comments (can be done with https://wordpress.org/plugins/reverse-order-comments/) and then in discussion settings you choose to show the "first" comments but because you already reversed the order, you will actually get the last comments. Everything works perfect with this method except the permalinks... the no point to the reversed comment.

#7 @arikg
9 years ago

Is anyone working on this bug?

#8 follow-up: @obenland
9 years ago

The change would break existing comment permalinks, which I doubt we'll be fine with. This feels like a wontfix to me.

#9 in reply to: ↑ 8 ; follow-up: @marshallowen
9 years ago

  • Severity changed from normal to major
  • Version changed from 3.9 to 4.2.4

Replying to obenland:

The change would break existing comment permalinks, which I doubt we'll be fine with. This feels like a wontfix to me.

It should at least be an available option! It is a straightforward premise to add a "Use Reversed Comment Pagination" checkbox to Settings > Discussion > Other Comment Settings. Boolean, default=false so it doesn't affect existing users, but could be set true during new WP installs henceforth.

Please don't doom this to wontfix. It's a huge UX issue all the way around, has been for years, and it should really be a simple, supported fix for those who don't care about their current comment permalinks (or will redirect them).

In my opinion, there should really already be a much more intelligent way of handling comment (and archive!) pagination/permalinks in place by now anyway. Even if that's not happening any time soon, it won't hurt anyone to have an optional stopgap in place in the meantime.

Besides... many core upgrades were more catastrophic than screwing with comment permalinks! #dontwaitdeprecate

#10 in reply to: ↑ 9 @atomicjack
9 years ago

Replying to marshallowen:

Replying to obenland:

The change would break existing comment permalinks, which I doubt we'll be fine with. This feels like a wontfix to me.

It should at least be an available option! It is a straightforward premise to add a "Use Reversed Comment Pagination" checkbox to Settings > Discussion > Other Comment Settings. Boolean, default=false so it doesn't affect existing users, but could be set true during new WP installs henceforth.

Please don't doom this to wontfix. It's a huge UX issue all the way around, has been for years, and it should really be a simple, supported fix for those who don't care about their current comment permalinks (or will redirect them).

In my opinion, there should really already be a much more intelligent way of handling comment (and archive!) pagination/permalinks in place by now anyway. Even if that's not happening any time soon, it won't hurt anyone to have an optional stopgap in place in the meantime.

Besides... many core upgrades were more catastrophic than screwing with comment permalinks! #dontwaitdeprecate

I completely, and wholeheartedly agree with this comment. There should be a better way by now of handling pagination and permalinks.

#11 @peterwilsoncc
9 years ago

  • Severity changed from major to normal
  • Version changed from 4.2.4 to 3.9

While discussion is welcome, please keep the meta data correct while doing so.

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


8 years ago

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


8 years ago

#14 @boonebgorges
8 years ago

In 34729:

Simplify pagination logic in comments_template().

[34561] "fixed" the problem of newest-first comments showing fewer than
'per_page' comments on the post permalink when the total number of comments
was not divisible by 'per_page'. See #29462. But this fix caused numerous
other problems. First, comment pages reported by get_page_of_comment()
(which expects comment pages to be filled oldest-first) were no longer correct.
Second, and more seriously, the new logic caused comments to be shifted
between pages, making their permalinks non-permanent.

The current changeset reverts the changed behavior. In order to preserve the
performance improvements introduced in [34561], an additional query must be
performed when 'default_comments_page=newest' and 'cpage=0' (ie, you're viewing
the post permalink). A nice side effect of this revert is that we no longer
need the hacks required to determine proper comment pagination, introduced in
[34561].

See #8071. See #34073.

#15 @boonebgorges
8 years ago

If we had real comment permalinks, we could make the requested changes. #34106

In the meantime, here's an idea I talked over with peterwilsoncc. Currently, if your post foo has 25 comments with comments_per_page=10, the pages will have the following comments:

  • foo (or foo/comment-page-3): 21-25
  • foo/comment-page-2: 11-20
  • foo/comment-page-1: 1-10

I propose that, instead of treating foo and foo/comment-page-3 the same way, we do the following:

  • When you visit foo/comment-page-3, you will continue to see 21-25.
  • Comment permalinks will continue to have foo/comment-page-3, even when foo/comment-page-3 == foo
  • However, when viewing foo, we'll combine pages 2 and 3 of comments. So you'll see 11-25. This ensures that you don't have a page with just a small handful of comments.

I think this addresses the root issue - that it's ugly to have the 51st comment spill onto its own page - while still maintaining permalinks. What do others think?

#16 follow-up: @mark-k
8 years ago

Sorry, but I don't get the permalink discussion, this sounds to me like backward compatibility gone nuts.

  1. Who cares about backward compatibility here? Is there anyone in the world that is trying to improve the SEO of those pages?, anyone linking to them?. IIRC no sitemap generating plugin add them to the sites sitemap. In addition the feature was probably never used as it was always broken (the trigger for this ticket is that I had to write code to reverse the comments order as the build-in feature was so unacceptable to the client)
  1. The permalink are still there, they are not going anywhere, it is just the content that changes, the same way it would have changed if comments were added/modified/removed, so where is the issue?

#17 in reply to: ↑ 16 ; follow-ups: @boonebgorges
8 years ago

Replying to mark-k:

  1. Who cares about backward compatibility here?
  1. The permalink are still there, they are not going anywhere, it is just the content that changes

The fact that content changes is exactly the problem. If I leave a blog comment, I should be able to use the permalink of that comment to refer back to that comment at any time: through a link on my own site, through a link I send someone in an email, through a browser bookmark, etc.

#18 in reply to: ↑ 17 @mark-k
8 years ago

Replying to boonebgorges:

Replying to mark-k:

  1. Who cares about backward compatibility here?
  1. The permalink are still there, they are not going anywhere, it is just the content that changes

The fact that content changes is exactly the problem. If I leave a blog comment, I should be able to use the permalink of that comment to refer back to that comment at any time: through a link on my own site, through a link I send someone in an email, through a browser bookmark, etc.

ok, now I understand the issue, but

  1. My first point stands, it was such a visually broken feature I can't imagine anyone using it at all, so whatever backward compatibility will be broken by a change will not have any real life effect.
  1. And the specific issue is links to comments? How rare is that in general?
  1. This discussion really highlights that comments do not have a permalink. A permalink should always lead to a specific content. no matter how content is displayed, but right now the permalinks of paginated comments depend on the number of comments displayed at each page which is an easy configuration change away which do not warn the user that the in-bound comment links will be broken by that.

Comment permalink should probably be a proper one - <site url>/<post slug>/comment/<comment number> and let wordpress figure out how many comments and which should be displayed with the requested comment (or use the hash notation which works when all comments are displayed with the post).

#19 @boonebgorges
8 years ago

I don't disagree with any of these three points. The third one is especially important. See #34106. But even when/if we have real permalinks, we have to think about legacy permalinks, and the extent to which we're willing to break them. What I proposed above (merging the nth and n-1th pages when viewing the post permalink) is a way of easing the current situation without breaking anything, which is why I think we should consider it in the short term.

#20 @arikg
8 years ago

If you don't care about permalinks for some reason than the solution is quite simple.
You simply reverse the order of comments (can be done with https://wordpress.org/plugins/reverse-order-comments/) and then in discussion settings you choose to show the "first" comments but because you already reversed the order, you will actually get the last comments. Everything works perfect with this method except the permalinks... they do no point to the right comment anymore

Last edited 8 years ago by arikg (previous) (diff)

#21 @leanderbraunschweig
8 years ago

This is an issue indeed and I am also looking forward to seeing it corrected.

Has anyone been looking at https://wordpress.org/plugins/fix-reversed-comments-pagination? It is 5 years old and I cannot easily test it on our site because we are using a custom comment walker already (would have to merge those two) but maybe it proposes a solution?

Otherwise I'd also be in favor of @boonebgorges solution where we have two values in place, one specifying the regular comment amount to be pagination (50) and thus one (automatic) special value for the newest page (50-99) so the user is always properly seeing comments on the post/page.

#22 in reply to: ↑ 17 @FolioVision
8 years ago

Replying to boonebgorges:

The fact that content changes is exactly the problem. If I leave a blog comment, I should be able to use the permalink of that comment to refer back to that comment at any time: through a link on my own site, through a link I send someone in an email, through a browser bookmark, etc.

In this case, as the original feature is hopelessly broken by design, adding some fresh code to handle comment permalinks and pagination is just sensible. No one can or would use the current comment permalinks.

@obenland

It is relatively trivial to write a redirection routine to make sure that the old links continue to work. We've done a lot of work on the Redirection Plugin https://wordpress.org/plugins/redirection/ over the years and would be happy to help contribute some redirection code to handle the legacy comment links once a new working comment structure has been written.

A bigger issue for me than handling redirection of legacy links is hashing out new comment code and structure which can stand the test of time and not leave us hobbled in three years like the current code.

#23 @obenland
7 years ago

  • Owner obenland deleted

#25 @pento
5 years ago

#46027 was marked as a duplicate.

#26 @afercia
5 years ago

This ticket should be reopened, as it is still an issue and new reports are being reported (see #46027 closed as a duplicate). Pinging the current component maintainer listed on https://make.wordpress.org/core/components /Cc @wonderboymusic also @boonebgorges

#27 @boonebgorges
5 years ago

  • Milestone set to Future Release
  • Status changed from assigned to reopened

I agree that it should be reopened. The path I outlined above still seems like a viable solution in the short- to medium-term (until such time as real comment permalinks are put in place). https://core.trac.wordpress.org/ticket/29462#comment:15

#28 @juslintek
5 years ago

Okay I figured this out. Why not use: comments_template_query_args filter hook, for overriding default params. Ofcourse this could have UI in wp-admin for controlling all these settings, inside site/web/wp/wp-includes/comment-template.php handler. And if you're building your own comments handler then I thinks its not core anymore. I've added this code to my theme and pagination and permalinks still work and manage to resolve to the page of the comment:

<?php
add_filter('comments_template_query_args', function($comment_args) {
    $comment_args['order'] = 'DESC';
    return $comment_args;
});

#29 @ituk
5 years ago

I agree also it should be reopened. The issue is still there and the broken permalinks shouldn't be as important in my opinion.
Don't know about @juslintek solution, it doesn't seems like the right clean approach, but there must be a way to bypass this thing.

This ticket was mentioned in Slack in #core-comments by peterwilsoncc. View the logs.


3 years ago

#31 @askel45
3 years ago

  • Keywords needs-testing 2nd-opinion added
  • Resolution set to invalid
  • Status changed from reopened to closed

Using paginate_comments_links() to show numbered pagination in comments instead of 'older comments' or 'newer comments' causes a problem. When comments are broken into pages, with the last page and newer comments shown by default, the numbered pagination shows the current page as the last in the pagination. For example, if there are 60 comments broken down to 20 comments per page, the initial post-loading shows page 3 as the current page (instead of page 1) and a person has to navigate backwards to read other comments (page 2 then page 1).

#33 @desrosj
3 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened
Note: See TracTickets for help on using tickets.