Problem/Motivation

Bogus errors are displayed and logged in the use case of a form with a 'dependent' element whose 'source' element is changed and this triggers an AJAX request.

Use case: a form with a 'dependent' element whose 'source' element change triggers an AJAX request.
Pre-condition: the 'dependent' element has a non-empty value.
Event: change value of 'source' element to trigger an AJAX request to refresh options in 'dependent' element.
Result: Bogus errors are displayed and logged stating 'An illegal choice has been detected'.

The 8x-9x workflow for an AJAX request has regressed from 7x for this use case.

A 'dependent' select element is one whose options depend on the value of another form element. For example, with automobiles, the list of makes or models depends on the manufacturer. If an AJAX request is tied to a change in the manufacturer element to refresh the list of makes-models, then this will always produce 1+ error messages and log entries. In 7x this does not occur.

A similar situation is described in the related issues.

steps to reproduce
Add two fields to a form in custom module.

  $form['field_list'] = [
    '#type' => 'select',
    '#options' => ['a', 'b', 'c', 'd', 'e'],
    '#default_value' => 'a',
    '#limit_validation_errors' => [
      array_merge($form['#parents'], ['field_list]),
    ],
    '#ajax' => [
      'callback' => 'my_module_dependent_refresh',
      'event' => 'change',
    ],
  ];

  $form['field_dependent_list'] = [
    '#type' => 'select',
    '#options' => my_module_dependent_options(),
  ];

  return $form;

action steps
Display form.
Set value of 'field_dependent_list'.
Change the value of 'field_list' to trigger the AJAX request.
Form refreshes with new list of options on 'field_dependent_list'.

expected result
No errors or log entries.

actual result
Have errors and log entries related to 'An illegal choice has been detected'.

workflow analysis
In 7x the AJAX request goes through these routines:

ajax_form_callback()
  list($form, $form_state, $form_id, $form_build_id, $commands) = ajax_get_form();
  drupal_process_form($form['#form_id'], $form, $form_state);
ajax_get_form()
  $form = form_get_cache($form_build_id, $form_state);
  $form_state['input'] = $_POST;
drupal_process_form()
  drupal_validate_form($form_id, $form, $form_state);
  _form_validate($form, $form_state, $form_id);
  $form = drupal_rebuild_form($form_id, $form_state, $form);

This workflow:

  • omits the form builder routine and
  • uses the cached copy of the form
  • which has the 'field_dependent_list' options based on the original value of 'field_list'
  • so the submitted values for 'field_dependent_list' are valid when drupal_validate_form executes
  • rebuilds the form with options based on the new value of 'field_list'
  • and returns the ajax response

In 8x-9x the AJAX request goes through these routines:

  Drupal\Core\Form\FormBuilder::buildForm
  Drupal\Core\Form\FormBuilder::retrieveForm
  Drupal\Core\Form\FormBuilder::prepareForm
  Drupal\Core\Form\FormBuilder::processForm
  Drupal\Core\Form\FormBuilder::doBuildForm
  Drupal\Core\Form\FormValidator::validateForm
  Drupal\Core\Form\FormValidator::doValidateForm
  if (isset($elements['#needs_validation'])) {
    Drupal\Core\Form\FormValidator::performRequiredValidation
  }
  $form_state->setLimitValidationErrors($this->determineLimitValidationErrors($form_state));
  foreach ($elements['#element_validate'] as $callback)
    call_user_func_array($form_state->prepareCallback($callback), [&$elements, &$form_state, &$complete_form]);
  }

This workflow:

  • is the same as the initial form display
  • builds the options for 'field_dependent_list' based on the submitted value of 'field_list'
  • automatically produces an inconsistent state between
  • the submitted values for 'field_dependent_list' (based on the original value of 'field_list')
  • and the new list of options for 'field_dependent_list'
  • when validateForm executes
  • emits error messages and inserts log entries related to 'An illegal choice has been detected'

A custom module can suppress the error messages but not the log entries.
So, although the user may not see error messages, the log will contain bogus errors.

Proposed resolution

These changes:

  • add routine to test whether errors should be output for an element
  • add this test to the conditions on performRequiredValidation
  • execute the performRequiredValidation block after
  • setting the limit_validation_errors array and
  • calling the '#element_validate' callback routines

To wit:

  Drupal\Core\Form\FormValidator::validateForm
  Drupal\Core\Form\FormValidator::doValidateForm
  $form_state->setLimitValidationErrors($this->determineLimitValidationErrors($form_state));
  foreach ($elements['#element_validate'] as $callback) {
    call_user_func_array($form_state->prepareCallback($callback), [&$elements, &$form_state, &$complete_form]);
  }
  if (isset($elements['#needs_validation']) && $form_state->recordError($elements)) {
    Drupal\Core\Form\FormValidator::performRequiredValidation
  }

With this change in call order the AJAX request workflow:

  • conforms to the comments in FormValidator::determineLimitValidationErrors
  • conforms to the concept of '#limit_validation_errors'

Remaining tasks

Review, add test, clarify documentation.

User interface changes

None other than absence of bogus error message.

API changes

None.

Data model changes

None.

Release notes snippet

Not applicable.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

solotandem created an issue. See original summary.

solotandem’s picture

Status: Active » Needs review
FileSize
3.03 KB

Attached patch implements proposed resolution.

solotandem’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: 3137947-2-ajax-request-workflow.patch, failed testing. View results

solotandem’s picture

Status: Needs work » Needs review
FileSize
2.9 KB

Revise order of execution.

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.

Giuseppe87’s picture

I'dd add that this patch partially helps also for this case:

a node add\edit form, where there are two selects.
The options of the latter are defined depending on the value of the first one.
The code is likewise the one in the description: the options are defined during the hook_form_alter, while the ajax callback simply returns the dependend element (wrapper).

If there is some other field on the form that trigger an ajax - e.g. a image field - that ajax triggers an invalid error on the dependant field, as the form_state values\form default value are empty, while the "validation sees the inserted value".

With the patch the error doesn't appear during the ajax. Altough, if the user removes that image, the "submit form" validation still triggers the error on the dependant field.

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

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

k4v’s picture

Thank you, the patch solved my problem while building a form with dependent option fields. :)

