Collapsed Details Element causes JS Error for required fields

I can confirm that this issue is still happening in Drupal 9, and that the general idea of the patch (registering a listener for validation issues and opening parent details tags when triggered) fixes it.

This patch works for Drupal 9.5.x version

re-roll on drupal 9.5.x branch

re-compile .js from es6.js

As a bug will need a test case.

Will also need to verify this is a bug in 11.x branch and patch made for that as that's the current development branch.

Noticed that this patch is using jQuery.once and not Drupal once. Should this issue include using Drupal once?
https://www.drupal.org/node/3158256 →

Looks like I spoke too soon before diving in. The code in this issue is against 9.5.x and affects a file that was removed in `10.0.x` and `10.1.x`.

See https://www.drupal.org/project/drupal/issues/3269082 → for more details.

Next steps are to reroll this patch against D10, or at least D11

Fixed the issue on 11x branch and create merge request. Added a tests.

Removing tests tag as they have been added

1) Drupal\FunctionalJavascriptTests\Core\Form\FormGroupingElementsTest::testDetailsContainsRequiredTextfield
Text field is visible
Failed asserting that false is true.
/builds/issue/drupal-3333481/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-3333481/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/drupal-3333481/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/FormGroupingElementsTest.php:187
/builds/issue/drupal-3333481/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 4, Assertions: 27, Failures: 1.

This covers details but wonder if a future ticket could make it more dynamic in determining if it's hidden.

Thanks for working on this improvement!

I'm triaging RTBC issues → . I read the IS and the comments. There is a question in #19 that has not been answered.

I then tested the MR, with and without the fix and it works as expected. I then reviewed the MR and left comments there some of which require work. I have used the suggestion feature but they are just suggestions. Whoever works on this may have better ideas.

I am changing the status to NW.

Left some small comments.

Feedback has been addressed.

Couple of frequent usage case not supported (detail with required field added with ajax, changing the required state with the #states API)

Posted a solution that should work all the time.

I used a native event so I can listen in the capture phase that way opening the details element is the very first thing that runs, before the native browser callbacks are called, otherwise it causes an error with the details element being open too late.

So use my suggestion and add some more comments please.

Resolved and added more tests to cover form with ajax.

Applied 1 nitpicky suggestion and cleaned up the issue summary.

Changes look good though

This is a minor behaviour change but I think it is eligible for backport as fixing the bug is worse than leaving it in place.

Committed and pushed a31d15a87f to 11.x and 5663d5dd9c to 10.3.x and 4fbd912d57 to 10.2.x. Thanks!

longwave → closed merge request !5579

Automatically closed - issue fixed for 2 weeks with no activity.