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.
Comment | File | Size | Author |
---|---|---|---|
#5 | 3137947-5-ajax-request-workflow.patch | 2.9 KB | solotandem |
#2 | 3137947-2-ajax-request-workflow.patch | 3.03 KB | solotandem |
Comments
Comment #2
solotandem CreditAttribution: solotandem commentedAttached patch implements proposed resolution.
Comment #3
solotandem CreditAttribution: solotandem commentedComment #5
solotandem CreditAttribution: solotandem commentedRevise order of execution.
Comment #7
Giuseppe87 CreditAttribution: Giuseppe87 at Elicos commentedI'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.
Comment #11
k4v CreditAttribution: k4v as a volunteer commentedThank you, the patch solved my problem while building a form with dependent option fields. :)
Comment #12
wombatbuddy CreditAttribution: wombatbuddy commentedTo 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...
Comment #13
smustgrave CreditAttribution: smustgrave at Mobomo commentedMoving back to NW for tests
Comment #15
joachim CreditAttribution: joachim as a volunteer commentedWhy not move the check on #needs_validation into the new method, so that all the logic is inside the method?
Comment #17
kiwad CreditAttribution: kiwad commentedIn case this code snippet helps, here's how we've done it to avoid 'An illegal choice has been detected'.
By adding $form_state->getValue('taxonomy_select') inside the children form element makes it work
Comment #18
kiwad CreditAttribution: kiwad commentedI 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.