Comment text not marked as required

[infected]

I thought i already opened a ticket to this but i cant find it... Maybe i didnt hit the create ticket button?!

When using comment_form() the name and email fields marked as required while the textarea for commenttext is not. If i send my comment without text i'll get this notice

Error: please type a comment.

So it also should be marked as required field.

[dd32]

Sounds like a good idea, Kind of should be consistent in the forms :)

[solarissmoke]

Mark comment field as required

[nacin]

Looks like maybe my comment either got stopped by dd32's, or I was commenting to the other phantom ticket. Looks like the former.

Anyway, I don't mind adding aria-required, but the * seems just redundant and silly. Mentioned it to MarkJaquith and a few others and I seemed to recall them nodding in agreement on that.

[nacin]

Ah, comment text already has aria-required.

[infected]

My opinion: Either the field is required (the error notice is saying that) then the * isn´t redundant, otherwise it isn´t a required field. Maybe the code already has an aria-required but visitors don´t look into the code for required fields. ;-)

[SergeyBiryukov]

Replying to infected:

Maybe the code already has an aria-required but visitors don´t look into the code for required fields. ;-)

I guess in the case of a comment text that's just implied by common sense.

[infected]

Replying to SergeyBiryukov:

I guess in the case of a comment text that's just implied by common sense.

From this point of view, yes. It's just a cosmetic thing...

[MikeHansenMe]

Replying to SergeyBiryukov:

Replying to infected:

Maybe the code already has an aria-required but visitors don´t look into the code for required fields. ;-)

I guess in the case of a comment text that's just implied by common sense.

I would agree that the comment field is common sense. Proposing close

[ocean90]

#25384 was marked as a duplicate.

[nofearinc]

Based on reports from confused newbie clients, I'd also vote +1 for adding the asterisk for the comment field.

[SergeyBiryukov]

#34665 was marked as a duplicate.

[ocean90]

#35515 was marked as a duplicate.

[rianrietveld]

I'm reopening this ticket to discuss this further from an accessibility point of view.
Required fields need to be indicated, also visually.
I agree that it's obvious, but if you use an asterisk do it consistent.

[rianrietveld]

adds * to label comment textarea

[afercia]

I'd agree, since it uses also aria-required="true" required="required" then for consistency should use an asterisk too. By the way the sentence "Required fields are marked *" appears only when users are logged out. When logged in there's just the textarea. I guess this is OK, I'd just like to discuss it a bit in the next accessibility weekly meeting.

Worth noting required is a boolean attribute and doesn't need a value.

[ocean90]

By the way the sentence "Required fields are marked *" appears only when users are logged out. When logged in there's just the textarea. I guess this is OK

I thought we're adding the asterisk for consistency? Showing no help text feels inconsistent, especially for sites where only logged in users can comment.

[afercia]

Hm, maybe :) Was thinking when logged in it's the only field available so it's a "use it or don't". Saying "this unique field is required" would sound a little pedantic :) Then realized plugins and themes can add additional fields so maybe better always use an asterisk.

[rianrietveld]

Updated the patch after discussion with @joedolson and @afercia in Slack:

Added to the defaults array:

'comment_notes_logged_in_before' => '<p class="comment-notes">'. $required_text . '</p>',

Added

echo $args['comment_notes_logged_in_before'];

to the form if is_user_logged_in()

If a user is logged in the message shows: Required fields are marked *
If a user is not logged in the message shows: Your email address will not be published. Required fields are marked *

