Problem/Motivation

Validation messages for errors in section configuration forms do not get displayed because of #2897377: $form['#attributes']['data-drupal-selector'] is set to a random value so can't be used for its intended purpose.

Proposed resolution

Set the $form['#id'] explicitly so the messages will be displayed.

Remaining tasks

Review

User interface changes

The error messages will show.

API changes

None

Data model changes

None

CommentFileSizeAuthor
#9 interdiff_1-9.txt1.98 KBraman.b
#9 3103812-9.patch3.94 KBraman.b
#9 3103812-9-test-only.patch2.35 KBraman.b
#2 3103812-1.patch1.59 KBrromore

Comments

rromore created an issue. See original summary.

rromore’s picture

rromore commented
Related issues: +#2968110: Layout Builder's ConfigureBlockFormBase forms do not display validation errors on submit
StatusFileSize
new 3103812-1.patch1.59 KB
cilefen’s picture

cilefen commented
Status: Active » Needs review
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 8.8.x-dev » 8.9.x-dev
Status: Needs review » Needs work
Issue tags: +Blocks-Layouts, +Needs tests

I just hit this in contrib. NW for tests, but this is the correct fix.

Version: 8.9.x-dev » 9.1.x-dev

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.

b_sharpe’s picture

b_sharpe commented

Confirming fixed for me as well, +1

Version: 9.1.x-dev » 9.2.x-dev

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.

sakiland’s picture

sakiland commented

Works for me too, thanks. Tested on Drupal 8.9.7

raman.b’s picture

raman.b commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 3103812-9-test-only.patch2.35 KB
new 3103812-9.patch3.94 KB
new interdiff_1-9.txt1.98 KB

Adding some test coverage for this

The last submitted patch, 9: 3103812-9-test-only.patch, failed testing. View results

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

Thanks for the test coverage! I think this is ready to go.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
Related issues: +#2897377: $form['#attributes']['data-drupal-selector'] is set to a random value so can't be used for its intended purpose

Why don't we fix #2897377: $form['#attributes']['data-drupal-selector'] is set to a random value so can't be used for its intended purpose instead of adding this workaround? That issue has test coverage and looks close?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

This workaround exists in two other places already, as shown in that issue. Yes, someone (me) should take the time to review that other issue.
But I think this test coverage is valuable. So for me it makes more sense to commit this first.

If you disagree, should we take this test coverage and add it to the other issue?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

I agree the test coverage is valuable. Let's go ahead here - only more to tidy up in the other issue. It does feel backwards to keep adding workarounds when the real fix is so close.

Committed and pushed 832592c1a4 to 9.2.x and 0e0a8ba1b8 to 9.1.x. Thanks!

  • alexpott committed 832592c on 9.2.x 
    Issue #3103812 by raman.b, rromore, tim.plunkett: Layout Builder's...

  • alexpott committed 0e0a8ba on 9.1.x 
    Issue #3103812 by raman.b, rromore, tim.plunkett: Layout Builder's...

Status: Fixed » Closed (fixed)

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