Layout Builder's ConfigureBlockFormBase forms do not display validation errors on submit

Here is a patch

1. The validation messages work.
2. For some reason the messages aren't at the top even that they have -1000 weight as set by \Drupal\Core\Ajax\AjaxFormHelperTrait::ajaxSubmit
3. Had to add a work around for this #2897377: $form['#attributes']['data-drupal-selector'] is set to a random value so can't be used for its intended purpose. Could we add this to AjaxFormHelperTrait?
4. Needs tests. Should convert \Drupal\Tests\layout_builder\Functional\LayoutBuilderTest to a Javascript test to be able to test this?

1. Realized that UpdateBlockForm also was not showing validation messages so I moved the fix logic up to \Drupal\layout_builder\Form\ConfigureBlockFormBase
2. Also I am not sure why the messages weren't showing up at the top. From what I could tell they should have been at the top because they have the lowest weight
   

     '#type' => 'status_messages',
     '#weight' => -1000,
   

   So it somehow seems like a form #weight bug but I am probably missing something
   So I updated \Drupal\Core\Ajax\AjaxFormHelperTrait to actually move status_messages to the beginning of the array.

   I am not sure why I had to update that but couldn't figure out why it wasn't ordering correctly. \Drupal\settings_tray\Block\BlockEntitySettingTrayForm doesn't have this problem but I can't see the difference.

Here is a patch with tests:

1. It tests that messages appear when you add and update a block
2. It asserts that the message are at the top of the form
3. It copies some functions from InlineBlockTestBase. We should probably have a LayoutBuilderTestBase but I am going to look at #2924201: Resolve random failure in LayoutBuilderTest so that it can be added to HEAD now to see if we can finish that up.
4. I didn't provide a interdiff but the TEST_ONLY patch is the same as the interdiff in this case.

Forgot @group 😜

Whoops #9 was not clean. Reuploading

Just running the 1 test.

I am not sure why this is failing. Extending the wait for the element that is causing the fail. But I don't think that will help.

