Using $form_state->getValue() in BlockBase's blockForm throws "subform and parent form must contain the #parents property" exception

Having the same issue. On my setup what I see so far is that all block types added by Chaos Tools Blocks fail.

@mariagwyn
Do you also have ctools installed? Maybe it is a bug in ctools itself.

I can't place or edit Ctools content blocks, I'm getting the same error.

It's failing when the form calls $form_state->getValue().

This is a core issue, any usage of $form_state->getValue() in BlockBase's blockForm causes this error.

$form_state->getCompleteFormState()->getValue() will fix.

That does fix it, but FormStateInterface doesn't have that method. Shouldn't BlockBase/BlockPluginInterface then have SubformStateInterface in the method signature instead?

I'm not 100% sure this is the right fix, but it's a start.

Most form builders should not be calling getValue/getValues, but it is valid to do so when a form is rebuilding.

That fixes the immediate error, but then the form doesn't rebuild.

This is the code from Ctools that is breaking (from ctools/modules/ctools_block/src/Plugin/Block/EntityField.php):

  public function blockForm($form, FormStateInterface $form_state) {
    $config = $this->getConfiguration();

    $form['formatter_label'] = [
      '#type' => 'select',
      '#title' => $this->t('Label'),
      '#options' => [
        'above' => $this->t('Above'),
        'inline' => $this->t('Inline'),
        'hidden' => '- ' . $this->t('Hidden') . ' -',
        'visually_hidden' => '- ' . $this->t('Visually Hidden') . ' -',
      ],
      '#default_value' => $form_state->getValue('formatter_label') ?: $config['formatter']['label'],
    ];

    $form['formatter_type'] = [
      '#type' => 'select',
      '#title' => $this->t('Formatter'),
      '#options' => $this->getFormatterOptions(),
      '#default_value' => $form_state->getValue('formatter_type') ?: $config['formatter']['type'],
      '#ajax' => [
        'callback' => [static::class, 'formatterSettingsAjaxCallback'],
        'wrapper' => 'formatter-settings-wrapper',
        'effect' => 'fade',
      ],
    ];

    // Add the formatter settings to the form via AJAX.
    $form['#process'][] = [$this, 'formatterSettingsProcessCallback'];
    $form['formatter_settings_wrapper'] = [
      '#prefix' => '<div id="formatter-settings-wrapper">',
      '#suffix' => '</div>',
    ];
    $form['formatter_settings_wrapper']['formatter_settings'] = [
      '#tree' => TRUE,
      // The settings from the formatter plugin will be added in the
      // ::formatterSettingsProcessCallback method.
    ];

    return $form;
  }

If Ctools is not rebuilding the form correctly I'd be happy to write a patch, but I'm having a hard time finding an example of the correct way to do this now after the SubformState changes.

I'm one of the authors of the subform state code, and I've got some time to look at this problem this week and the next. However, I'm not too familiar with Ctools. Could someone update the issue summary with step-by-step instructions on how to reproduce the problem on a clean Drupal installation? That would be a great help in debugging this issue. Thanks!

Try to configure ANY block with the -fail patch applied.

Here's a more targeted approach. Actually getting the #parents wasn't ever really possible during build anyway.

To make a long IRC conversation short:

1. Embeddable forms had similar problems in Drupal 7: to properly know a subform's place in its parent form, we must wait until the #process callbacks are executed.
2. Knowing a subform's place in its parent form is necessary to make sure the subform can be embedded in *any* parent form, and to make sure that even if there is only a single parent form, its structure can be refactored without breaking its subforms. Subforms that rely on their parent forms' structure will inevitably break if the parent form decides to wrap the subform in an additional element (especially when '#tree' => TRUE), or if someone does this using hook_form_alter().
3. SubformState relies on this information to work, effectively forcing subforms that receive it to use #process.
4. This can be considered an implementation (class) break, but not an API (interface) break.
5. Any form that does not also expose a form ID should be considered a subform. This applies to all PluginFormInterface and BlockPluginInterface implementations, for instance.
6. The people who've worked on this issue, and on #2537732: PluginFormInterface must have access to the complete $form_state (introduce SubFormState for embedded forms) seem to have had different views on the previous point, and on how embeddable subforms need to be: should they be embeddable in any parent form with any structure, or only in one specific parent form with a specific structure (see item #2)? We should agree on this first, before moving on. If we don't, we'll risk people working on different, incompatible approaches.

