Checkbox input can't have aria-required attribute

Patch works as intended.

Before:

After:

+++ b/core/lib/Drupal/Core/Render/Element/RenderElement.php
@@ -139,7 +139,12 @@ public static function setAttributes(&$element, $class = []) {
+      if (isset($element['#type']) && $element['#type'] != 'checkbox') {
+        $element['#attributes']['aria-required'] = 'true';
+      }
+      elseif (!isset($element['#type'])) {
+        $element['#attributes']['aria-required'] = 'true';
+      }

we can do this in one line

if (($element['#type'] ?? NULL) !== 'checkbox') {
  // ...
}

As this is a bug-fix, we need test-coverage that confirms it is fixed to ensure it stays fixed. I guess \Drupal\Tests\system\Functional\Form\CheckboxTest would be a good place to look to add it.

Re-rolled patch #7, Attached interdiff for same. Please review.

RTBC!

+++ b/core/modules/system/tests/src/Functional/Form/CheckboxTest.php
@@ -102,4 +102,13 @@ public function testFormCheckbox() {
+    $element = $this->cssSelect('input[aria-required]');

can we add a positive assert here to.

E.g. assert that there is a single checkbox on the page. otherwise someone could remove the checkbox from that form and this test would happily keep passing, but be redundant

Made modifications to the test to add coverage for if checkbox exist

Verified and tested patch #20.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: admin/structure/types/manage/article/fields
# Click on Add field
# Select a Boolean from "Add a new field"
# Mention the Label name
# Set the field as required.
# Click on save
# Go: node/add/article
# User will able to see checkbox of Boolean

Expected Results:
# Inspect the checkbox field in the form page to add a new article and see that the required attribute is set

Actual Results:
# Inspect the checkbox field in the form page to add a new article and see that the aria-required attribute is set.

Looks good to me.
Can be a move to RTBC.

+++ b/core/modules/system/tests/src/Functional/Form/CheckboxTest.php
@@ -102,4 +102,15 @@ public function testFormCheckbox() {
+    $checkbox = $this->cssSelect('input[type=checkbox]');
+    $this->assertNotEmpty($checkbox);

we could use

$this->assertEmpty($this->assertSession()->elementExists('css', 'input[type=checkbox][required]')->getAttribute('aria-required'));

here and save some lines, but also ensure that we are adding a positive assert that the checkbox is required. At present, someone could remove the #required attribute from that checkbox and this test would pass.

Also, are there any existing test methods we could use here.

When running a functional test, each new test{Something} adds the setup cost of a new install

Moving to RTBC, after updated changes.

Fixed the #24 fails.

+++ b/core/lib/Drupal/Core/Render/Element/RenderElement.php
@@ -139,7 +139,9 @@ public static function setAttributes(&$element, $class = []) {
       $element['#attributes']['required'] = 'required';
-      $element['#attributes']['aria-required'] = 'true';
+      if (($element['#type'] ?? NULL) !== 'checkbox') {
+        $element['#attributes']['aria-required'] = 'true';
+      }
     }

Let's add an inline link to the W3C documentation.

I'm also slightly confused that this isn't allowed, since the 'required checkbox' is a common element on a lot of forms these days (usually for terms and conditions).

Had to reroll patch to apply cleanly to 9.27
Incorrect pathing - updated patch in next comment.

Edit: added patch 33, but realized approach is incorrect. 32 works.

#32 tested and works as desired. Also accounts for request in #30.

I also agree with Catch. It seems odd to not allow the aria role of required on a checkbox as it is a common pattern. I think the issue is that there was no agreement on how the role should behave for multiple checkboxes. And if there is no agreement on the role, it could not be spec'd as how it should behave, so it could not be included as a possible role.

Another version of patch to remove unnecessary NULL check on checkbox.

#36 works. Unclear what the request is for the change back to "needs work".

Forgot to account for "checkboxes." This patch adds.

Having reviewed the latest patch it appears to need to be rerolled, and per the coding standards the "See" line should be written in the format "@see [URL]".

Rerolled, and updated the syntax of the "@see" comment.

Manually tested the output and can confirm it worked against 9.4. I'm setting this back to RTBC because the patch now applies cleanly.

As spotted by SomeIntern - is this a duplicate of #3096790: aria-required attribute is redundant when required attribute is present?

Good q, @damienmckenna. It seems to me that https://www.drupal.org/project/drupal/issues/3096790 is recommending that we pull out aria-required, entirely. I think I could get behind this initiative. Per https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Tec...,

Note: HTML also specifies the required attribute which is supported well in user agents. Use aria-required for backwards compatibility only.

Applied patch #18 applied successfully and looks good for me
for ref sharing screenshot.....

Just to note DamienMcKenna's 42 altered EthanT's 39 patch slightly.
#39 excluded checkbox, checkboxes, and fieldset
#42 excluded checkbox, and checkboxes

fieldsets is another one that does not support aria-required. https://www.drupal.org/project/drupal/issues/3205691
Unfortunately both of these issues and related patches touch the same lines and pattern which makes them messy to apply.

I suggest changing from a simple string matching to an in_array pattern where things can be added without calamity. Patch coming.

Suggesting that we close this issue and put our support behind https://www.drupal.org/project/drupal/issues/3096790

Closing as a duplicate of #3096790: aria-required attribute is redundant when required attribute is present.