#reply-title.comment-reply-title not updating when replying to an individual

[iguel]

From what I can tell, in the $defaults array 'title_reply_to' on Line 2227 of wp-includes/comment-template.php is supposed to be called when I click "Reply" to another commenter, but it is not doing so in every theme I have tried, however, it does work in the customizer view. Here's a screenshot of what I found.

[stopdesign]

Adding to this to confirm that I also see this behavior of title_reply_to not changing the title. I have never seen the title change to 'Leave a Reply to %s' as documented when replying to another comment. Even though the form moves and the 'cancel reply' link is made visible -- signs that comment-reply.js seems to have been properly enqueued. This is true not only on my own custom theme that I’m working on now, but on every WordPress site I have found so far, including @matt's own site:

​https://ma.tt/2018/05/wordpress-at-15/#comments

and others with the exact same reproducible bug:

​https://www.positivityblog.com/22-inspirational-quotes-on-fear/#comments

​https://rafaelnadalfans.com/2018/05/31/rafael-nadal-pays-tribute-to-zinedine-zidane/#comments

​http://hurrahforgin.com/2017/12/14/adulthood-sucks/#comments

Observed on WordPress v 4.9.6

[obenland]

Hi @iguel, welcome to WordPress Trac!

What it comes down to here is context. The comment-reply script moves the comment form right underneath the comment you want to reply to. With that context, I assume, comment-reply was not built to update the respond title.

If that script is not present however (or in case of the Customizer the reply link is followed), the form remains at the bottom of the page, and to create context the respond title gets amended with the original commenter's name.

[stopdesign]

Hi @obenland - Thanks for the reply.

I understand the context and how the comment form moves or not in case the script doesn't exist. What I am pointing out (and I think, @iguel too) is that according to the codex for 'comment_form()' the title_reply_to arg exists for the purpose of contextually changing the title to Leave a Reply to %s.

Even if title_reply_to is currently ignored when the script is present, we’re pointing out that title_reply_to should be respected in addition to moving the form, especially when the arg is explicitly included in comment_form() args. If the title is important enough to modify when the form can’t move because of a missing script, it's important enough to modify when the form moves too. The context of where the form appears can still be complemented by a title that responds to the shifted context.

The additional context a modified title helps provide is useful to the user whether the comment form moves or not. It looks broken to me that the title does not update when the context of the reply and form switches, when I know an arg exists for declaring a modified title for replying to another comment.

Thanks for considering.

[audrasjb]

Hi,

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

<!--
Hi there! Thanks for contributing to WordPress!

