Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#55717 closed defect (bug) (fixed)

aria-hidden="true" attribute in comment template for visible text (required fields)

Reported by: juliemoynat's profile juliemoynat Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 5.8
Component: Comments Keywords: has-patch required-fields has-testing-info has-dev-note
Focuses: accessibility Cc:

Description

Hi,

In the comment form, the aria-hidden="true" attribute is used on the sentence explaining which are the required fields and on the tag around the asterisk (*) for each required field.

I think it's not correct because, as you can see it in this survey of WebAIM, blind people are not the only ones who use a screen reader. So, if a sighted person wants to make their screen reader reads the text, it will not work.

I think the aria-hidden="true" attributes should simply be removed.

The code is in the wp-includes/comment-template.php file (you can see it in the documentation).

Thanks

Attachments (2)

55717.png (110.7 KB) - added by afercia 3 years ago.
55717.diff (931 bytes) - added by sabernhardt 2 years ago.
refreshing after r53888

Download all attachments as: .zip

Change History (29)

#1 @audrasjb
3 years ago

  • Version changed from 5.9.3 to 5.8

Introduced in [52029].

This ticket was mentioned in PR #2706 on WordPress/wordpress-develop by audrasjb.


3 years ago
#2

  • Keywords has-patch added

#3 @audrasjb
3 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to 6.1

Hello and thank you for opening this ticket,

The above PR removes two aria-hidden="true" attributes from the comment template.

This looks pretty good to me, but I'm adding 2nd-opinion keyword to get a second thought on this change.

I'm also moving this ticket to milestone 6.1.

#4 @audrasjb
3 years ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

#5 @joedolson
3 years ago

  • Keywords 2nd-opinion removed

I think that this is basically over-engineered for accessibility. The aria-hidden is presumably there to prevent double reporting of the information, but I don't think avoiding that is particularly important. If the information is there visually, it should also be findable audibly.

#6 @sabernhardt
3 years ago

  • Keywords required-fields added

For reference, the discussion about adding these attributes was in comments 44 to 51 on ticket:16206.

The patch on #54394 needed a refresh already, but this would be another change to make there.

Last edited 2 years ago by joedolson (previous) (diff)

#7 @juliemoynat
3 years ago

Ah! Thank you for the reference. I suspected that this was a relatively new problem so this is true.

If the asterisk/star is really a problem, I think that we could imagine to replace it by an explicite word like "(required)" but hiding visible text is definitely not a good solution.

#8 @afercia
3 years ago

If removing aria-hidden is the preferred option, then there are a few more things to take care of.

  • aria-hidden should be removed from the asterisk used for the form fields as well.
  • At that point, we should make sure assistive technologies announce the asterisk correctly. Some screen readers may not announce it at all or announce it as star.
  • The asterisk should be moved out from the field labels. Otherwise, it would be part of the accessible name of the form fields. See example in the screenshot below, where a field would be announced as"

Comment star, required, edit text

Last edited 3 years ago by afercia (previous) (diff)

@afercia
3 years ago

#9 @juliemoynat
3 years ago

I’ve discussed about this subject with an accessibility expert friend, Romain Gervois, and he has an idea which is interesting.

We could:

  • rephrase the "Required fields are marked *" sentence so that the symbol would be named clearly just like Required field are preceded by an asterisk <span aria-hidden="true">*</span>.
  • make the asterisk in the label an image with a clear label so that each screen reader will always read "asterisk": <label for="stuff">Name of the field <span role="img" aria-label="asterisk">*</span></label>
  • of course, the word "asterisk" must be the same translation string for the aria-label and for the sentence to prevent a different translation between both use of the word.

Finally, I (and he too) don't think it's really a problem to have the symbol read in the label but if it's really a problem and we put the asterisk out of the label, we would need to have an HTML container for the label + the asterisk to be able to style these elements correctly in CSS.

What do you think?

#10 @sabernhardt
3 years ago

Silencing only the (asterisk) symbol in both the labels and the message text would make sense in the original English. Required fields are marked: visually by the character or audibly by the required attribute. Unfortunately, French translations—plus Spanish and several other languages—add the word for with before the symbol. The extra word is probably necessary for at least one of the translations. If not, adding a period might help:
Required fields are marked.%s

(I removed the space from the suggestion above to consider using or allowing non-breaking spaces, which I wanted to do earlier but not enough to change the string.)

Also, moving the character outside the label would break any theme that sets the label elements to display: block. A directory search found some (many of these CSS results do not affect the comments form, but the search does not seem to include any multiline matches and it does not include any label styles that define another property before display).

#11 @juliemoynat
3 years ago

I don't think it makes sense to hide the symbol if the result is the total absence of vocalization of it. As I told it in my very first message, screen readers are not used by blind people only. This is not a decorative symbol: it makes sense.

