Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#51175 closed defect (bug) (fixed)

Wrong reply box title

Reported by: mailnew2ster's profile mailnew2ster Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.5.2 Priority: normal
Severity: normal Version: 5.5
Component: Comments Keywords: has-patch commit fixed-major
Focuses: docs Cc:

Description

Probably introduced in #38009.

Steps: Click to reply in a new tab. Then click on any other reply link. Observe the duplicate text of the name.

Attachments (4)

vw5aBI0EtO.gif (471.0 KB) - added by mailnew2ster 4 years ago.
51175.diff (706 bytes) - added by wpamitkumar 4 years ago.
comment form title() function $link_to_parent parameter default value going false after it's working fine.
51175.1.diff (917 bytes) - added by audrasjb 4 years ago.
Comments: Return false to $link_to_parent parameter by default in comment_form_title() to avoid double title edge cases
51175.2.diff (1.6 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (18)

#1 @SergeyBiryukov
4 years ago

  • Component changed from General to Comments
  • Milestone changed from Awaiting Review to 5.5.2

#2 @mukesh27
4 years ago

Hi there!

Same issue replicated Twenty Twenty theme.

@wpamitkumar
4 years ago

comment form title() function $link_to_parent parameter default value going false after it's working fine.

#3 @wpamitkumar
4 years ago

  • Keywords has-patch dev-feedback added

#4 @audrasjb
4 years ago

  • Keywords needs-refresh added

The patch looks good at a glance.
I think we'll just want to update the Doc Block comment as well.

@audrasjb
4 years ago

Comments: Return false to $link_to_parent parameter by default in comment_form_title() to avoid double title edge cases

#5 @audrasjb
4 years ago

  • Keywords needs-refresh removed

#6 @mukesh27
4 years ago

  • Focuses docs added
  • Keywords commit added; dev-feedback removed

Thanks @audrasjb for doc updates.

Added commit keyword

#7 @desrosj
4 years ago

  • Keywords needs-refresh added; commit removed

I'm pretty hesitant to change the default value of a function without exploring it's usage in the wild (especially since this function has been around since 2.7.0). I think a better solution would be to call comment_form_title() with the optional third parameter to override the default when the reply to query arguments are present.

Another way to fix this could be to always include the "default" title in the output markup and update the JS to look for that (though that may have backwards compatibility concerns for themes).

Looking into this further, it doesn't appear that #38009 is causing this (though it if is, any changes here should be backported to the 5.5 branch. It seems like this line is responsible.

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


4 years ago

#9 @audrasjb
4 years ago

  • Milestone changed from 5.5.2 to Future Release

Thanks @desrosj. Indeed, those are valid concerns. Let's move this ticket to Future release.

#10 @SergeyBiryukov
4 years ago

  • Keywords commit added; needs-refresh removed
  • Milestone changed from Future Release to 5.5.2

After some testing, it looks like this is indeed a bug in the implementation of [47506] / #38009.

When clicking the "Reply" link, the text content of the heading is updated to include the comment author's name, but that doesn't account for the link to the initial parent comment, which remains there as well.

51175.2.diff hides the extra link when clicking "Reply", and shows it again when clicking "Cancel reply".

#11 @SergeyBiryukov
4 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#12 @SergeyBiryukov
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 49187:

Comments: Hide the link to the initial parent comment in the reply heading when replying to another comment.

This ensures that the reply heading is updated correctly on single post URLs with the ?replytocom query argument.

Follow-up to [47506], [48876], [48904].

Props mailnew2ster, wpamitkumar, audrasjb, desrosj, SergeyBiryukov.
Fixes #51175.

#13 @SergeyBiryukov
4 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 5.5 branch.

#14 @SergeyBiryukov
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 49191:

Comments: Hide the link to the initial parent comment in the reply heading when replying to another comment.

This ensures that the reply heading is updated correctly on single post URLs with the ?replytocom query argument.

Follow-up to [47506], [48876], [48904].

Props mailnew2ster, wpamitkumar, audrasjb, desrosj, SergeyBiryukov.
Merges [49187] to the 5.5 branch.
Fixes #51175.

Note: See TracTickets for help on using tickets.