Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#34068 closed defect (bug) (invalid)

Comment permalink always include comment page

Reported by: knutsp's profile knutsp Owned by: boonebgorges's profile boonebgorges
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

comment-page-1/ now appears in all comment permalinks, even for the first comment page.

Related: #8071

Attachments (2)

34068.diff (2.1 KB) - added by boonebgorges 10 years ago.
34068.2.diff (1.0 KB) - added by knutsp 10 years ago.
Only add comments_pagination_base when not on first comment page

Download all attachments as: .zip

Change History (12)

#1 @boonebgorges
10 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.4

This is due to the fact that pagination is always forced, so 'page_comments' no longer does anything.

In fact, prior to [34561], 'comment-page-1' *did* appear on all comment first pages, when 'page_comments' was enabled.

It seems to me that 'comment-page-1' should be banished. Page 1 of comments should always have the same URL as the post itself. This change will require a battery of unit tests to account for the various scenarios (specifically, the various values of 'default_comments_page'). get_comment_link() appears to be the correct place for this change.

@boonebgorges
10 years ago

#2 @boonebgorges
10 years ago

  • Keywords has-patch added; needs-patch removed

Battery's about to die :) so I can't finish the tests at the moment, but I think that 34068.diff gets it pretty close to right. Test it with this:

function wp_34068( $text, $comment ) {
	$link = get_comment_link( $comment );

	return $link . '<br />' . $text;
}
add_filter( 'comment_text', 'wp_34068', 10, 2 );

and with various pagination settings.

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


10 years ago

@knutsp
10 years ago

Only add comments_pagination_base when not on first comment page

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


10 years ago

#5 @boonebgorges
10 years ago

In 34716:

Use array hash notation for get_comment_link() $args.

See #34068, #34073.

#6 @boonebgorges
10 years ago

  • Keywords needs-unit-tests removed

It turns out that this ticket is most easily solved alongside #34073. knutsp and others - your testing on the latest patch there would be most helpful.

#7 @wonderboymusic
10 years ago

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

#8 @boonebgorges
10 years ago

  • Milestone 4.4 deleted
  • Resolution set to invalid
  • Status changed from assigned to closed

After some discussion with Viper007Bond and peterwilsoncc (see #34073 and https://core.trac.wordpress.org/ticket/8071#comment:49), I'm going to close this ticket as invalid. Here's the argument:

  • Comment permalinks should be permanent
  • Permalinks without cpage/comments-page-x are only permanent when comments do not paginate
  • After #8071, comments always paginate at some threshold or other
  • So we need cpage/comments-page-x

#9 @boonebgorges
10 years ago

(To clarify the comments above: comment-page-1 will be left off when the *oldest* comments are shown first. See #34073. But when *newest* comments are shown first, we will add the pagination query var, even if the post has only a single page of comments; because as soon as it breaks into multiple pages, the permalinks must continue to work.)

#10 @boonebgorges
10 years ago

In 34735:

Ensure that comment permalinks reflect pagination.

After [34561], wp_list_comments() no longer passed all of a post's comments
to Walker_Comments. As a result, calls to get_comment_link() occurring
inside the comment loop had insufficient context to determine the proper
'cpage' value to use when generating comment permalinks. This, in turn, caused
comment permalinks to behave erratically.

The current changeset addresses the problem as follows:

  • get_comment_link() now accepts a 'cpage' parameter. When present, 'cpage' will be used to build the comment permalink - no automatic calculation will take place.
  • When called within the main loop, wp_list_comments() calculates the proper 'cpage' value for comments in the loop, and passes it down to get_comment_link().
  • cpage and comment-page-x query vars are generally required in comment permalinks (see #34068), but an exception is made when 'default_comment_page=oldest': the bare post permalink will always be the same as cpage=1, so cpage is excluded in this case.

Props peterwilsoncc for assiduous spreadsheeting.
Fixes #34073.

Note: See TracTickets for help on using tickets.