About this and about the fact of having the asterisk outside from the label, I forgot an argument from Romain Gervois (and it seems that this idea can also be found in an article by Adrian Roselli): if you try to bring assistance to a blind person with this form and talk about an asterisk, it would not be understood/found. In this case, we can imagine that this is not a blocking point because we have the label but we can sometimes find anything and everything in themes.

We could also imagine to have this for the sentence at the top of the form:

Required field are marked <span role="img" aria-label="asterisk">*</span>.

The advantage of naming the character at first is that everybody will know that this is how it is named in code.

#12 @juliemoynat
3 years ago

@afercia I thought again about the subject and I'm wondering why do we have a problem with the asterisk being told a "star" by screen readers? It is told like this everywhere in the OS, each time there is the character. And the pronunciation can be changed in the screen reader settings if the user doesn't like it. Am I missing something?

#13 @afercia
3 years ago

@juliemoynat I'm not sure the real problem is whether screen readers announce that character as 'star' or 'asterisk'. I think the real problem is whether they announce it or not.

Screen readers do have verbosity settings. When the verbosity it set to low, some punctuation and other characters are not announced. That would produce again the problem that what is visible on the screen it's not spoken by screen readers.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

#15 @joedolson
2 years ago

I think that we should leave the asterisk unmarked up. While it is an information-bearing symbol, it's not unique information: screen readers will also learn that the fields are required because they are marked as required in the HTML. We shouldn't attempt to override user preferences for verbosity by forcing the symbol to be communicated regardless of the user's settings.

If a user has verbosity configured so that they hear punctuation, they will hear it. If they don't, they won't - and to me, that's the correct behavior, since the information in the punctuation isn't necessary to operate or understand the interface.

#16 @juliemoynat
2 years ago

I agree with Joe.

Regarding the whole discussion I still think removing the aria-hidden="true" attributes is the better solution. I think it's more important to be sure that no one will miss any information than having too much. The comment form is quite small and once people are used to this form, they can easily interrupt the screen reader.

@sabernhardt
2 years ago

refreshing after r53888

#17 @sabernhardt
2 years ago

#56389 proposes filters that site owners could use to change the markup (if they disagree with the decision here, either way).

If you apply both that patch and the one on this ticket, you could try this:

add_filter( 'wp_required_field_indicator', 'wptrac_add_aria_hidden_indicator', 10, 1 );
function wptrac_add_aria_hidden_indicator( $indicator ) {
	$indicator = str_replace( '<span class="required">', '<span class="required" aria-hidden="true">', $indicator );
	return $indicator;
}

add_filter( 'wp_required_field_message', 'wptrac_add_aria_hidden_message', 10, 1 );
function wptrac_add_aria_hidden_message( $message ) {
	$message = str_replace( '<span class="required-field-message">', '<span class="required-field-message" aria-hidden="true">', $message );
	return $message;
}
Version 0, edited 2 years ago by sabernhardt (next)

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

#20 @costdev
2 years ago

  • Keywords has-testing-info added

Test Report

Patch tested: https://core.trac.wordpress.org/attachment/ticket/55717/55717.diff

Steps to Reproduce or Test

  1. Navigate to a single post and examine the form for adding a new comment.
  2. 🐞 In DevTools, inspect the required indicator (by default, this is *).
  3. (when testing the patch) Run grunt patch:55717.

Expected Results

Before the patch:

  • ❌ The aria-hidden attribute is applied.

After the patch:

  • ✅ The aria-hidden attribute is not applied.

Environment

  • Server: Apache (Linux)
  • WordPress: 6.1-alpha-53344-src
  • Browser: Chrome 104.0.0.0
  • OS: Windows 10
  • Theme: Twenty Twenty-One
  • Plugins: None activated

Actual Results

Before the patch:

  • ❌ The aria-hidden attribute is applied. Issue reproduced.

After the patch:

  • ✅ The aria-hidden attribute is not applied. Issue resolved with patch.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 years ago

#22 @ryokuhi
2 years ago

  • Keywords commit added

After checking the ticket today during the weekly Accessibility Team's bug scrub, we're marking this ticket for commit.

#23 @audrasjb
2 years ago

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

In 54136:

Comments: Remove aria-hidden="true" attribute for visible text in comment template.

This changeset removes aria-hidden="true" attribute used for required fields in comments template, for better accessibility. It removes the attribute from both wp_required_field_indicator() and wp_required_field_message() patterns.

Follow-up to [53888].

Props juliemoynat, audrasjb, joedolson, sabernhardt, afercia, costdev.
Fixes #55717.

#25 @milana_cap
2 years ago

  • Keywords add-to-field-guide added

#26 @sabernhardt
2 years ago

  • Keywords commit removed

#27 @milana_cap
2 years ago

  • Keywords has-dev-note added; add-to-field-guide removed
Note: See TracTickets for help on using tickets.