Make WordPress Core

Opened 15 years ago

Closed 4 months ago

Last modified 3 months ago

#16576 closed enhancement (fixed)

comment_form() fields being displayed only for non logged in users

Reported by: maorb's profile maorb Owned by: davidbaumwald's profile davidbaumwald
Milestone: 6.9 Priority: normal
Severity: normal Version: 3.0.5
Component: Comments Keywords: has-patch bulk-reopened commit
Focuses: Cc:

Description

I've just noticed this - When using the comment_form() function and adding some comment meta fields,using the fields array in the $args, these fields are being showed in the front-end only for non logged in users.
In this case, the registered users can never use these comment fields.

Look at wp-includes/comment-template, lines 1561-1573 (WP 3.0.5), it parse the $argsfields? in the else block -

<?php if ( is_user_logged_in() ) : ?>
	<?php echo apply_filters( 'comment_form_logged_in', $args['logged_in_as'], $commenter, $user_identity ); ?>
	<?php do_action( 'comment_form_logged_in_after', $commenter, $user_identity ); ?>
<?php else : ?>
	<?php echo $args['comment_notes_before']; ?>
	<?php
	do_action( 'comment_form_before_fields' );
	foreach ( (array) $args['fields'] as $name => $field ) {
		echo apply_filters( "comment_form_field_{$name}", $field ) . "\n";
	}
	do_action( 'comment_form_after_fields' );
	?>
<?php endif; ?>

I think it is better that added meta fields should be displayable for all users (logged in and not logged in), or even to add a parameter to let the developer decide which extra fields are shown to logged or not logged users.

Attachments (1)

16576.diff (2.3 KB) - added by valendesigns 11 years ago.

Download all attachments as: .zip

Change History (30)

#2 @valendesigns
14 years ago

I agree, it should be an option. Just because you're logged in doesn't mean you have set your URL in your profile and there is still the chance that users are subscribers and you hide the admin from them. I'm currently dealing with this. I was using the comment_form() function but just realized the inputs are hidden when logged in and our system doesn't allow them to edit their WP profile because we use aMember to authenticate and maintain profile data. Back to regular manual comment forms I guess.

#3 @chriscct7
11 years ago

  • Keywords dev-feedback added

#4 @valendesigns
11 years ago

Hey Guys,

I would love to work on this ticket as my first contribution to the core, but a group consensus should be reached about how to approach the subject before I start coding anything.

How about adding an additional argument that if set to true (defaults to false) will loop over the $args['fields'] in the logged in block. However, adding the loop again isn't very DRY and would still need to be distilled down to something that doesn't repeat the $args['fields'] loop by creating a new function that looks something like this...