Also changed required="required" into required (​http://www.w3.org/WAI/GL/wiki/Techniques/HTML5/Using_the_required_attribute_to_indicate_a_required_input_field)

[rianrietveld]

adds comment_notes_logged_in_before

[strider72]

Replying to rianrietveld:

Also changed required="required" into required (​http://www.w3.org/WAI/GL/wiki/Techniques/HTML5/Using_the_required_attribute_to_indicate_a_required_input_field)

I know it's all HTML5 technically, but aren't we generally using the XHTML standard for attributes?

[afercia]

Replying to strider72:

I know it's all HTML5 technically, but aren't we generally using the XHTML standard for attributes?

Hm, not sure but interesting. Do you have any reference or argumentation to stick with the old syntax?

[strider72]

Replying to afercia:

Replying to strider72:

I know it's all HTML5 technically, but aren't we generally using the XHTML standard for attributes?

Hm, not sure but interesting. Do you have any reference or argumentation to stick with the old syntax?

Yes. The WordPress Core Handbook section on HTML Coding Standards specifically references XHTML standards:
​https://make.wordpress.org/core/handbook/best-practices/coding-standards/html/

Also, this is the way we've actually been doing it.

[strider72]

Replying to strider72:

Replying to afercia:

Replying to strider72:

I know it's all HTML5 technically, but aren't we generally using the XHTML standard for attributes?

Hm, not sure but interesting. Do you have any reference or argumentation to stick with the old syntax?

Yes. The WordPress Core Handbook section on HTML Coding Standards specifically references XHTML standards:
​https://make.wordpress.org/core/handbook/best-practices/coding-standards/html/

Or if you mean other than "That's just the way we do it"... well, I do think code consistency is a good thing, but I also think XHTML syntax is much cleaner and easier to read. The same with the closing slash on IMG tags — not technically necessary on HTML5, but consistently used it makes the code just a bit easier to read (and easier for more parsers to parse). There are a number of long-established coding standards within WordPress that are not technically necessary, such as spaces in parentheses in PHP, e.g.: ( $this ) not ($this)

[afercia]

Oh well, sorry I was focused only on the admin screens (which are HTML5). Since this is something output also on the front end for themes yep we shouldn't assume HTML5.

[rianrietveld]

restores required="required"

[afercia]

Thinking maybe it is possible to simplify a bit. Since now the Comment textarea is always indicated as required, the "required text" should always be printed out regardless of the require_name_email option (in the backend is "Comment author must fill out name and email "). So when logged in maybe just append the required text to the logged_in_as string? See screenshot below:

Additionally, the required attribute can use the HTML5 syntax depending on the support for HTML5.

Lastly, I'd propose to change the error message for missing required fields:

since we shouldn't assume they're just "name" and "email".

Thoughts?

[rianrietveld]

Replying to afercia:

Thinking maybe it is possible to simplify a bit. Since now the Comment textarea is always indicated as required, the "required text" should always be printed out regardless of the require_name_email option (in the backend is "Comment author must fill out name and email "). So when logged in maybe just append the required text to the logged_in_as string?

Is adding the "required text" default not conflicting with already made changes in the filters by theme developers?

Additionally, the required attribute can use the HTML5 syntax depending on the support for HTML5.

Is this an HTML5/XHTML/HTML4 compliance issue or a code standards compliance issue?
If it's a code standards compliance issue we should keep required="required" in whatever the theme is build in.

Lastly, I'd propose to change the error message for missing required fields:
since we shouldn't assume they're just "name" and "email".

Agree on that and think we should address this in a separate ticket.

[rachelbaker]

@rianrietveld @afercia is there an attainable path forward here for 4.5? Or should this ticket be punted?

[rianrietveld]

This started easy and got more and more complicated, I’m for punting it and look better at it in the next release.

[rianrietveld]

related: #37331

[rianrietveld]

Discussed this in the wpa11y bug scrub:
We'll have a go infixing this for 4.7. together with #37331

[SergeyBiryukov]

Related: #23870

[afercia]

Not enough time to discuss and establish a convention for all the required form fields in core, punting for now.

[afercia]

Just tested a bit using:
Safari 10, VoiceOver 7, on macOS Sierra
Firefox 50, NVDA 2016.3, on Windows 10
IE 11, JAWS 17, on Windows 10
and the "asterisk" is always announced as star. Also, worth noting since this character is actually part of the label, it will be announced both when reading out the label and when focusing the related field. For example, when you tab and focus the Email field, screen readers will announce Email star ... which is a bit annoying:

[joedolson]

It's worth noting that if the fields are all marked as aria-required=true, then the asterisk is only a visual indicator, and could be marked as aria-hidden.

The problem with that, obviously, is that then the sentence stating that required fields are marked with an asterisk will be confusing to screen reader users, since there would be no evidence of that asterisk in the required fields.

[JG Visual]

Has there been any movement on this? From a usability standpoint it seems odd to say "Required fields are marked *", but then leave the asterisk off of the required "Comment" field.

[tobifjellner]

If the star/asterisk is hidden for screen readers, then I'd vote for also hiding the text "Required fields are marked...". I mean if those fields anyway are marked as mandatory via aria parameters...

[sabernhardt]

16206.6.patch​ hides the required field message from screen readers with aria-hidden.

The patch also uses a $required_glyph variable for easier customization in #23870. This results in an aria-hidden span inside the hidden field message span, but the redundancy is probably better than trying to make a special case.

[sabernhardt]

correction: comments field needs to be required for everyone

[sabernhardt]

The $html_req variable can be an empty string (in the latest patches), so it is not appropriate in the comments textarea.

[sabernhardt]

editing variable names, defining HTML5 attributes together (both checked and required)

[sabernhardt]

I polished the patch a little:

• Changing variable names (for one, $required_indicator includes more than the asterisk glyph itself)
• Defining HTML5 attributes together, both checked and required, and then verifying within the sprintf function whether the required attribute belongs on that form field
• Paying more attention to spaces, using the trim function to avoid two spaces in the required note

[mai21]

Test Report

Env

• WordPress (5.9-alpha-51877) (could reproduce it and latest patch https://core.trac.wordpress.org/attachment/ticket/16206/16206.8.patch fixed it )
• Chrome 94.0.4606.54
• ubuntu 18.04.6
• Theme: Twenty Twenty One
• Plugin: WordPress Beta Tester

Steps to test

1. Open post incognito
2. Fill mandatory fields and submit

Actual:

An error displayed for comment txt field also it wasn't having *

Expected:

Make Comment Field mandatory in UI (having *)

Screencast after fix:

​https://jmp.sh/we7sbGT

[mai21]

Added needs-testing for automation tests

[rachelbaker]

@mai21 Can you elaborate on the automated tests needed on the 16206.8.patch​?

[mai21]

Replying to rachelbaker:

@mai21 Can you elaborate on the automated tests needed on the 16206.8.patch​?

Hello @rachelbaker, we can add in e2e tests that the * is displayed in UI for this field.

what do you think?

[audrasjb]

[silent 5.9 bugscrub]

Looking at the last patch, I'm not sure this is something where we really need an automated test. It wouldn't hurt of course (if we can define which specific tests are needed), but it would be worst if the patch can't ship at all :)

Anyway, I tested 16206.8.patch: it still applies cleanly on my side.

[audrasjb]

As per today's bug scrub, the last patch looks ready to go.
Marking for commit.

[sabernhardt]

The dev note for this can be included within a larger dev note for themes. Some themes might have a special style for the asterisks (Twenty Eleven is one of them).

[sabernhardt]

required note is missing when logged in (while label is marked with asterisk)

[sabernhardt]

fix missing message in default 'logged_in_as' argument

[sabernhardt]

alternative option: fix discrepancy with Comment label when name and email are required

[sabernhardt]

16206.9.patch​ adds the required fields message that was missing for logged-in users with most themes in 16206.8.patch.

At least ​437 themes—including Twenty Nineteen and Twenty Twenty-One—would still have the star inside the Comment label without the explanatory text because they edit/remove the 'logged_in_as' argument. And other themes edit the ​'comment_notes_before' text.

So I also uploaded a simpler option 16206.alternative.patch​ to focus on the form when users are logged out and are required to fill in their name and email. If we go in this direction for now, we still could consider a method to show the symbol and required message for everyone later.

Twenty Seventeen and other themes, before the patch

Logged out, name & email are not required:

• default 'comment_notes_before' text: Your email address will not be published.
• No fields have * symbol

Logged out, name & email required:

• default 'comment_notes_before' text: Your email address will not be published. Required fields are marked *
• Only Name and Email have * symbol, Comment and URL are available

Logged in (whether or not name & email are required):

• default 'logged_in_as' text: Logged in as test. Log out?
• No fields have * symbol (only Comment is available)

With 16206.alternative.patch

This patch adds the symbol to the Comment label when user is not logged in and their name ans email are required.

Logged out, name & email are not required:

• default 'comment_notes_before' text: Your email address will not be published.
• No fields have * symbol

Logged out, name & email required:

• default 'comment_notes_before' text: Your email address will not be published. Required fields are marked *
• Comment, Name and Email have * symbol

Logged in (whether or not name & email are required):

• default 'logged_in_as' text: Logged in as test. Log out?
• No fields have * symbol (only Comment is available)

With 16206.9.patch

The Comments label has symbol, regardless of require_name_email setting or user login status. The default 'comment_notes_before' and 'logged_in_as' text arguments include the required message.

Logged out, name & email are not required:

• default 'comment_notes_before' text: Your email address will not be published. Required fields are marked *
• Comment is the only label with * symbol

Logged out, name & email required:

• default 'comment_notes_before' text: Your email address will not be published. Required fields are marked *
• Comment, Name and Email have * symbol

Logged in (whether or not name & email are required):

• default 'logged_in_as' text: Logged in as test. Log out? Required fields are marked *
• Comment label has * symbol, and that is the only default field.

[joedolson]

Following review, I think that 16206.9.patch is the better choice. While it's not perfect, since the asterisk on Twenty Twenty One or in similar cases is not explained in visible text, it's better than it currently is - where the field is marked required in code, but excluded in the error message and has no marking of any kind that's visible.

[joedolson]

In 52029:

Comments: Mark comment text field as required.

Add required asterisk to the comment text field. Historically, the name and email fields are marked as required, but the comment text field is not, though it is actually a required field.

Props infected, solarissmoke, rianrietveld, afercia, sabernhardt, strider72, mai21, audrasjb.
Fixes #16206.

[sabernhardt]

Follow-up tickets:
#54392 - logged_in_as message for Twenty Nineteen and Twenty Twenty-One
#54408 - styling changes for Twenty Eleven
#54394 - add functions to re-use the indicator and message in other situations

[joedolson]

In 52149:

Bundled Theme: Display required text field in core themes.

Fix Twenty Nineteen and Twenty Twenty-One setting the logged_in_as parameter to null, which also eliminated the required field text. See #16206.

Props sabernhardt, poena.
Fixes #54392.

[sabernhardt]

class name change

[sabernhardt]

Reopening to consider changing the new comment-required-message class for better reusability (#54394):

A required-field-message class seems safe. The ​three plugin directory examples for 'required-field-message' use it for input field names, not as a class. And the directory search had ​no theme results.

[joedolson]

For future planning, I think this is a good idea. It's harmless if we never implement a message function, but not doing it would make that implementation significantly harder.

[joedolson]

In 52200:

Comments: Change new comment required text class.

Change the new class on comment required text from comment-required-message to required-field-message for better future-compatibility, enabling better reuse.

Props sabernhardt.
Fixes #16206.

[audrasjb]

Dev note published: ​https://make.wordpress.org/core/2022/01/04/miscellaneous-core-changes-in-wordpress-5-9/

[sabernhardt]

Additional note for changes to bundled themes:
​https://make.wordpress.org/core/2022/01/05/bundled-theme-updates-with-wordpress-5-9/