Radios element missing "required" attribute

Created on 8 March 2018, over 6 years ago
Updated 24 June 2024, 2 days ago

Problem/Motivation

The radios elements that are provided by core do not apply the required attribute to the generated <input> elements. Instead the required attribute is applied to the wrapping <fieldset>.

There are two issues with this:

  1. screen readers use required attribute on <input type="radio">. If the attribute is not present on the , then the required state will not be communicated to the screen reader user.
  2. the required attribute is not valid when applied to a <fieldset>, thus causing the page to fail W3C validation.

Proposed resolution

  1. Remove the required and aria-required attributes from the <fieldset> of Form API radios elements.
  2. Add required attribute to the <input type=radio> elements instead.
  3. CSS updates, to ensure no visual regression in Seven and Bartik.

Remaining tasks

  • Remove required + and aria-required attributes from the <fieldset> element to the <input> element.
  • CSS to avoid visual regression. Asterisk should be after the fieldset legend, not the individual radio buttons.
  • Evaluate impact on Classy - Can the CSS go into Classy? - Classy is removed from D10 so may be non issue
  • Appear to have been added

Manual testing tips

We need a required radios element to test this, e.g. add a "favourite colour" field to a content type. Use a field of type "List", mark it required, and use the radios/checkboxes widget under "manage form display".

User interface changes

Markup + CSS changes to

  1. Fix invalid HTML
  2. Convey required radio buttons correctly to assistive tech.
  3. Avoid visual regression.

API changes

None.

Data model changes

None.

Original report by @mfairchild365

This issue is split off from #2950999: Checkboxes element missing "required" attribute โ†’ . That issue originally addressed radios and checkboxes, but it looks like they need different solutions, so we're using separate issues now.

๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Renderย  โ†’

Last updated 33 minutes ago

Created by

๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

Live updates comments and jobs are added and updated live.
  • Accessibility

    It affects the ability of people with disabilities or special needs (such as blindness or color-blindness) to use Drupal.

  • CSS

    It involves the content or handling of Cascading Style Sheets.

  • Needs accessibility review

    Used to alert the accessibility topic maintainer(s) that an issue significantly affects (or has the potential to affect) the accessibility of Drupal, and their signoff is needed (see the governance policy draft for more information). Useful links: Drupal's accessibility standards, the Drupal Core accessibility gate.

Missing content requested by