function comment_form_fields( $fields = array() ) {
  if ( ! is_array( $fields ) )
    return false;

  /**
   * Fires before the comment fields in the comment form.
   *
   * @since 3.0.0
   */
  do_action( 'comment_form_before_fields' );
  foreach ( (array) $fields as $name => $field ) {
    /**
     * Filter a comment form field for display.
     *
     * The dynamic portion of the filter hook, $name, refers to the name
     * of the comment form field. Such as 'author', 'email', or 'url'.
     *
     * @since 3.0.0
     *
     * @param string $field The HTML-formatted output of the comment form field.
    echo apply_filters( "comment_form_field_{$name}", $field ) . "\n";
  }
  /**
   * Fires after the comment fields in the comment form.
   *
   * @since 3.0.0
   */
  do_action( 'comment_form_after_fields' );
}

Then it could be used in both if/else blocks without any repetition being added to the code. It's a simple solution. If anyone else has any suggestions or thinks this is bad idea please let me know. I'm eager to start contributing to the core and this looks like a quick and easy fix for me to get started with.

Cheers,
Derek

#5 @valendesigns
11 years ago

Actually, an alternative to the above would be to change up the if/else block. This would assume that there is an argument called $args['show_fields_logged_in'] to work, but the argument could be named anything. I think this would be a better solution because there is very little meddling in the code and very little needs to be changed in the docs to support it.

<?php if ( is_user_logged_in() ) : ?>
	<?php
	/**
	 * Filter the 'logged in' message for the comment form for display.
	 *
	 * @since 3.0.0
	 *
	 * @param string $args_logged_in The logged-in-as HTML-formatted message.
	 * @param array  $commenter      An array containing the comment author's
	 *                               username, email, and URL.
	 * @param string $user_identity  If the commenter is a registered user,
	 *                               the display name, blank otherwise.
	 */
	echo apply_filters( 'comment_form_logged_in', $args['logged_in_as'], $commenter, $user_identity );
	?>
	<?php
	/**
	 * Fires after the is_user_logged_in() check in the comment form.
	 *
	 * @since 3.0.0
	 *
	 * @param array  $commenter     An array containing the comment author's
	 *                              username, email, and URL.
	 * @param string $user_identity If the commenter is a registered user,
	 *                              the display name, blank otherwise.
	 */
	do_action( 'comment_form_logged_in_after', $commenter, $user_identity );
	?>
<?php endif; ?>
<?php if ( ! is_user_logged_in() || true === $args['show_fields_logged_in'] ) : ?>
	<?php echo $args['comment_notes_before']; ?>
	<?php
	/**
	 * Fires before the comment fields in the comment form.
	 *
	 * @since 3.0.0
	 */
	do_action( 'comment_form_before_fields' );
	foreach ( (array) $args['fields'] as $name => $field ) {
		/**
		 * Filter a comment form field for display.
		 *
		 * The dynamic portion of the filter hook, $name, refers to the name
		 * of the comment form field. Such as 'author', 'email', or 'url'.
		 *
		 * @since 3.0.0
		 *
		 * @param string $field The HTML-formatted output of the comment form field.
		 */
		echo apply_filters( "comment_form_field_{$name}", $field ) . "\n";
	}
	/**
	 * Fires after the comment fields in the comment form.
	 *
	 * @since 3.0.0
	 */
	do_action( 'comment_form_after_fields' );
	?>
<?php endif; ?>

@valendesigns
11 years ago

#6 @valendesigns
11 years ago

  • Keywords has-patch needs-testing added

I've added a patch that's a bit different than both of my previous suggestions.

I moved all of the fields related code out of the else block following the if ( is_user_logged_in() ) condition; it now comes directly after the endif for that code block. Then added a check for is_user_logged_in() inside the $args['fields'] loop that will skip the 'author', 'email', & 'url' array keys. Which means custom fields will be displayed to both sets of users, but the default fields will only be visible to those not logged in. This way we don't need any extra documentation or to add arguments to the comment_form() function.

#7 follow-up: @Yahire Furniture
11 years ago

Did you manage to sort it in the end? I am having a similar issue.

#8 in reply to: ↑ 7 @valendesigns
11 years ago

Replying to Yahire Furniture:

Did you manage to sort it in the end? I am having a similar issue.

Yes, the patch fixes the issue. However, it doesn't seem to be gaining traction with the core developers just yet. Hopefully we'll get someone in here to give feedback soon.

Cheers,
Derek

#9 @CarlSteffen
10 years ago

Hello - I see this topic hasn't received any comments in a while - this particular issue is killing me - the proposed solution would fix my issue but i certainly see how this change could cause unexpected (negative) changes to sites that have build around this restriction/bug.

Are there any updates?

Thanks,

Carl

NOTE - I was able to work around this issue by replacing the comment field with custom HTML that included my input fields.

        $commentField = '<p class="comment-form-comment"><label for="comment">' . _x( 'Comment', 'noun' ) . '</label> <textarea id="comment" name="comment" cols="45" rows="8"  aria-required="true" required="required"></textarea></p>'. addMySpecialStuff($post->ID);
        $comment_args = array(
            'comment_notes_after'=>'',
            'logged_in_as'=>'',
            'title_reply'=>'',
            'comment_field'        =>$commentField,
        );

where the addMySpecialStuff function was simply appending the HTML I wanted after the comment field and the submit button.

Last edited 10 years ago by CarlSteffen (previous) (diff)

#10 @swissspidy
10 years ago

  • Keywords needs-refresh added

This ticket was mentioned in Slack in #core-comments by rachelbaker. View the logs.


10 years ago

#12 @rachelbaker
10 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release

16576.diff needs testing to determine if there is any BC breakage here. Otherwise +1.

#15 @kushsharma
7 years ago

  • Keywords bulk-reopened added

This ticket was opened 8 years ago and I can confirm this is still reproducible.

#16 @abcd95
7 months ago

Reproduction Report

Description

This report validates that the issue can still be reproduced.

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.25
  • Server: nginx/1.27.2
  • Database: mysqli (Server: 8.4.4 / Client: mysqlnd 8.2.25)
  • Browser: Chrome 138.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.3

Actual Results

✅ Error condition occurs (reproduced).

Additional Notes

  • I am working on refreshing the patch according to the current trunk.

Supplemental Artifacts

I added a new field, Location, which was not visible to the logged-in user, but the logged-out user was able to see all the fields -

Logged-in user - https://i.ibb.co/wFDHP36k/Screenshot-2025-07-25-at-19-50-47.png

Logged-out user - https://i.ibb.co/PGvzp3vw/Screenshot-2025-07-25-at-19-51-33.png

This ticket was mentioned in PR #9343 on WordPress/wordpress-develop by @abcd95.


7 months ago
#17

  • Keywords needs-refresh removed

Trac ticket: https://core.trac.wordpress.org/ticket/16576

This PR fixes the issue where custom comment form fields added via the fields parameter are only visible to non-logged-in users. Logged-in users should be able to use custom comment fields. Currently, only default WordPress fields (author, email, and url) are hidden for logged-in users since this data comes from their profile, but custom fields should remain visible.

This PR modifies the field processing logic in comment_form() to show custom fields to both logged-in and non-logged-in users, while continuing to hide default WordPress fields for logged-in users.

#### Testing Instructions

  1. Add custom fields to the comment form using the fields parameter
  2. Test as both a logged-in and a non-logged-in user
  3. Verify custom fields appear for both user types
  4. Verify that default fields (author, email, url) only appear for non-logged-in users

#18 @abcd95
7 months ago

Thanks, everybody, for working on this.

This issue was long due to be fixed. I refreshed the patch and attempted to incorporate the latest changes on trunk for this fix, and tried to keep the fix as simple as possible.

I will also add the tests shortly to make sure any regressions hereafter are captured.

#19 @iamadisingh
7 months ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/9343

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.3
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 138.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.3
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • Using comment_form_default_fields filter to add a "Location" field to the comment form:
    <?php
    
    add_filter( 'comment_form_default_fields', function( $fields ) {
        $fields['Location'] = '<p class="comment-form-location">' .
            '<label for="location">' . __( 'Location' ) . '</label>' .
            '<input id="location" name="location" type="text" value="" size="30" /></p>';
        return $fields;
    });
    
    
  • Tested as Non-Logged-In User: The "Location" field appeared in the comment form as expected.
  • Tested as Logged-In User: The "Location" field also appeared for logged-in users.

Supplemental Artifacts

For Loged In Users: https://ibb.co/ZpzfFSPc
For Loged Out Users: https://ibb.co/vCHhPpfQ

#20 @oglekler
6 months ago

I don't much like the idea of duplicating fields in the array. I wonder if $original_fields = $fields; can be added before $fields = apply_filters( 'comment_form_default_fields', $fields ); and ! isset( $original_fields[ $name ] ) be used instead of ! in_array( $name, array( 'author', 'email', 'url', 'cookies' ), true )

@davidbaumwald can you please take a look at this? It looks quite straightforward to proceed.

#21 follow-up: @davidbaumwald
6 months ago

  • Keywords needs-testing removed
  • Milestone set to 6.9
  • Owner set to davidbaumwald
  • Status changed from new to reviewing

@oglekler Good call! This seems like a low-effort QOL fix that would render custom fields.

The first thing I wondered about was if we're setting an un-filterable list of default fields we always want to not show when a user is logged in, what about custom fields that a developer might also want to hide.

However, I think that the comment_form_fields filter right above the loop could easily be adapted to unset any other keys necessary.

#22 in reply to: ↑ 21 @abcd95
5 months ago

the comment_form_fields filter right above the loop could easily be adapted to unset any other keys necessary.

Yes, it's flexible that way. Also, @oglekler's suggestion looks a lot cleaner, and so that patch now follows that.

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


4 months ago

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


4 months ago

#25 @welcher
4 months ago

  • Milestone changed from 6.9 to 7.0

6.9 Beta 1 is tomorrow and there hasn't been movement on this. I'm going to move it to the 7.0 Milestone

#26 @davidbaumwald
4 months ago

  • Keywords commit added
  • Milestone changed from 7.0 to 6.9

#27 @wildworks
4 months ago

@davidbaumwald I noticed you changed the milestone to 6.9. This ticket is labeled as an enhancement, but do we need to do anything before 6.9 Beta1?

#28 @davidbaumwald
4 months ago

@wildworks I am working on committing this now. This was on my list to get in before Beta 1, but I was unable to attend the scrub yesterday when it was punted.

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


4 months ago

#30 @davidbaumwald
4 months ago

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

In 61034:

Comments: Ensure custom comment form fields appear for logged-in users

When a user is logged in, only the default comment textarea is shown by
the core comment_form() implementation, but custom fields supplied via the
fields argument are omitted. This mismatch means plugin- and theme-added fields
aren't visible to logged-in users, though they are visible to guests.

This change fixes this by moving the loop over $args['fields'] inside
comment_form(), so that extra fields are rendered, regardless of login status.

Props maorb, valendesigns, CarlSteffen, swissspidy, rachelbaker, kushsharma, abcd95, iamadisingh, oglekler, welcher.
Fixes #16576.

@abcd95 commented on PR #9343:


3 months ago
#31

Committed in [61034]

Note: See TracTickets for help on using tickets.