since I can't get it to fail locally I putting some debug messages to see if the dialog is actually closed or not

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
@@ -0,0 +1,153 @@
+    if ($assert_session->waitForElement('css', $messages_locator)) {
+      $this->fail('what change to get here?');

Hmm. I don't understand why waitForElement() didn't return NULL here again.

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
@@ -0,0 +1,153 @@
+      if ($assert_session->waitForElement('css', '#drupal-off-canvas')) {
+        $this->fail('the canvas is still open');
+      }
+      else {
+        $this->fail('the canvas is closed');

What? Even though the required title field is still to "" it the off-canvas dialog is still being closed.

Wonder if it could be problem that contextual link in the test is actually not opening in the off-canvas dialog?

Changing the assert to actually look for the elements in the off-canvas dialog.

1. +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
   @@ -60,9 +60,9 @@ public function testValidationMessage() {
   -    $this->assertNotEmpty($assert_session->waitForElementVisible('css', '.block-categories'));
   +    $this->assertNotEmpty($assert_session->waitForElementVisible('css', '#drupal-off-canvas .block-categories'));
   

   Leaving these changes in because it is a more explicit assertion

2. +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
   @@ -73,10 +73,10 @@ public function testValidationMessage() {
   -    $this->clickLink('Manage layout');
   +    $this->drupalGet($field_ui_prefix . '/display-layout/default');
   

   I have feeling this is what actually fixed the test. I ran into this in #2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder when clicking on the link was not the same for some reason as using drupalGet()

re #21
1) is the actual fix to get the messages to show.

Without 2) the messages will show up at the bottom of the form. Not sure why. My guess is the sorting already has happened at this point so simply setting the weight will not work.

re #25
1. fixed
2. fixed

And

1. +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
   @@ -0,0 +1,139 @@
   +    // @todo Remove after https://www.drupal.org/project/drupal/issues/2901792.
   +    'no_transitions_css',
   

   We don't actually need this if we have proper waits. the test module has been removed.
   @see #3020142: Test module no_transitions_css has invalid hook_page_attachments

2. +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
   @@ -0,0 +1,139 @@
   +  protected function assertSaveLayout() {
   +    $assert_session = $this->assertSession();
   +    $assert_session->linkExists('Save Layout');
   +    // Go to the Save Layout page. Currently there are random test failures if
   +    // 'clickLink()' is used.
   +    // @todo Convert tests that extend this class to NightWatch tests in
   +    // https://www.drupal.org/node/2984161
   +    $link = $this->getSession()->getPage()->findLink('Save Layout');
   +    $this->drupalGet($link->getAttribute('href'));
   +    $this->assertNotEmpty($assert_session->waitForElement('css', '.messages--status'));
   

   I don't think we need the work around this method provides. @tim.plunkett ran into this same problem in another test and figure out it was problem with a link or button being out of the viewport. I copied this \Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTestBase::assertSaveLayout but only needed there because adding more inline block pushes the link out of the viewport. This test is only adding 1 block so shouldn't be a problem. Will confirm with a 30x patch.

fixed #27

Looks like random fail because we weren't waiting for canvas to close.

@Kristen Pol sure! just used the layout and add/configure any block. remove the title(or other required field). You should see the validation error message in the off-canvas dialog.

The last patch failed here is another x30 patch

#36 is because aborted a test. I mean to retest the x30 patch from #34 but selected the wrong 1 twice.

Re-queued the x30 times patch a few times to be sure we got the random fail.

ok so now the x30 test that was passing is now failing

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
@@ -0,0 +1,119 @@
+    $this->waitForNoElement('#drupal-off-canvas');
+    $assert_session->assertWaitOnAjaxRequest();
+    $assert_session->linkExists('Save Layout');
+    $page->clickLink('Save Layout');

All failing right here when trying to save the layout.
with

1) Drupal\Tests\layout_builder\FunctionalJavascript\BlockFormMessagesTest::testValidationMessage
WebDriver\Exception\UnknownError: unknown error: Element ... is not clickable at point (60, 571). Other element would receive the click: ...
(Session info: headless chrome=62.0.3202.94)
(Driver info: chromedriver=2.33.506092 (733a02544d189eeb751fe0d7ddca79a0ee28cce4),platform=Linux 4.9.0-0.bpo.6-amd64 x86_64)

This is a common cause of random failures 😟

And it seems to not happen locally much. So something is happening that is making a link not clickable because another element would get the click.

My theory is something monetarily in the way and the time is different depending on something in the state of Drupalci, so it is very hard to fix.

What if we just tried a number of times click the link. Like we do with visibility in \Drupal\FunctionalJavascriptTests\JSWebAssert::waitForElementVisible().

Here is a x30 patch with clickElementWhenClickable(). which should be moved to a trait if this a solution.

yay! A totally different random error in #39.

While #34 seem to fail about 1/3 of the time #39 failed 1 out of 30.

This

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
@@ -0,0 +1,147 @@
+    $this->clickElementWhenClickable($page->findLink('Save Layout'));
+    $this->assertNotEmpty($assert_session->waitForElement('css', 'div:contains("The layout has been saved")'));

This is where it failed.

This the same random error I had to put a very strange fix in for in \Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTestBase::assertSaveLayout()

@tim.plunkett ran into this same problem recently and solved it my just reloading the page.

Trying that here

#42 set it to Needs Work because use a ".patch" extension for the interdiff file. Whoops

Ok the patches before had unrelated changes

1. it looks the whole problem with the message appearing at the bottom of the form can be solved with $form['#sorted'] = FALSE;
2. Added a todo to #3032275: Create a fault-tolerant method for interacting with links and fields in Javascript tests for the work around here for random test failures

🎉 thanks @xjm and all those you worked on this!