Pull Requests in this GitHub repository must be linked to a ticket in the WordPress Core Trac instance (​https://core.trac.wordpress.org), and are only used for code review. No pull requests will be merged on GitHub.

See the WordPress Handbook page on using PRs for Code Review more information: ​https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

If this is your first time contributing, you may also find reviewing these guides first to be helpful:

• FAQs for New Contributors: ​https://make.wordpress.org/core/handbook/tutorials/faq-for-new-contributors/
• Contributing with Code Guide: ​https://make.wordpress.org/core/handbook/contribute/
• WordPress Coding Standards: ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/
• Inline Documentation Standards: ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/
• Browser Support Policies: ​https://make.wordpress.org/core/handbook/best-practices/browser-support/
• Proper spelling and grammar related best practices: ​https://make.wordpress.org/core/handbook/best-practices/spelling/

-->

<!-- Insert a description of your changes here -->

When replying to an existing comment, the comment form is moved to below the existing comment with JS, but the form heading was not being updated. This fixes the issue by introducing a new data-attribute to the reply link with the correct heading string, and applying that string to the heading when the form is moved.

Trac ticket: <!-- insert a link to the WordPress Trac ticket here -->

This addresses https://core.trac.wordpress.org/ticket/38009

---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See [GitHub Pull Requests for Code Review](​https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/) in the Core Handbook for more details.

<!-- Insert a description of your changes here -->

Removes most of the boilerplate from the PR description field, so that the synced Trac comment won't be too noisy.
(I have manually deleted the boilerplate from this PR, so see https://core.trac.wordpress.org/ticket/38009 for a sample of what it looks like currently)

Trac ticket: <!-- insert a link to the WordPress Trac ticket here -->
Fixes https://core.trac.wordpress.org/ticket/49508

---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See [GitHub Pull Requests for Code Review](​https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/) in the Core Handbook for more details.

[isabel_brison]

Ugh, the second PR ended up linked to this ticket by mistake, sorry :P

[audrasjb]

Removing trunk as the bug is not specific to the current development branch :)

Superseded by ​https://meta.trac.wordpress.org/ticket/5057.

I spent some time yesterday considering how to avoid adding the new element to the header, without success, I'll continue looking at that today.

Otherwise I think this is good to go.

I spent some time yesterday considering how to avoid adding the new element to the header, without success, I'll continue looking at that today.

Otherwise I think this is good to go.

Note re PHP linting test failure: checking locally comment-template.php contains only warnings so I am pretty sure the tests will only fail on PRs but pass on master. Probably.

I've been working on a couple of approaches to avoid the new element in the comment template, either:

• document.getElementById( 'reply-title' ).firstNode to get the textNode (which can then be replaced. This may need some extra work to get the first textNode to account for sites that have modified the form.
• Cloning the reply title and removing anything that isn't a text nodes document.getElementById( 'reply-title' ).querySelectorAll( '*' )

There are options available.

[noisysocks]

​PR #167 is approved.

[noisysocks]

This was fixed by r47506. Not sure why Trac didn't pick up the ticket mention :)

[mailnew2ster]

This change breaks my theme. See:
​https://i.imgur.com/3eWwbYw.png

[mailnew2ster]

The solution would be to change:

var initialHeadingText = ( 'undefined' !== typeof replyElement ) ? replyElement.firstChild.textContent : '';

to:

var initialHeadingText = ( null !== replyElement ) ? replyElement.firstChild.textContent : '';

or, to be consistent with other similar usage:

var initialHeadingText = replyElement ? replyElement.firstChild.textContent : '';

here:

​https://github.com/WordPress/WordPress/commit/b4e1e8f0d144df6337f1b8978c8f3f79023aa88f#diff-705579fc6109ea62afe07661843ba138R411

Fixes a bug introduced here:
https://core.trac.wordpress.org/ticket/38009#comment:20

See also:
​https://i.imgur.com/3eWwbYw.png

[SergeyBiryukov]

Replying to mailnew2ster:

This change breaks my theme. See:
​https://i.imgur.com/3eWwbYw.png

Thanks for the report and the PR!

Moving to 5.5.1, as this appears to be a regression in [47506].

In the future though, instead of reopening the ticket closed on a completed milestone, please create a new one for any follow-up issues. Thanks again!

[davidbaumwald]

Per the bug scrub today, removing commit and adding needs-testing for the new PR.

[sarahricker]

@johnbillion @SergeyBiryukov I wasn't able to reproduce the bug on 5.5, but the patch looks good to me. Tested with a few core themes. No regressions, no console errors.

[mailnew2ster]

Well, I use a custom theme which doesn't have the relevant element. To reproduce, you can edit any theme to remove that element as well.

[SergeyBiryukov]

In 48876:

Comments: Correct the check for reply element existence in comment-reply.js.

document.getElementById() returns null if no matching element was found, so the previous comparison didn't work as expected.

Follow-up to [47506].

Props mailnew2ster, sarahricker.
Fixes #38009.

[SergeyBiryukov]

In 48877:

Comments: Correct the check for reply element existence in comment-reply.js.

document.getElementById() returns null if no matching element was found, so the previous comparison didn't work as expected.

Follow-up to [47506].

Props mailnew2ster, sarahricker.
Merges [48876] to the 5.5 branch.
Fixes #38009.

[johannadevos]

I tried testing [48876] in the 5.5.1 RC.
In comments.php in my theme (Twenty Twenty), I replaced:
title_reply_before' => '<h2 id="reply-title" class="comment-reply-title">', with:
'title_reply_before' => '<h2>',.

In other words, I removed the "reply-title" id attribute, so that in comment-reply.js the commentReplyTitleId would be undefined.

To test, I opened the console and replied to a comment on a post. That leads to the following error:
Uncaught TypeError: Cannot read property 'nodeType' of null on line 340 of comment-reply.js (concerning replyHeadingTextNode.nodeType).

[mailnew2ster]

@johannadevos I pushed a fix for this.

[SergeyBiryukov]

In 48904:

Comments: Check if reply heading text node exists before accessing its property in comment-reply.js.

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

Props johannadevos, mailnew2ster.
Fixes #38009.

[SergeyBiryukov]

Reopening for the 5.5 branch.

[desrosj]

👍🏼 for backport.

[desrosj]

In 48917:

Comments: Check if reply heading text node exists before accessing its property in comment-reply.js.

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

Props johannadevos, mailnew2ster.
Merges [48904] to the 5.5 branch.
Fixes #38009.

Merged into WP Core in https://core.trac.wordpress.org/changeset/48876