Comment form “Leave a Reply” heading misbehaves in Twenty Nineteen

[westonruter]

The #reply-title element is not getting the correctly populated in Twenty Nineteen. It is being output as:

<span [text]='commentform_post_2013.replyToName ? "Leave a Reply to " + commentform_post_2013.replyToName + "" : "Leave a Reply"'></span>

Notice the text content of the span is empty, but no value of [text] is empty.

In #development=1 mode, this is generating a warning:

Default value () does not match first result (Leave a Reply) for <SPAN [text]="commentform_post_2013.replyToName ? "Leave a Reply to " + commentform_post_2013.replyToName + "" : "Leave a Reply"">. We recommend writing expressions with matching default values, but this can be safely ignored if intentional.

It also exhibits a strange behavior when adding comment text to the form.

Upon entering text:

Then hit tab to go to the Submit button, and the “Leave a Reply” heading all of a sudden appears:

The problem here is that Twenty Nineteen renders its comment form via twentynineteen_comment_form() which does:

comment_form(
	array(
		'logged_in_as' => null,
		'title_reply'  => null,
	)
);

In particular, the title_reply here is supplied as null. This is causing problems in the \AMP_Theme_Support::filter_comment_form_defaults() method which is doing the following:

amp-wp/includes/class-amp-theme-support.php

Lines 1162 to 1191 in 3985eae

	/**
	* Filter comment form args to an element with [text] AMP binding wrap the title reply.
	*
	* @since 0.7
	* @see comment_form()
	*
	* @param array $args Comment form args.
	* @return array Filtered comment form args.
	*/
	public static function filter_comment_form_defaults( $args ) {
	$state_id = self::get_comment_form_state_id( get_the_ID() );
	$text_binding = sprintf(
	'%s.replyToName ? %s : %s',
	$state_id,
	str_replace(
	'%s',
	sprintf( '" + %s.replyToName + "', $state_id ),
	wp_json_encode( $args['title_reply_to'], JSON_UNESCAPED_UNICODE )
	),
	wp_json_encode( $args['title_reply'], JSON_UNESCAPED_UNICODE )
	);
	$args['title_reply_before'] .= sprintf(
	'<span [text]="%s">',
	esc_attr( $text_binding )
	);
	$args['cancel_reply_before'] = '</span>' . $args['cancel_reply_before'];
	return $args;
	}

The problem is that comment_form() applies filters on comment_form_defaults, not on the actual args passed to comment_form(). Therefore, we don't actually know what value for title_reply was passed.

This hasn't been a problem before Twenty Nineteen because themes normally are calling comment_form() without any args at all.

[laurelfulford]

We're running into a similar issue in Newspack, where we have a simple plugin that lets you change the comment form text, including the "reply" header:

https://github.com/Automattic/newspack-rename-comments

The Newspack theme is also a fork of Twenty Nineteen and does have the duplicate header issue, but I'm working on removing some of that code, since it's not needed. We will still need to pass a unique title to comment_form(), though, which gets changed to "Leave a Reply" when you interact with the form or toggle the search.

For the short term, we can work around both by hiding and replacing that title, but if it's possible to have AMP use the title value passed to comment_form(), that would be awesome!

[westonruter]

@laurelfulford I've opened a PR to prevent this from happening. Please test #4388.

[westonruter]

I've also opened #4389 to redo the AMP implementation of WordPress comments so that we can achieve full feature parity.