eca_form: Ajax using limited validation errors leads to faulty reads of submission values

Oh wow, this is a pretty complex one. Great, you have a use case to identify this and be able to fix it. I've reviewed the code, which looks OK, I've only made a couple of small comments about dealing with NULL values. Nothing major.

Apart from reviewing this, it's hard to test other than with the 2 samples you provided. On the other hand, we have a comprehensive CyPress test suite for a client project which does a lot of form altering and validation. No ajax involved there, but it would be great to see if all tests still passed with this MR. As our project is using ECA 1.1 at the moment, it would be great to have an MR for that branch too. If that passes, the confidence is much higher that nothing broke.

As no other changes expected for the 1.2 MR, would it be possible to provide one for 1.1 without too much of a hassle?

Thanks for reviewing this. Will address the threads and will create a MR for 1.1.x.

Created MR346 for 1.1.x and resolved all threads.

Thanks @mxh for providing the second MR for 1.1 - almost all tests in our fairly large test suite still work. But there is one use case that's been used in a number of tests which fails with this stack trace:

Sorry, only have a screenshot at this point. Must be something with the FormBuildEntity action when that calls processForm from the FormBuilder class.

Any idea what could be going wrong here, or do we have to dig deeper into our model and provide more details?

Thanks for testing. Hm, that's a bad sign I guess. Are there any special characteristics on the forms being tested, e.g. using inline entity form / special form widgets or alike? I could then try to reproduce this. Assuming the fatal didn't appear before, right?

Will try to reproduce this.

Correct, it does NOT crash without the patch. We can provide more details tomorrow, will see if I can identify any special circumstances, or be able to provide any reproducible setup.

That would be super helpful, thanks.

It may be that a contrib or custom module is setting a form field element on its own, setting the '#type' key to be not a string.

Is the test clicking on a button other than the default save?

Tried various things using Drupal core components, no luck so far reproducing this. Maybe there are some contrib modules involve that do something with forms.

First debugging phase discovered this:

• It get to $this->formBuilder->processForm in \Drupal\eca_form\Plugin\Action\FormBuildEntity::execute twice with seemingly the same form.
• The first time is has no issue.
• The second time, it gets to $element[$key] = $this->doBuildForm($form_id, $element[$key], $form_state); in \Drupal\Core\Form\FormBuilder::doBuildForm and that crashes when $array_parents contains gin_sidebar and footer

I'll continue debugging.

OK, the first time it goes through process form is the regular form build from the form submission, and the second time is caused by the ECA model.

The context for $this->elementInfo->getInfo($element[$key]['#type']) with key being actionis this:

So, the delete action has an empty array as its #type which is probably because the user has no permission to delete the node.

@mxh is that the reason? And do we have a way to prevent that from happening? Please let me know if you needed more details.

For completeness, getting to the same point for the "regular" form processing, the action element looks like this:

So, at that point, the #type is still a string, i.e. actions

So, the delete action has an empty array as its #type which is probably because the user has no permission to delete the node.

@mxh is that the reason? And do we have a way to prevent that from happening? Please let me know if you needed more details.

At least this one would lead to the fatal as the '#type' key is an empty array. Question now is, which component sets an empty array on that.

What I currently suspect is, because of the 'actions' key being unset in FormBuildEntity action, another component will then merge something into 'actions' which doesn't exist anymore, leading to this result.

I can have another look at this later this day.

Took a quick look into the Gin code and found the source of the problem. It's currently resided within the gin theme (3.x), in class Drupal\gin\GinContentFormHelper on line 183 the following is happening:

$form['gin_sidebar']['actions']['#type'] = ($form['actions']['#type']) ?? [];

.. which is setting the empty array on the '#type' key.

Although the bug is in the Gin theme, I can try to make FormBuildEntity not running into it. Will address this later this day.

That's incredible. Let me talk to Sascha, I'm sure he's happy to fix tat in Gin.

I've opened an issue with Gin: 🐛 Edge case: Illegal offset error in Drupal FormBuilder Needs review

Great stuff, all our tests are passing again with the patch for the Gin theme as described above.

Therefore, this issue can be set to RTBC and I will merge and back port your 2 MRs. Thanks a lot for all your support on this, has been awesome.

Maybe it makes sense that I adjust FormBuildEntity so that it won't run into the bug in the meantime?

It seems the 1.1.x merge was not yet done.

Not sure if we needed to do that. It really seems to be an edge case where

• the delete permission must be missing
• the Gin theme must be used
• the FormBuildEntity action must be used

And even if all 3 conditions apply, we haven't seen this issue every time. I'd say, if really anyone else were facing this before Gin gets fixed, they should find this issue with the reference to the MR in Gin so that they can patch their site.

Agree, thanks!

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