wombatbuddy’s picture

To avoid this issue you should do the manipulation with form elements in the buildForm() or in hook_form_alter() instead. References:
1. https://drupal.stackexchange.com/a/31486/82441
2. https://www.metaltoad.com/blog/avoiding-drupal-7-ajax-pitfalls
3. https://api.drupal.org/api/examples/ajax_example%21src%21Form%21Dependen...

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Moving back to NW for tests

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joachim’s picture

+++ b/core/lib/Drupal/Core/Form/FormValidator.php
@@ -241,14 +241,14 @@ protected function doValidateForm(&$elements, FormStateInterface &$form_state, $
+      if (isset($elements['#needs_validation']) && $form_state->recordError($elements)) {

Why not move the check on #needs_validation into the new method, so that all the logic is inside the method?

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

kiwad’s picture

In case this code snippet helps, here's how we've done it to avoid 'An illegal choice has been detected'.

     // Champ de sélection de termes de taxonomie (uniquement les termes parents).
     $form['taxonomy_select'] = [
      '#type' => 'select',
      '#title' => t('Disciplinary grouping'),
      '#options' => $this->getTaxonomyParentOptions(),
      '#empty_option' => t('- Select -'),
      '#ajax' => [
        'callback' => '::updateChildrenCheckboxes',
        'wrapper' => 'children-checkboxes-wrapper',
      ],
    ];

    // Cases à cocher pour les enfants du terme sélectionné.
    $form['children_checkboxes' . $form_state->getValue('taxonomy_select')] = [
      '#type' => 'checkboxes',
      '#title' => t('Field of specialization'),
      '#options' => $this->getTaxonomyChildrenOptions($form_state->getValue('taxonomy_select')),
      '#prefix' => '<div id="children-checkboxes-wrapper">',
      '#suffix' => '</div>',
      '#default_value' => !empty($form_state->getValue('children_checkboxes')) ? $form_state->getValue('children_checkboxes') : [],
      '#states' => [
        'invisible' => [
          ':input[name="taxonomy_select"]' => ['value' => ''],
        ],
      ],
    ];

By adding $form_state->getValue('taxonomy_select') inside the children form element makes it work

kiwad’s picture

I had to re-write my code, previous snippet was causing other problems on validation

I used DependentDropdown from Ajax Examples as base code for my form and patch in #5 did correct the "Illegal choice" problem.