backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

[A11Y] Properly indicate the "required" status of a form input #5669

Open indigoxela opened 2 years ago

indigoxela commented 2 years ago

Description of the bug

Currently Backdrop indicates required status of a form item:

However, that's insufficient re web accessibility, when it comes to WCAG.

A * on its own means nothing, "title" attributes won't get read aloud by screen readers (at least in their default setting). EDIT wait, it's an abbr tag, so it would get read, but in an odd and distracting way.

What we need instead:

Additional information

Affected functions: _form_set_class(), theme_form_required_marker().

See also: https://www.w3.org/WAI/WCAG21/quickref/?showtechniques=413#input-assistance

indigoxela commented 2 years ago

A PR exists, but it needs more work. In particular radio groups (API type radios) need extra care (role="radiogroup"). And also checkboxes need a little more consideration.

indigoxela commented 2 years ago

Here we go, the PR is now ready for testing and review.

Some remarks: As we now add the "required" attribute to form elements, that support it, the form_required_marker may seem redundant, but I don't think it is. (One nice side effect of the "required" attribute is that most clients won't even send the form, as long as such a required item is still empty.)

The rewrite of the form_required_marker may seem odd, but the asterisk, when read aloud by a screen reader, doesn't "say" anything. And the "long form of the abbreviation" isn't read by default (AFAIK). That's why I decided to completely "hide" it with aria-hidden, and provide the full and proper text as visually hidden alternative.

One thing turned out to be trickier than expected: groups of checkboxes - of which at least one has to get checked, when required. No idea, how to solve this in an accessible way. Didn't find much helpful stuff in the net, either. For such checkboxes the browser puts out something like "Please tick these boxes to continue" when trying to submit the form - not ideal, but not worse, either.

indigoxela commented 1 year ago

PR rebased, still asking for testing and review. :wink:

indigoxela commented 1 year ago

Another rebase. :wink:

@yorkshire-pudding you seem to wonder why I use required="required" - in this case I took D9 as model: https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/lib/Drupal/Core/Render/Element/RenderElement.php#L144

But I guess, we can do that differently. Not sure, what the policies are for emtpy attributes in B.

yorkshire-pudding commented 1 year ago

I do find it odd, but having read further in the stackoverflow post I originally referenced, I note that this is part of the HTML5 standard.

Of the two, I think the empty is clearer, but I don't think it should block this.

I note that webform uses both versions. required="required" for those required and non-conditional; required for conditional required. ??

I've therefore withdrawn my concern around this.

quicksketch commented 1 year ago

Thanks @indigoxela and @yorkshire-pudding! I'd like to give it more extensive review. This looks like it could use additions to automated testing. I see that the label checking was updated, but we should check for the new attribute being printed out on each element as well.

One area of concern: Adding the required attribute to an HTML element will normally prevent the form from being submitted by the browser until the field is filled. This doesn't come up in our automated testing because it's not a real browser. Unless we also add a form-level novalidate attribute, I think the browser-based validation will start taking effect. The built-in browser form validation can cause all kinds of new accessibility issues, like required fields in vertical tabs being completely invisible when you attempt to submit a form; the form just completely silently fails to be submitted.

indigoxela commented 1 year ago

Sure, extending the functional tests a bit makes sense. :+1:

The built-in browser form validation can cause all kinds of new accessibility issues

Actually, it's supposed to fix accessibility issues by preventing useless form submission, when we already know it will fail.

... like required fields in vertical tabs being completely invisible when you attempt to submit a form

To my understanding vertical tabs should never contain required fields. Correct me if I'm wrong, though. AFAIK the purpose of vertical tabs is to hide away things that can safely be ignored, no?

bugfolder commented 1 year ago

To my understanding vertical tabs should never contain required fields. Correct me if I'm wrong, though. AFAIK the purpose of vertical tabs is to hide away things that can safely be ignored, no?

I think you're right, but there are certainly legacy contrib modules that came over from D7 that have required fields in vertical tabs (I'm looking at you, Ubercart).

indigoxela commented 1 year ago

@bugfolder Oh, didn't know that. (Nasty Ubercart! :grin:)

However, if it's a required field and has no default value, the validation will fail anyway. The only difference is that with a required attribute the browser will refuse to submit and without it the form gets submitted and the server-side form validation fails (page loads, error message shown...).

It's possible, though, that I just miss the point of @quicksketch's concern.