WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#39280 closed defect (bug) (fixed)

comment permalink wrong in WordPress 4.7

Reported by: jikamens Owned by: boonebgorges
Milestone: 4.7.1 Priority: normal
Severity: normal Version: 4.7
Component: Comments Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

I just got notified via email about a new comment on a page on my blog. The notification email says:

Permalink: https://blog.kamens.us/send-later/comment-page-1/#comment-1684426

There are three pages of comments on this page. "comment-page-1" is the oldest comments, obviously, not the newest ones, so this permalink is wrong.

When I go to edit the comment in the WordPress dashboard, the permalink displayed on the comment editing page is wrong as well.

I believe this issue is a regression in 4.7.

Attachments (2)

Selection_003.png (33.2 KB) - added by jikamens 6 months ago.
Screen shot of "Other comment settings"
39280.diff (1.9 KB) - added by rachelbaker 6 months ago.
revert of r38740

Download all attachments as: .zip

Change History (20)

#1 @jikamens
6 months ago

This problem is even more pervasive than reported above... I just posted a new comment on my blog (through the front-end, not through the dashboard), and when it redirected me to the comment after accepting it, it had "/comment-page-1" in the redirect URL.

It would seem that there is a pretty low-level bug with generating comment URLs.

#2 @johnbillion
6 months ago

  • Component changed from Permalinks to Comments
  • Focuses administration removed
  • Keywords reporter-feedback added
  • Milestone changed from Awaiting Review to 4.7.1

Thanks for the report @jikamens.

I'm unable to reproduce the issue on a fresh installation of WordPress 4.7. Could you provide a screenshot of your Other comment settings section on the Settings -> Discussion screen in WordPress please?

Are you able to switch to one of the default themes (such as Twenty Seventeen) to see if the issue persists?

@jikamens
6 months ago

Screen shot of "Other comment settings"

#3 @jikamens
6 months ago

If this were a theme issue, then the permalink at the top of the page when editing a comment would not be affected, since the dashboard isn't affected by themes.

I just switched to the Twenty Seventeen theme temporarily and confirmed that the permalink on the comment editing page is still wrong.

#4 @jikamens
6 months ago

  • Keywords reporter-feedback removed

#5 @johnbillion
6 months ago

  • Keywords needs-patch needs-unit-tests added

Thanks for the further information.

I've confirmed this is an issue with that particular configuration of comment settings. The newer comments at the top of the page setting seems to be the problem.

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


6 months ago

#7 @sandradepalma
6 months ago

I see exactly the same problem in my Wordpress installations (self-hosted on IIS). What I have found. Comment permalinks are wrong in:

  • Notification Emails
  • "Latest comments" widget
  • Redirect when posting a comment
  • In the comments overview in the "In reply to" link

Going back to Wordpress 4.6.1 cures the problem.

It is not theme related as I tried with 3 different themes.

It is not plugin related as I tried on an installtions with no plugins and it happens there as well.

It happens only when you have multiple pages of comments, my settings are:

  • "Break comments into pages with 5 top level comments and the LAST page displayed by default"
  • "Comments should be displayed with the NEWER comments at the top of each page"

The permalinks are showing with "comment-page-1/#comment-13911"
But the should show with "comment-page-26/#comment-13911"

Where "26" is in my case the highest page number of the comments.

Last edited 6 months ago by sandradepalma (previous) (diff)

#8 @WetCatBooks
6 months ago

Subscribe to Comments Reloaded plugin is also affected. I had someone add some code that does fix the redirect problem the readers were experiencing when hitting the comment submit button. But since the new code went into the theme's functions.php file, it doesn't help other issues caused by the glitch.

The following URL is for an existing comment, as it now appears correctly on the public page, after the "fix" was added to the theme’s functions.php file:

http://www.ultimatepapermache.com/daily-sculptors-group-page/comment-page-76#comment-140264

On the back end, which is still governed by the WP program, the same email permalink is generated this way:

http://www.ultimatepapermache.com/daily-sculptors-group-page/comment-page-1#comment-140264

Wordpress is counting the comment pages from the wrong end, (which is probably obvious to everyone by now), and the Subscribe to Comments plugin is sending out the wrong links. I disabled it, until the next WP plugin.