I'm going to bump this to critical since it's an unexpected (albeit @internal) API change from 8.1.x to 8.2.x, and it looks like we can mitigate the uncaught exception in core.

Here's a test, fwiw

#22 doesn't fix the form in the linked Ctools issue. It doesn't outright break the same as it did before, but now the AJAX callback doesn't update the form.

It might be that Ctools is depending on old behavior that Subforms now change/fix, and will have to be updated, but like I said above—I can't find an example of how the Subform authors are now expecting these forms to work so I can't write that patch. It would be helpful to have an example form and some documentation around what the expectations are now. All I can find is "Embedded forms do not have to do anything, except assume that their $form and $form_state parameters belong together" which is clearly not the case when AJAX callbacks are involved.

Here's an example:

Formatters list:

When changing the formatter, the AJAX callback tries to load configuration into the form, but now fails (Image style was from the Image formatter):

So, I looked at this for a bit tonight. Seems like a requirement of using SubformState is that both the parent and subform must have documented #parents. I naively fixed this by forcing BlockForm::form() to properly document this before creating the SubformState. I don't even think I did it right and that's working for me. Is this a wrong approach?

If not, we could introspect the subform and parentform in the SubformState::__construct() to ensure it has parents and throw much more reasonable error messages based upon incorrect usage of the class. Just spit-balling here.

Eclipse

This adds deprecation errors to the two types of plugin forms we converted to using subform states in #2537732: PluginFormInterface must have access to the complete $form_state (introduce SubFormState for embedded forms). Let's see which plugins still don't receive subform states from their calling code in practice.

I do not think the approach in #22 is a good idea. Even in Drupal 7, subforms could only be properly supported by leveraging #process, as it is the earliest moment where #parents is known, which is needed to get the subform's values from the complete form state. This cannot be solved any other way until we change this behavior in Form API. Instead, I think we should warn developers to build subforms the right way. Next to that, #22 can lead to weird behavior, because it returns empty arrays by reference to pretend the current input is empty, while in a situation in which the input cannot technically be retrieved.

So, I think the underlying problem here is that $form_state->getValue() SHOULD NOT be called during form building, prior to when input processing has taken place, since there is no logical definition for what value ought to be there. So, in principle, I think for Drupal 9, we should have FormState::getValues() and friends throw an exception when called too early. And a step in that direction might be something along the lines of #22, but moved from SubformState to FormState. However, I think adding deprecation errors should happen in a minor release, not a patch release.

But since HEAD's current implementation of SubformState::getValues() represents an unintentional and undocumented breakage to places that used to receive a FormState object (such as in blockForm()), I think we should still consider some kind of fix in an 8.2 patch release. I don't yet have a suggestion for what that fix should be.

But I'm not sure about this being Critical priority. From https://www.drupal.org/core/issue-priority#critical-bug:

Regressions in functionality are not automatically considered critical.... it should be prioritized according to its impact.

Do we know of anything in the wild that is running into this error other than the ctools_block submodule of CTools? And that's a module that's in the "Chaos tool suite (Experimental)" package. I don't think that a break in a single contrib module, listed as Experimental, justifies this issue being Critical.

I have had to patch the Field as Block module with

$form_state->getCompleteFormState()->getValue()

In blockForm() To prevent a fatal error. But the maintainer gets a warning that I do not, so its maybe not a universal solution.

https://www.drupal.org/node/2810783
https://www.drupal.org/project/fieldblock

Do we know of anything in the wild that is running into this error other than the ctools_block submodule of CTools? And that's a module that's in the "Chaos tool suite (Experimental)" package. I don't think that a break in a single contrib module, listed as Experimental, justifies this issue being Critical.

I don't think the issue is that there's only one or two modules that are known to be breaking, the issue is that there's no documentation on what Ctools is doing wrong. How should a form be rebuilt based on the current value of a field?

Ctools is doing exactly what I've seen in many other places in D7, and is what the pattern used to be.

For example: http://cgit.drupalcode.org/examples/tree/ajax_example/ajax_example.modul...

in ajax_example_autotextfields() line 470:

if (!empty($form_state['values']['ask_first_name']) && $form_state['values']['ask_first_name']) {
  $form['textfields']['first_name'] = array(
    '#type' => 'textfield',
    '#title' => t('First Name'),
  );
}

Unfortunately there isn't an AJAX example for D8, and there's no docs I can find about how this should work. Was it just that this situation wasn't thought about when making the SubformState changes?

