#51189 closed defect (bug) (fixed)
comments_pagination_base missing in get_comment_reply_link() function
Reported by: | MrPauloEn | Owned by: | joedolson |
---|---|---|---|
Milestone: | 5.8 | Priority: | normal |
Severity: | normal | Version: | 5.5 |
Component: | Comments | Keywords: | has-screenshots good-first-bug has-patch has-testing-info commit |
Focuses: | accessibility | Cc: |
Description
Hi guys.
I noticed that get_comment_reply_link() does not create a valid reply link when comments are paginated.
When I say: "not valid" I mean without comments_pagination_base part like this is in previous_comments_link or next_comments_link or get_comment_link function.
I reffer exactly to this line:
https://core.trac.wordpress.org/browser/tags/5.5/src/wp-includes/comment-template.php#L1738
This issue is hard to notice until javacsript is disabled or the comment-reply script is not loaded or works incorrectly.
Only then this link is an ordinary clickable link. If you click it, the page reloads as usually to show us such a reply form:
You reply to: Jane Doe:
Username is then linked to his comment. So when you click it, it should move you to this comment. It works correctly if you have only one comment page.
But…
It won't work, if you split comments into pages.
Option: page_comments is enabled an there are more than one comment page.
It won't move you, if comment is on another page.
Why?
Because there is comments_pagination_base missing in that url.
Consider this with an example.
Now my comment reply link is this:
http://example.com/about/page-with-comments/?replytocom=179#respond
And the comment has this link:
http://example.com/about/page-with-comments/comment-page-2/#comment-179
If I want to jump for a moment to this comment, I can't. It just produce this link and nothing happens:
http://example.com/about/page-with-comments/?replytocom=179#comment-179
But when I slightly modify it:
http://example.com/about/page-with-comments/comment-page-2/?replytocom=182#respond
So, the mentioned author anchor has this link:
http://example.com/about/page-with-comments/comment-page-2/?replytocom=179#comment-179
And so, the comment has this link:
http://example.com/about/page-with-comments/comment-page-2/#comment-179
It skip to this comment when I clik user name anchor and come back when I click Reply link, becaouse username anchor has comment pagination url part.
So whan I did.
I looked into get_comment_reply_link() function, I copied some of a piece of code and I made a new filter, hooked to comment_reply_link filter.
I just added this to url in add_query_arg function:
get_permalink( $post->ID ) . $wp_rewrite->comments_pagination_base . '-' . $page . '/'
Whole filter looks like this:
add_filter( 'comment_reply_link', 'the_bootstrap_blog__filter_comment_reply_link', 10, 4 ); function the_bootstrap_blog__filter_comment_reply_link( $link, $args, $comment, $post ){ global $wp_rewrite; $page = get_page_of_comment( $comment->comment_ID ); // Copied from get_comment_reply_link() function if ( get_option( 'comment_registration' ) && ! is_user_logged_in() ) { $link = sprintf( '<a rel="nofollow" class="comment-reply-login" href="%s">%s</a>', esc_url( wp_login_url( get_permalink() ) ), $args['login_text'] ); } else { $data_attributes = array( 'commentid' => $comment->comment_ID, 'postid' => $post->ID, 'belowelement' => $args['add_below'] . '-' . $comment->comment_ID, 'respondelement' => $args['respond_id'], 'replyto' => sprintf( $args['reply_to_text'], $comment->comment_author ), ); $data_attribute_string = ''; foreach ( $data_attributes as $name => $value ) { $data_attribute_string .= " data-${name}=\"" . esc_attr( $value ) . '"'; } $data_attribute_string = trim( $data_attribute_string ); $link = sprintf( "<a rel='nofollow' class='comment-reply-link' href='%s' %s aria-label='%s'>%s</a>", esc_url( add_query_arg( array( 'replytocom' => $comment->comment_ID, 'unapproved' => false, 'moderation-hash' => false, ), get_permalink( $post->ID ) . $wp_rewrite->comments_pagination_base . '-' . $page . '/' ) ) . '#' . $args['respond_id'], $data_attribute_string, esc_attr( sprintf( $args['reply_to_text'], $comment->comment_author ) ), $args['reply_text'] ); } return $args['before'] . $link . $args['after']; }
And it works fine.
I know that this is only for my testing purposes, but if I have to put the theme in the WordPress repository, it should be more like in this (get_comment_link()) function.
I use:
- WordPress v5.5
I noticed this issue a long time ago, but somehow I ignored it, didn't know how to fix it.
I tested this on default theme (twentytwenty), with all plugins deactivated and on my theme with all plugins activated. In each cases is the same.
Only when I add that filter, thoses links works fine.
Attachments (2)
Change History (26)
This ticket was mentioned in Slack in #accessibility by azhiyadev. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#9
@
4 years ago
- Keywords good-first-bug added
This ticket was discussed today during the accessibility team's bug-scrub.
Given it might be an easy fix, it can be a good first bug: if any new contributor wants to jump in, feel free to do it!
#10
@
4 years ago
- Keywords has-patch added; needs-patch removed
In addition to @MrPauloEn's solution, I added an if condition to check if a user has enabled the 'Break comments into pages' option from the settings.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
3 years ago
#12
@
3 years ago
How to test:
1) Turn on 'Break comments into pages' at Settings > Discussion
2) Add comments and set the comments per page to a number that will generate at least two pages of comments.
3) Disable comment-reply.js
<?php function test_disable_comment_js(){ wp_deregister_script( 'comment-reply' ); } add_action( 'init', 'test_disable_comment_js' );
4) Go to page 2 of comments on any post with multiple pages of comments.
5) Click on 'Reply' to a comment on this page.
6) Click on the commenter's name to return to comment you're replying to.
Without patch: link points to an anchor on the current page, but the comment does not appear on this page.
With patch: link should point to an anchor on the correct page of comments.
Note: this is a case where the link should open in a new tab if the user has started writing their comment, as otherwise their comment text can be lost.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
3 years ago
This ticket was mentioned in Slack in #accessibility by chaion07. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-test by boniu91. View the logs.
3 years ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
3 years ago
#21
@
3 years ago
I can confirm the issue before applying the patch and confirm also that the patch solved this issue.
But I have a question here:
Do we need to change this link also for the first page or leave the first page reply link to go to the post itself?
as current code will make the link for first page go to different url but with the same content, is it bad for SEO?
For the post page (first comments page)
before patch
http://localhost:8889/hello-world/?replytocom=6#respond
after patch
http://localhost:8889/hello-world/comment-page-3/?replytocom=6#respond
and I think before patch, first page link is the correct one, what do you think?
#22
@
3 years ago
No, I don't think that's a concern. The link that get_comment_link()
will produce for the comment is a more permanent link; you'll always be able to find it there barring changes to the pagination settings. The other link is not permanent, as adding additional comments could push it off the front page.
get_comment_link()
is the correct way to produce a comment link, and I don't think we should attempt to determine whether a given comment should currently be on the front page.
The SEO concerns should be nominal, given that this link is overridden with JavaScript in normal usage & WordPress provides canonical URL references.
The ticket was reviewed today during the accessibility team's bug-scrub. For the moment, we'll move the ticket to 5.8 to try to include it in next release and eventually punt it later.