Problem/Motivation

After updating Drupal Core 10.5.1 to 10.5.2 nodes or entitys with plenty of fields are not saved anymore.

Nodes and Entities with just a few fields are saved as it should be. If a node is bigger, the browsers stays on the edit-Form and it is imposible to save the node.

I increased max_input_vars and post_max_size but that doesn`t help at all.

Greetings
Berthold

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Comments

blausch created an issue. See original summary.

blausch’s picture

blausch commented
Issue summary: View changes
blausch’s picture

blausch commented
Issue summary: View changes
cilefen’s picture

cilefen commented
Issue tags: -save form +Needs steps to reproduce

There have been no other reports of this critical bug in the days since 10.5.2 released so I am tagging this "needs steps to reproduce" because there must be some setup we need to identify that's causing the problem. My organization has been using 10.5.2 with large forms without issues.

When you say that the browser stays on the edit form, does that mean the form doesn't post? Are there any error messages logged, or on the screen? Are there browser console errors?

blausch’s picture

blausch commented

@the form doesn`t post
yes, that is what it looks like.
There a no error messages.... no JavaScript error and no php-error.

I tested it on different server. all server have the same problem.
An entity with 23 fields works fine. An entity with 69 fields cannot be saved anymore.
It is a plattform, which is not new.... it runs a couple of years.

I downgrade do 10.5.1 again and everything works fine.

cilefen’s picture

cilefen commented

#3465041: FormState::getTriggeringElement() behaves inconsistently when request is malformed is the only change to the forms system in 10.5.2. Does reverting the change to core/lib/Drupal/Core/Form/FormBuilder.php fix 10.5.2 on that site?

blausch’s picture

blausch commented

Yes, it does.... with the old FormBuilder.php (Version: 10.5.1) I can save the entities and nodes with plenty of fields.... :-)

cilefen’s picture

cilefen commented

Thanks for checking. I commented about the regression on that issue.

cilefen’s picture

cilefen commented

Based on this the site may have contributed or custom modules that alter form processing.

blausch’s picture

blausch commented

Yes I do.... a lot of form_alter :-)

blausch’s picture

blausch commented

I think, we are on a good way.
I deaktivated the form_alter Hook for a special Entity Type and now the 10.5.2 Version can save the entity.
Obviously it has nothing to do with the number of the fields in the entity.

But what is the reason for that.

I used the form_hook_alter for years like this:

function MODULNAME_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {    }
cilefen’s picture

cilefen commented

We would have to see the code within the hook function to answer your question.

blausch’s picture

blausch commented

for example....
there is a entity-Type with the name: gebaeude and the bundle with the same name: gebaeude

  if (
    ($form_id == 'gebaeude_gebaeude_form') or  // hier für Gebäude
    ($form_id == 'gebaeude_gebaeude_edit_form')
  ) {
    // zweiter Speichern button rechts unten
    $form['actions']['submit_secondary'] = [
      '#type' => 'submit',
      '#value' => t('Speichern'),
      '#attributes' => [
      ],
    ];
}

this code snipped generates a second Save-Button on the form.....

Everything is very simple.... no strange things.

cilefen’s picture

cilefen commented

Yet that code seems exactly like what #3465041: FormState::getTriggeringElement() behaves inconsistently when request is malformed is trying to prevent, in principle.

cilefen’s picture

cilefen commented
Title: Core Update 10.5.1 --> 10.5.2 » #3465041 prevents submitting a form having additional submit buttons
wla_g’s picture

wla_g commented

from a UX point of view another submit button with large forms is help full for the user (one near the top, one near the bottom). Why should that be forbidden?

eastdrive’s picture

eastdrive commented

We are also seeing this after upgrade to 10.5.2. Custom form does not post back.

As a temporary fix we have downgraded to 10.5.1.

cilefen’s picture

cilefen commented
Priority: Critical » Major

This doesn't meet the criteria for a critical bug.

blausch’s picture

blausch commented

@eastdrive
it is not necessary to downgrade the whole drupal installation to 10.5.1.... Look at my post #7

I don't understand, while they do this.....

a second submitbuttom is very helpfull......

greetings
Berthold

jvollebregt-swis’s picture

jvollebregt-swis commented

max_input_vars and post_max_size

These have nothing to do with it. This is a combination of an undocumented behavioral change in #3465041 and a bug in either core or your form. (Depending on how it's interpreted)

What's happening

When a form is submitted it's received on the server and the form is rebuilt. Drupal goes through every submit element and checks to see if it was the one that was submitted.

  • For legacy ancient HTML reasons the label on a submit input is the value, and you can't have a submit with a different value from the label.
  • For drupal reasons the name for all submit and button elements defaults to "op" instead of the typical hierarchical name used by other inputs.

This combination means that if you have two buttons with a "Cancel" label on your form then the server only receives op=Cancel and has to guess which of the buttons is supposed to be triggered.

Normally this isn't a problem, until you have different submit handlers or callbacks on two buttons with the same label. In old behavior Drupal would just clobber the triggering element and you would consistently receive the last one in the form. In the new behavior you will consistently receive the first one.

A lot of forms may have worked coincidentally because the most important triggering elements were at the end of the form, but these are now broken.

If the story stopped there it would be a clear case of "Your form is bad, this just exposed it. Fix it yourself" but there are other problems.

Non-interactable inputs' handlers are being triggered

There's no apparent way to make drupal ignore a button element.

When #type = button and #submit_button = FALSE drupal generates a slightly different input from submit. With type=button the button does not submit the form on press. So if you have a button labelled "Cancel" followed by a submit labelled "Cancel" the button won't work but the submit will.

But when you click the submit and the server checks to see what the triggering element is it will first match the button, even when the button is #submit_button = FALSE. Drupal will try to run the handlers on the supposedly uninteractable button instead of the submit that was actually clicked.

Ambiguous submits

Making the name of all buttons "op" means it's not necessarily possible to distinguish between them. There are valid business reasons to have different buttons with the same text that do different things. The ideal fix would be to give submits and buttons unique names like all the other inputs, but that's a far bigger breaking change and would make other things like JS more complex.

There should probably be a warning if FormBuilder detects duplicate submit key/value combinations in a form array.

Workaround

Manually ensure all of your submits and buttons have a unique #name, or at least that the ones you rely on do.

quietone’s picture

quietone commented
Title: #3465041 prevents submitting a form having additional submit buttons » [regression] #3465041 prevents submitting a form having additional submit buttons
Issue summary: View changes
thedut’s picture

thedut commented
Version: 10.5.x-dev » 11.3.x-dev

Hello, this issue exists on others main branches.
The #3465041 issue, causing the regression, was introduced from drupal 11.2.3 release.
I confirms it exists for the current drupal 11.3 branch.
So :
For sites on Drupal11 : downgrading to Drupal 11.2.2 could be a workarround. But, because of database updates, I would not recommand to proceed. This is mostly for people that run D11 <= 11.2.2 : do not upgrade to 11.2.3 or upper if you don't want to meet this issue.

quietone’s picture

quietone commented
Version: 11.3.x-dev » 11.x-dev

Changes are made on 11.x first, then backported.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.