Sooo... what if we just reverted blockForm? Maybe a handful of other places where this change wasn't entirely necessary? After discussing this at length in irc, it's obvious that some of the acrobatics Tim and I did to make blocks be embeddable for things like panels really made this change unnecessary there. Or at the very least, much less likely to run into problems.

The problem of course with calling to the getCompleteFormState() method is that you no longer have a sub-set of form state values. This is fine so long as no one down the chain mis-uses a #tree, but if that were to happen at any point, the form state values we're attempting to retrieve would instantly become problematic. Reverting blockForm to the manually created FormState specific to the 'settings' form element would actually solve most of the "critical" problem here. I'm happy to discuss what a proper solution looks like in the CTools code base too, but just for the sake of not breaking potential undiscovered contrib blocks at this point, I think this is our best course of action. (just my suggestion)

This doesn't mean the new API isn't useful, and it doesn't mean we're not using it, it just means that for the block system, we would continue to use what was working in 8.1.x. I haven't looked at conditions yet, but I think the same nuances that were used to make block plugin config forms completely stand alone was also used for conditions, so if we've made the same expectations at that level, we should probably revert that implementation as well. I don't know of any other core plugins that had this level of detail. Tim could provide more color to this I'm sure (and he may totally disagree with me, I don't know) Whatever the case I wanted to propose this and see what the reaction was.

Eclipse

8.0.0 blocks shipped with a fix for *form values* only.

Any other method calls made on $form_state within block plugins were completely thrown away.
Most problematic is form errors.

Drupal\block\Tests\BlockUiTest::testBlockValidateErrors() and Drupal\block_test\Plugin\Block\TestSettingsValidationBlock illustrate this problem.

Essentially, setting an error in BlockBase::blockValidate() did absolutely nothing before 8.2.0

We could revert, and solve that specific problem by continuing to code around the problem in the block system.
Or we could come up with some as-yet-unknown fix for this problem.

How should a form be rebuilt based on the current value of a field?

Within Drupal\ctools_block\Plugin\Block\EntityField::blockForm(), what if you simply change:

'#default_value' => $form_state->getValue('formatter_label') ?: $config['formatter']['label'],
...
'#default_value' => $form_state->getValue('formatter_type') ?: $config['formatter']['type'],

to:

'#default_value' => $config['formatter']['label'],
...
'#default_value' => $config['formatter']['type'],

In other words, why do you need #default_value to reflect the new value during blockForm()? During form processing, validation, and submission, you have access to #value instead, which will be correct, regardless of #default_value.

If there are cases where there's other code that branches based on #default_value rather than #value, then you could add a #process callback to $form (or whatever other appropriate parent element) that adjusts the descendent element's #default_value to match what's in $form_state, but per #25, that's the earliest point at which you can do so in a way that works for subforms.

How should a form be rebuilt based on the current value of a field?...For example: in ajax_example_autotextfields() line 470:...Was it just that this situation wasn't thought about when making the SubformState changes?

Yes, that example is incorrect for subforms, because it assumes that you know that ask_first_name appears at the top-level of $form_state['values'].

Unfortunately there isn't an AJAX example for D8

Yep, good point. This would be really nice to get done.

In other words, why do you need #default_value to reflect the new value during blockForm()?

I made that change and it did indeed seem to work, I'll post a patch in the Ctools issue and see what they think over there.

@effulgentsia: Form builders may need to access submitted form values if they're multi-step. Say you're at step 2, and go back to step 1. The elements should no longer default to the hardcoded defaults for every instance of that form, but to the values you entered the first time you submitted step 1.

The fix for this is, and has been in Drupal 7 and likely even in earlier Drupal versions, to use #process callbacks. Those callbacks are invoked after Form API sets #array_parents and #parents, which are needed to determine the subform's position within the parent form, and the subform's values within the parent form's values. If these have not been set when SubformState is asked to get the subform's values, it throws an exception telling you to use #process callbacks.

The only way I see around this, is to provide a way to set #parents and #array_parents in the parent form's build method. We could abstract out this logic from FormBuilder, so form build methods can call this code and set these properties before they invoke subform builders, and FormBuilder itself can also call it to ensure the properties are set everywhere, just like it does now. There'll be some duplication and a small speed penalty, but it'll ensure subforms do not have to use #process as long as their parent forms use this new code. We might be able to perform this inside SubformState, so it's all done automatically.

Now I've looked at SubformState again I'm not sure if the approach I suggested in #35 is even possible, as SubformState would have to know its position in the parent form when it is constructed, and that's exactly the information its factory method does not receive. Luckily the constructor was originally made protected, so we can always modify or extend it and add additional factory methods. If we can come up with a solution that tells SubformState which exact parent form's element the subform will become, we may have a shot at the approach I suggested we try.

Back to the question of why form build methods need to access submitted form values: what if the submitted form values cause the form to be rebuilt with a different structure? Can we still properly access form values in build methods then? If not, what would be the right solution?

This was discussed with @xjm, @catch, @webchick, @effulgentsia, @Cottser, @cilefen and myself. We decided that this should be downgraded to a major because they is nothing in the issue that qualifies it to be critical as per https://www.drupal.org/core/issue-priority but only after a CR exists that details the difference between 8.1 and 8.2 and is attached to this issue and #2537732: PluginFormInterface must have access to the complete $form_state (introduce SubFormState for embedded forms).

@Xano created https://www.drupal.org/node/2774077 back in July and I published it in October. Are there updates we need to add to it?

Way back @effulgentsia, @catch, @alexpott, and I (and maybe other committers) discussed this bug, as per #37, and agreed to downgrade it. As it's been a month now, I'm doing so. Thanks!

This affected using the contrib module bootstrap_layout (with layout_discovery) as well. Patch #25 no longer applies (8.3.4), but patch #22 applied and worked around the issues I faced.

I'm trying to build a block form that will have a dynamic aspect to it. If a user chooses one option from a specific select menu, the form will be rebuilt with new options available. Is this possible with this issue still present in core? I looked at ctools module and it's doing something with a #process callback that I can't quite figure out... but maybe that's how it worked around the problem.

Has any progress been made with this recently?

@bkosborne I'm doing the same, a block form that changes dynamically when users selects options, altering the form build with ajax calls. My solution to work around this issue is doing something like this:

  public function blockForm($form, FormStateInterface $form_state) {
    $form = parent::blockForm($form, $form_state);

    if ($form_state instanceof SubformStateInterface) {
      $ajax_values = $form_state->getCompleteFormState()->getValues();
    }
    else {
      $ajax_values = $form_state->getValues();
    }
    $form_type = isset($ajax_values['settings']['myform']['type']) ? $ajax_values['settings']['myform']['type'] : 'default';

    $form['myform'] = [
      '#type' => 'container',
      '#attributes' => ['id' => 'myform-wrapper'],
    ];
    $form['myform']['type'] = [
      '#type' => 'select',
      '#title' => $this->t('From type'),
      '#default_value' => $form_type,
      '#options' => $this->getFormTypeOptions(),
      '#required' => TRUE,
      '#ajax' => [
        'callback' => [$this, 'ajaxFormSettingsCallback'],
        'wrapper'  => 'myform-wrapper',
        'event'    => 'change',
      ],
    ];
    // Build the rest of the form dynamically using $ajax_values. 
    $form['myform']['dynamic_select'] = [
      '#type' => 'select',
      '#title' => $this->t('Dynamic select'),
      '#options' => $this->getSelectOptions($form_type),
      '#validated' => TRUE, // Options list change dynamically on ajax calls.
    ];
    if ($form_type != 'default') {
      // New dynamically options...
    }
  }

  public function ajaxFormSettingsCallback(array &$form, FormStateInterface $form_state) {
    $form_state->setRebuild(TRUE);
    return $form['settings']['myform'];
  }

Probably it's not a best practice but works.

is it still actual?

Still running into it in 8.7, using the same type of workaround as in #46.

I'm having the same issue in Drupal 8.8.5 trying to put a custom Block in Layout Builder.

Yep, still have same issue with 9.0.6.

I literally spent a couple of workdays trying to figure this out, until I stumbled across #46.

This bug (or an extremely similar one) occurs with:

• D9.4.5 , and
• Paragraphs 1.14.0 , and
• layout_paragraphs 2.0.0-beta9
• Behaviors -> Paragraphs Layout option is checked for any paragraph type added to node creation form

When attempting to add more than one paragraph item to a node, the second and subsequent paragraphs fail to AJAX into existence. Then the WSOD / exception appears on node save.

If Behaviors -> Paragraphs Layout option is unchecked, everything proceeds as expected.

Exact exception message for this operation case is :

RuntimeException: The subform and parent form must contain the #parents property, which must be an array. Try calling this method from a #process callback instead. in Drupal\Core\Form\SubformState->getParents() (line 76 of core/lib/Drupal/Core/Form/SubformState.php).