If anyone is interested in the temporary partial fix, I'd be happy to share it if someone will point out the way to add the code to the text box. I don't want to break anything... ;)

#9 @boonebgorges
6 months ago

The problem stems from [38740] #31101. Looking more closely, the fix in that ticket was incorrect for, I think, two different reasons.

  1. The "bug" identified in the ticket wasn't actually a bug. The Comments should be displayed with the [oldest|newest] comments at the top of each page setting has nothing to do with pagination. It only has to do with how comments are sorted on a specific page of results. I believe that the ticket should've been closed as invalid.
  2. The expected behavior described in the test introduced in [38740] is incorrect. The page of a comment (comment-page-x) does *not* depend on either the 'comment_order' or 'default_comment_page' options. The comment pagination system is designed in such a way that pagination-based URLs are "permalinks" (though not quite - see #34106). The 'default_comment_page' setting works by flipping the order in which pages appear, *not* the content of those pages. Consider a post with 10 pages of comments. When 'default_comment_page' is set to 'last' (option value newest), the first comment page is the post URL and is equivalent to page 10; hitting "Older comments" takes you to comment-page-9.

I believe the proper fix is simply a revert of [38740]. @rachelbaker @johnbillion Could I get a couple sanity checks on this, since we got it wrong last time? I will take the blame for the previous error, but I don't want to make it twice.

#10 follow-up: @WetCatBooks
6 months ago

I don't know if this will help or not but I asked a friend, Alan Jorgensen, to look into this issue before I knew you were working on it. He asked me to pass on his comments. I put them in a text file and you can find it here: http://www.ultimatepapermache.com/wp-content/uploads/2016/12/comment-glitch.txt

#11 in reply to: ↑ 10 @fumbo
6 months ago

I have the same problem.

I use comment.php from my wp 4.5.3 and it works. For me is OK.

#12 @rachelbaker
6 months ago

#39266 was marked as a duplicate.

#13 @boonebgorges
6 months ago

@rachelbaker While this is fresh (2 hours!) in your mind, could you have a look at my reasoning above? I'd like to make sure this is included in any 4.7.1.

@rachelbaker
6 months ago

revert of r38740

#14 @rachelbaker
6 months ago

  • Keywords has-patch added; needs-patch removed

@boonebgorges Wow. Comment permalinks are amazing. I agree, a revert of r38740 is what should happen here. The revert diff is 39280.diff. Can you do a final sanity check on my diff?

Before committing, it would be nice to have unit tests for the expected behavior. Also seems like after this ticket is fixed, we should change #31101 to be "works as intended".

#15 @boonebgorges
6 months ago

Thanks for reviewing, @rachelbaker. Revert looks good to me.

I will add tests demonstrating the behavior of pagination alongside comment_order and default_comment_page.

Also seems like after this ticket is fixed, we should change #31101 to be "works as intended".

I'll reference the ticket in the commit message, for future viewers of that ticket. I don't want to change the resolution on a ticket that was closed against an old milestone.

#16 @boonebgorges
6 months ago

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

In 39663:

Ignore the 'comment_order' setting when determining comment pagination.

[38740] incorrectly introduced logic that changed a comment's page when
'comment_order' was set to 'desc'. This is in violation of the design
of the comment pagination system: a comment's page is designed not to
change when 'comment_order' or 'default_comment_page' are changed.
See #31101.

Props rachelbaker.
Fixes #39280.

#17 @boonebgorges
6 months ago

In 39664:

Ignore the 'comment_order' setting when determining comment pagination.

[38740] incorrectly introduced logic that changed a comment's page when
'comment_order' was set to 'desc'. This is in violation of the design
of the comment pagination system: a comment's page is designed not to
change when 'comment_order' or 'default_comment_page' are changed.
See #31101.

Merges [39663] to the 4.7 branch.

Props rachelbaker.
Fixes #39280.

#18 @wtarkan
6 months ago

Here is the fixed comment.php , one of my friend (A php coder) has fixed it

https://drive.google.com/open?id=0B1BZr74vnHUlUUVzN0NyU2p4VjQ

Note: See TracTickets for help on using tickets.