๐Ÿ‡ฆ๐Ÿ‡บAustralia dpi
7 months ago
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @andrewmacpherson
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson
  • I wasn't sure if I should unset the required attributes (required, aria-required, and aria-invalid) from all fieldsets, or just the fieldset for radios. The required attribute is technically invalid on all fieldsets.

  • The last submitted patch, 3: radios-a11y-required-fix.patch, failed testing. View results โ†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • The last commit had a file permission change that should not have beed added.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    Thanks for working on this @mfairchild365.

    Manual testing of patch #9:

    1. The required + aria-required attributes are no longer on the fieldset element. Good.
    2. Individual radio input elements get required and aria-required instead. Good.
    3. Screen readers correctly announce that the radio is required, when focused on a radio. Good. (Tested with Chrome 65 + ChromeVox, Win7 + NVDA + Chrome 65, Win7 + NVDA + IE11, Win7 + NVDA + Firefox 52 ESR.)
    4. Every radio gets a red asterisk. The patch doesn't touch CSS yet, so I was expecting this. Needs work.

    Code Review of patch #9. Looking good so far, with a few minor issues:

    1. diff --git a/core/includes/form.inc b/core/includes/form.inc
      index 052c9ce87a..05078d08bd 100644
      --- a/core/includes/form.inc
      +++ b/core/includes/form.inc
      @@ -222,6 +222,13 @@ function template_preprocess_fieldset(&$variables) {
           // Add the description's id to the fieldset aria attributes.
           $variables['attributes']['aria-describedby'] = $description_id;
         }
      +  
      +  //Remove invalid attributes from the fieldset for radio groups. These are applied to the child radio elements instead.
      

      The first line added here had some trailing spaces in it, which should be removed for coding standards.

    2. +  //Remove invalid attributes from the fieldset for radio groups. These are applied to the child radio elements instead.
      

      Use a space after the the double slash to start the comment, and split this into two lines, wrapping at 80 characters.
      I think this comment could be worded better. The word "invalid" sounds ambiguous here, because that's like the name of one of the attributes. Perhaps "Remove unnecessary attributes ..."?

    3. && 'radios' === $variables['element']['#type'])
      We don't generally see Yoda conditions in Drupal. Not actually part of our PHP coding standards yet, but being discussed in #2372543: [policy, no patch] Explicitly disallow yoda conditions โ†’ . Until that's resolved it's probably best to stick to the non-Yoda way.
    4. +    unset($variables['attributes']['aria-invalid']);
      

      Good catch. I think this is appropriate. AFAIK aria-invalid doesn't work on fieldset elements.

    TODO: update CSS to fix the issue of too many asterisks. This comes from form.cssin Classy theme, so let's try updating that, to avoid adding the asterisk it to required radios which are inside fieldsets (not sure how specific it will need to be). The framework maintainers might say no to changing Classy, in which case we can put separate overrides in Bartik and Seven. But let's try updating Classy CSS first for simplicity.

  • Thank you for the review @andrewmacpherson

    A new patch is attached which addresses all of your concerns.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    @mfairchild365 - thanks for the patch in #11.

    A few tips for contributions generally:

    • Include a issue-number and comment-number in the patch filename, to help reviewers be sure which one version they apply at the command line. Something like "2951317-11.patch". It really helps where issues go on beyond a few issues.
    • Have you tried making interdiffs yet? This patch is small so it's easy to follow, but for larger patches, or issues where patches go though lots of changes, interdiffs help reviewers. It's a good thing to include, but a bit fiddly to learn. There's a guide at Creating an interdiff โ†’ . I've included one here. It shows how the comment wrapping was fixed, for example.
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    Review of patch #11. Good progress!

    • Fixes code + comment standards issues noted in #10, great.
    • Addresses the CSS TODO from #10. It works, so the asterisk appears after the fieldset legend, but not the individual radio inputs.
      +.form-radios .form-required:after {
      +  display: none;
      +}
        

      The .form-radios targets the div wrapper which around the radio options, but inside the fieldset.
      The .form-required class targets the individual labels inside the radios set.

    The CSS works, and targets FAPI #radios only, so I think this would be fine if we put it in the CSS for Bartik and Seven themes.

    But is it safe enough to include in the Classy theme, so that lots of themes benefit? This issue corrects an accessibility problem in markup at the form rendering level, but needs some CSS to avoid a visual regression in any theme inheriting form Classy. I think a more experienced CSS/front-end contributor should answer this better than I can. Leaving at needs review, tagging for others to find.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    A good one to work on at the Nashville sprints. Novices could try

    - Get screenshots form Bartik + Seven, confirm no visual regression
    - Get an assessment of whether this is safe enough to include in the Classy theme, to answer my question in the previous comment. Maybe track down a front-end framework manager and ask them in person :-)
    - Write a functional test for the markup.

  • Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule โ†’ and the Allowed changes during the Drupal 8 release cycle โ†’ .

  • ๐Ÿ‡ฆ๐Ÿ‡ทArgentina matias

    Manual tested #11 and it looks good on Bartik and Seven

  • ๐Ÿ‡ซ๐Ÿ‡ทFrance andypost

    needs screenshots to compare before-after! #16 provides only after ones, so to be sure we need initial state

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine KondratievaS

    I have tested patch from comment #11 and result is OK

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands Edwin Knol

    Reviewing from Drupal europe 2018 mentored by @mmbk

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands Edwin Knol

    @mfairchild365 While reviewing the code i noticed the following function in form.inc.

    if (isset($variables['element']['#type']) && $variables['element']['#type'] === 'radios') {
      unset($variables['attributes']['required']);
      unset($variables['attributes']['aria-required']);
      unset($variables['attributes']['aria-invalid']);
    }
    

    We could also choose to not set the attributes instead of unsetting them by overwriting setAttributes() in the class Radios.
    I will try this to see if this could work and if so apply a patch.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany mmbk MeiรŸen

    @edwin-knol thank you for your work here, for me the result looks good.

    I just wonder whether we need some tests here, if not I think it's RTBC

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands Edwin Knol

    @mmbk Thanks for your support at Drupal Europe.

    What kind of test do you mean ?
    User testing or automated test.
    Should i be the one to put the issue on RTBC or should this be done by another Community user ?

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands Edwin Knol

    Added Patch let me know what you think.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany mmbk MeiรŸen

    What kind of test do you mean ?
    User testing or automated test.

    I mean automated tests, they were already suggested at #15 ๐Ÿ› Radios element missing "required" attribute Needs work

    Should i be the one to put the issue on RTBC

    No definately not, I'ld like to have a review by a more experienced contributor

  • Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule โ†’ and the Allowed changes during the Drupal 8 release cycle โ†’ .

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands Edwin Knol

    @mmbk I'm not able to find the time to create a test for the markup right now.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia 2pha

    Just an FYI for anyone else running into this issue...
    You can override the preprocess of the radiobuttons with `MYTHEME_element_info_alter(array &$info)`.
    This is the way I had to do it on a project (which required major accessibility) and it seems to work.
    eg.

    function MYTHEME_element_info_alter(array &$info) {
      $info['radios']['#pre_render'] = ['custom_prerender_radios'];
    }
    
    function custom_prerender_radios($element) {
      // Set the element's title attribute to show #title as a tooltip, if needed.
      // This first part is copied directly from CompositeFormElementTrait.php
      if (isset($element['#title']) && $element['#title_display'] == 'attribute') {
        $element['#attributes']['title'] = $element['#title'];
        if (!empty($element['#required'])) {
    
          // Append an indication that this field is required.
          $element['#attributes']['title'] .= ' (' . t('Required') . ')';
        }
      }
      if (isset($element['#title']) || isset($element['#description'])) {
    
        // @see #type 'fieldgroup'
        $element['#attributes']['id'] = $element['#id'] . '--wrapper';
        $element['#theme_wrappers'][] = 'fieldset';
        $element['#attributes']['class'][] = 'fieldgroup';
        $element['#attributes']['class'][] = 'form-composite';
        $element['#attributes']['class'][] = 'form-composite-radios';
      }
    
      // Move required to the actual radio buttons.
      if($element['#required']) {
        foreach($element['#options'] as $key => $value) {
          $element[$key]['#required'] = TRUE;
        }
        // Remove required from the fieldset.
        unset($element['#required']);
      }
      return $element;
    }
    

    It seems to me this issue should be garnering a lot more attention as no only is it bad for accessibility and screen readers, but it also produces invalid html (required on a fieldset).

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Mile23 Seattle, WA

    Reroll #22 for 8.8.x.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Mile23 Seattle, WA

    This is not so good a solution because it yields a required marker on all the elements the user could choose from. That could be confusing.

    Edit: On second thought, this might be our CSS.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia 2pha

    I thought I would chime in here again.
    After trying to validate my page, I am being flagged for having aria-required set on radio buttons.
    After searching around the interwebs, it seems that a radiobutton group that is required should have a default value.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jrockowitz Brooklyn, NY

    To move this patch along I added a test form to the form_test.module.

    Below shows the patch from #30 is working when navigating to /form-test/radios-required

    โ†’

    @2pha is right the aria-required attribute is not supported by radio buttons. I am not sure what the best approach is going to be to address this issue.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jrockowitz Brooklyn, NY

    The patch adds role="radiogroup" to the radios fieldset wrapper and removes the aria-required attribute from individual radio buttons.

    I think the next step is someone to confirm and verify the requirements. Once we know this is the right direction we can add some test coverage to \Drupal\Tests\system\Functional\Form\ElementTest

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States leraa

    For #33: ARIA radiogroup and radio example

    Backs up the role="radiogroup" for fieldset and also adds aria-labelledby

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States leraa

    I tested this patch and found correct:

    aria-required="true" is set on the fieldset
    role="radiogroup" is set on the fieldset

    However, I still saw aria-required="true" on the individual radio buttons.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jrockowitz Brooklyn, NY

    This patch should remove the aria-required.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States leraa

    This patch confirmed working as expected.

    Manual test steps:

    • aria-required="true" is set on the fieldset
    • role="radiogroup" is set on the fieldset
    • aria-required="true" is NOT on the individual radio buttons
    • required="required" is on the individual radio buttons

    .
    Needs accessibility review.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jrockowitz Brooklyn, NY

    Patch now hash test coverage.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    @2pha - re. #31:

    After trying to validate my page, I am being flagged for having aria-required set on radio buttons.

    Good catch, thanks for reporting this.

    After searching around the interwebs, it seems that a radiobutton group that is required should have a default value

    This is NOT the case as far as I know. The WAI-ARIA recommendation (version 1.0 or 1.1) certainly doesn't state this. The HTML 5.2 recommendation is clear that it is permitted to have a group of radio buttons where none of them is in the checked state (emphasis mine):

    Constraint validation: If an element in the radio button group is required, and all of the input elements in the radio button group have a checkedness that is false, then the element is suffering from being missing.

    If none of the radio buttons in a radio button group are checked when they are inserted into the document, then they will all be initially unchecked in the interface, until such time as one of them is checked (either by the user or by script).

    The part about "suffering from being missing" is relevant at validation time, but it doesn't mean that there must be a checked radiobutton. when the form is first rendered.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    Thanks for working on this everyone. This issue has taken some twists and turns, and isn't quite right yet. It's had some scope creep, and misunderstandings. Fiddly!

    Firstly, my original instructions were faulty. I said we needed to put an aria-required attribute on the <input type"radio"> element. That turned out to be incorrect - @2pha and @jrockowitz caught it in #31-32. I've checked the ARIA recs, and I agree. The input element needs a required attribute, but not an aria-required attribute.

    Re. #33-#34:

    • <fieldset role="radiogroup"> is invalid. The HTML5.2 recommendation does not list radiogroup as a permitted role on the fieldset element.
    • The example linked in #34 doesn't backup the use of role="radiogroup" on a fieldset element; the example there is using the radiogroup role on a DIV.
    • I don't think we need a radiogroup role here at all. The purpose of that role is to identify which radio buttons form a group, so the accessibility tree can be correctly built. (Especially if the radios are something other than HTML input elements, like the <div role="radio"> in the example linked to).
    • With proper HTML radios, the radio button button group is computed from the name attribute on the input element.
    • Since the fieldset element isn't allowed a radiogroup role, it isn't allowed an aria-required attribute either. The default role for a fieldset is "group", for which aria-required isn't permitted. According to HTML5.2, the fieldset element is only allowed ARIA states and properties which would be allowed for the group or presentation roles.

    Upshot: the fieldset element shouldn't have the radiogroup role, or aria-required.

    Re. #37:
    The fieldset element has a required attribute in that screenshot, which isn't valid HTML.

    Testing patch #38:

    Before patching:

    • <fieldset> has aria-required="true". INCORRECT, not permitted on fieldset, remove it.
    • <fieldset> has required="required". INCORRECT, not permitted on fieldset, remove it.
    • <input type="radio"> doesn't have the aria-required="true" attribute. CORRECT, no need for it.
    • <input type="radio"> doesn't have the required="required" attribute. INCORRECT, we need to add this.

    After patch #38:

    • <fieldset> has role="radiogroup". INCORRECT, not permitted on fieldset per HTML, remove it.
    • <fieldset> has aria-required="true". INCORRECT, not permitted on fieldset, remove it.
    • <fieldset> has required="required". INCORRECT, not permitted on fieldsest,remove it.
    • <input type="radio"> doesn't have the aria-required="true" attribute. CORRECT, no need for it.
    • <input type="radio"> has the required="required" attribute. CORRECT.

    Needs work:

    1. Remove the role="radiogroup" attribute from the fieldset, because it's not a permitted role, per HTM.
    2. Remove the aria-required="true" attribute from the fieldset, because it's not permitted for the fieldset element's default role of group.
    3. Remove the required="required" attribute from the fieldset, because it's not valid HTML.
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    Updating proposed resolution and tasks.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia 2pha

    A problem I found when having a required attribute on each <input type="radio" is that screen readers would announce each individual radio button as required, which is confusing.
    I was / am working on a government website which requires extreme usability. I am testing on 4 different devices with 4 different screen readers. The only option I could find that kept them all happy was to set a default value.
    I think it is important that if these changes are going to happen, that it is tested with multiple screen readers and the outcomes are documented. This would probably just mean setting up a page somewhere with the proposed output and testing in JAWS / Talkback / Voiceover etc, then posting the results here.
    By testing, I mean validating the html and the expected screen reader announcements.
    I could do the testing if needed.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    #42 - I love the term "extreme usability" :-)

    It's the normal behaviour of most screen readers to announce each individual radio button as required. The important thing is their radio button role is announced, because that's what conveys the fact that it has a single-selection behaviour for the group. The "required" constraint applies to the group as a whole, so it's fine to repeat this for each radio button. (When you change from one radio option to another, you are still technically on the same control group as far as assistive tech sees it, even if they are separate inputs in HTML terms).

    It really isn't necessary to enforce a pre-selected default value for radio buttons. Most screen readers will announce them as "unselected, required" when they are first encountered. As soon as you select one explicitly using the spacebar (or maybe implicitly, using arrow keys; screen readers vary a bit in this) then they are announced as "selected, required" from that point onwards.

    I agree that it's important to test with multiple screen reader and browser combinations, and I regularly do. I'm not sure what you mean by "keep them all happy"; that's a bit vague. The experience with each screen reader doesn't have to be identical, and numerous quirks are to be found. They don't all announce fieldsets the same way, or list the number of options. Some announcements are user-configurable. The important thing is that all the necessary information is conveyed in the accessibility tree. There are some combinations of browser and screen reader which have bugs beyond our control, and I don't think it's worth jumping through hoops to satisfy uncommon choices. I usually test IE11, Edge, Firefox, Firefox ESR, Safari, and Chrome browsers, alongside JAWS, NVDA, Narrator, Talkback, and macOS VoiceOver. I could do with more practice with iOS VoiceOver/Safari, which is a popular combination; it has lots of reported quirks and bugs, and has been a bit unstable between different iOS versions.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    Code review of patch 38:

    1. diff --git a/core/lib/Drupal/Core/Render/Element/Radios.php b/core/lib/Drupal/Core/Render/Element/Radios.php
      index 2c9797a3c1..8413113df0 100644
      --- a/core/lib/Drupal/Core/Render/Element/Radios.php
      +++ b/core/lib/Drupal/Core/Render/Element/Radios.php
      @@ -32,6 +32,31 @@ class Radios extends FormElement {
       
         use CompositeFormElementTrait;
       
      +  /**
      +   * {@inheritdoc}
      +   *
      +   * Overwrite setAttributes to remove unnecessary attributes from the
      +   * fieldset for radio groups. These are applied to the child radio
      +   * elements instead.
      +   */
      +  public static function setAttributes(&$element, $class = []) {
      +    if (!empty($class)) {
      +      if (!isset($element['#attributes']['class'])) {
      +        $element['#attributes']['class'] = [];
      +      }
      +      $element['#attributes']['class'] = array_merge($element['#attributes']['class'], $class);
      +    }
      +    // This function is invoked from form element theme functions, but the
      +    // rendered form element may not necessarily have been processed by
      +    // \Drupal::formBuilder()->doBuildForm().
      +    if (!empty($element['#required'])) {
      +      $element['#attributes']['class'][] = 'required';
      +    }
      +    if (isset($element['#parents']) && isset($element['#errors']) && !empty($element['#validated'])) {
      +      $element['#attributes']['class'][] = 'error';
      +    }
      +  }
      +
      

      I think this is the right method to use, but the brute-force override is a bit risky from a long-term maintenance point of view. If RenderElement::setAttributes() ever changes, then Radio won't get the benefits, and we'd have to replicate some logic again in Radio::setAttributes().

      The only thing we're interested in changing is the way required and aria-required are used, not other attributes like class. It would be safer to override it more like this:

      /**
       * (@inheritdoc}
       */
      public static function setAttribute(&$element, $class = []) {
        parent::setAttribute($element, $class);
      
        // Unset the HTML required attribute which isn't permitted on <fieldset>.
        // ...
      }
      
    2. +        // Only add the required attribute to radio buttons.
      +        // @see https://stackoverflow.com/questions/34300154
      

      This could be phrased better. The important thing is that we're avoiding adding the aria-required attribute to the <input type="radio"> element, because it would be invalid HTML/ARIA.
      I don't rate that stackoverflow question/answer very highly; it has some misleading information, which I've commented about there. Instead, we can refer directly to the HTML5.2 rec, in section "3.2.8.4. Allowed ARIA roles, states and properties" (https://www.w3.org/TR/html52/dom.html#allowed-aria-roles-states-and-prop...).

    3. +  /**
      +   * Adds role "radiogroup" to radios fieldset wrapper.
      +   */
      +  public static function preRenderRadiosFormElement($element) {
      +    $element['#attributes']['role'] = 'radiogroup';
      +    return $element;
      +  }
      +
      

      This preRender callback is the part which produces the invalid <fieldset role="radiogroup">. It can be removed.

    4. +    $elements = $this->xpath('//fieldset[@id="edit-radios-required--wrapper" and @aria-required="true"]');
      +    $this->assertCount(1, $elements)
      

      This is correct. I wonder if we should also assert that the fieldset element doesn't have the (invalid) HTML required attribute?

    5. +    $elements = $this->xpath('//fieldset[@id="edit-radios-required--wrapper" and @role="radiogroup"]');
      +    $this->assertCount(1, $elements);
      

      The assertion about the radiogroup role won't be needed.

    6. +    $elements = $this->xpath('//input[@id="edit-radios-required-0" and @required="required"]');
      +    $this->assertCount(1, $elements);
      +    $elements = $this->xpath('//input[@id="edit-radios-required-0" and @aria-required="true"]');
      +    $this->assertCount(0, $elements);
      

      Looks good.

    7. diff --git a/core/themes/classy/css/components/form.css b/core/themes/classy/css/components/form.css
      index ff8780c03f..5a6e220c74 100644
      --- a/core/themes/classy/css/components/form.css
      +++ b/core/themes/classy/css/components/form.css
      @@ -82,6 +82,9 @@ label.option {
         background-repeat: no-repeat;
         background-size: 6px 6px;
       }
      +.form-radios .form-required:after {
      +  display: none;
      +}
      

      I couldn't see where this selector actually matches a DOM element. Is this cruft from an earlier version of the patch?

  • A couple of notes, since this is new to me too. The radiogroup role is in fact valid on a <fieldset>. See the HTML-ARIA spec for details. Additionally, the W3C is no longer publishing their own version of the HTML spec, and are instead working with WHATWG in their repositority.

    I recently tested the support of aria-required on the radiogroup role. It has pretty good support, but using the required attribute on each radio button will most likely yield the most robust support.

  • Drupal 8.9.0-beta1 โ†’ was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule โ†’ and the Allowed changes during the Drupal 8 and 9 release cycles โ†’ .

  • Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9โ€™s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule โ†’ and the Allowed changes during the Drupal 8 and 9 release cycles โ†’ .

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrewmacpherson

    Re. #45: Thanks @mfairchild365.

    The radiogroup role is in fact valid on a . See the HTML-ARIA spec for details.

    You're right, that's a nice find! ARIA-in-HTML does say that the radiogroup role is permitted on the fieldset element, but the HTML5.2 rec says it isn't. So these two W3C standards are at odds with each other. The ARIA-in-HTML doc is only a working draft (and I think it's intended to have W3C Note status, rather than Recomendation status), but it's more recent than HTML 5.2. I don't think there's an easy way to square that.

    Additionally, the W3C is no longer publishing their own version of the HTML spec, and are instead working with WHATWG in their repositority.

    I'm not sure that's relevant; note that the WHATWG version of HTML doesn't offer any opinion on ARIA at all.

    I'm not certain what to do about the radiogroup role at this point. I'm inclined to say it's out of scope here, because the central problem in this issue is the way we're indicating the required-ness of the radios, not which container role is most appropriate.

    ... but using the required attribute on each radio button will most likely yield the most robust support.

    I'm in agreement with that.

    I think we have established the following things:

    • The required attribute should be present on the input elements, but not on the fieldset element.
    • The aria-required="true" attribute can be present on the fieldset element, but not on the input elements.
    • aria-required is allowed on elements with either thegroup, or radiogroup role. So it's permitted on the fieldset element regardless of whether the fieldset can have the radiogroup role.

    I think it's worth checking these with assistive tech:

    • Whether there is any practical difference between <fieldset> and <fieldset role="radiogroup"> in the real world, given the discrepancy in the standards.
    • Whether the accessible name of the radiogroup is correctly computed from the legend element. Do we need to include an explicit aria-labelledby along with the radiogroup role.
    • Whether it makes any practical difference whether the fieldset has the aria-required attribute. (Assuming the individual radio inputs have the required attribute.)
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada joseph.olstad

    wondering if something like this would work as a stop-gap?

    function mymodule_preprocess_fieldset(&$variables) {
      unset($variables['attributes']['required']);
      unset($variables['attributes']['aria-required']);
      unset($variables['attributes']['aria-invalid']);
    }
    
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom maseyuk

    Anyone looking for a quick solution to this on a custom form you can add the following:

    '#attributes' => ['required' => 'required']

    e.g.

    $form['my_radios'] = [
          '#type' => 'radios',
          '#title' => $this->t('adios'),
          '#options' => [
            'aaa' => $this->t('aaa'),
            'bbb' => $this->t('bbb'),
          ],
          '#attributes' => ['required' => 'required'],
          '#required' => TRUE,
        ];

    That will add the required attribute to each radio and doesn't mess around with anything else

  • Drupal 9.5.0-beta2 โ†’ and Drupal 10.0.0-beta2 โ†’ were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • Drupal 9.4.0-alpha1 โ†’ was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • Drupal 9.3.0-rc1 โ†’ was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • Drupal 9.2.0-alpha1 โ†’ will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • Drupal 9.1.0-alpha1 โ†’ will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule โ†’ and the Allowed changes during the Drupal 9 release cycle โ†’ .

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia dpi Perth, Australia

    Not sure where "#radios" / "#checkboxes" came from.

  • First commit to issue fork.
  • @dieterholvoet opened merge request.
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium DieterHolvoet Brussels

    I opened a MR with a rebased version of 2951317-38.patch.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada mgifford Ottawa, Ontario

    Adding WCAG 1.3.1 Tag.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia fenstrat Australia

    Attached is simply a patch version of #59 for use in composer.patches.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany mmbk MeiรŸen

    Hmm, @fenstrat the widget with the merge request contains a link plain diff that can be used for the composer patches.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Akram Khan Cuttack, Odisha

    Fixed some Coding standard issue

  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Akram Khan Cuttack, Odisha
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia fenstrat Australia

    @mmbk indeed that's how that patch was generated. Linking to diffs from MR's in composer patches is not a good idea. Anyone can change the code it would include by simply pushing to the MR. On your next build you're now including potentially broken, possibly malicious code.

  • ๐Ÿ‡ฒ๐Ÿ‡ฆMorocco simobm

    On 9.3.22 Right now, i had to go with #50, that was the only solution that worked for me.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sonam.chaturvedi Pune

    Verified and tested patch #63 on 10.1.x-dev. Patch applied cleanly.

    Test Result:
    1. The required attribute is present on the input elements - CORRECT
    2. The aria-required="true" attribute is present on the fieldset element, but not on the input elements - CORRECT
    3. As per #46, required attribute should not be present on the fieldset elements but it is present - INCORRECT

    Keeping it to NR to confirm the expected solution.
    Please refer attached before and after screenshots

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Reviewing the remaining tasks

    Remove required + and aria-required attributes from the <fieldset> element to the <input> element.

    Still needs to happen

    CSS to avoid visual regression. Asterisk should be after the fieldset legend, not the individual radio buttons

    May still need to happen.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States dww

    Adding ๐Ÿ› ['#states']['required'] broken for radios Needs work as related. We should probably fix this issue first, then circle back there to make sure #states works, too. Tagging this bug to be smashed...

  • Drupal core is moving towards using a โ€œmainโ€ branch. As an interim step, a new 11.x branch has been opened โ†’ , as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule โ†’ and the Allowed changes during the Drupal core release cycle โ†’ .

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    FWIW we discussed this in bug smash hard problems meeting (in conjunction with the related issue dww linked).
    Here's a codepen https://codepen.io/larowlan/pen/oNPVEax that demonstrates that putting required on each item is fine as suggested by mstrelan in that meeting.

    One thing to think about though is how does a user unset a radio value once set, but I think that impacts the states issue more, will comment there

  • ๐Ÿ‡ญ๐Ÿ‡บHungary nevergone Nyรญregyhรกza, Hungary, Europe

    How can the resolution of this proceed?

  • First commit to issue fork.
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia acbramley

    acbramley โ†’ changed the visibility of the branch 2951317-radios-element-missing to hidden.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia acbramley

    acbramley โ†’ changed the visibility of the branch 11.x to hidden.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia acbramley
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia acbramley
  • ๐Ÿ‡ซ๐Ÿ‡ทFrance Nicolas S. Lyon, France

    Patch on comment #79 apply in a Drupal 10.2.7
    Attribute required appear on html input radio but when you submit a form with a required radio TRUE, we don't have native error message but in console I see a mysterious javascrit error

    An invalid form control with name='field_contact_email' is not focusable. <input data-drupal-selector=โ€‹"edit-field-contact-email-1" required=โ€‹"required" type=โ€‹"radio" id=โ€‹"edit-field-contact-email-1" name=โ€‹"field_contact_email" value=โ€‹"1" class=โ€‹"form-radio">โ€‹

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands gidarai

    Rerolled patch in #62 to work with Drupal 10.3.0

Production build 0.69.0 2024