Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
If a submit handler sets a batch process, all the following submit handlers are queued as batch sets to be executed after that the batch process has completed. If a submit handler is defined in a conditionally included file, the following code might not work:
<?php
function _batch_next_set() {
$batch = &batch_get();
if (isset($batch['sets'][$batch['current_set'] + 1])) {
$batch['current_set']++;
$current_set = &_batch_current_set();
if (isset($current_set['form_submit']) && ($function = $current_set['form_submit']) && function_exists($function)) {
// We use our stored copies of $form and $form_state to account for
// possible alterations by previous form submit handlers.
$function($batch['form_state']['complete form'], $batch['form_state']);
}
return TRUE;
}
}
?>
Since the file holding the submit handler may have been included in the original page request but not while processing the batch. In this case the submit handler will be skipped silently.
To reproduce this just set a batch from a submit handler acting before another one living in a not always included file, e.g. field_ui_field_settings_form_submit().
Proposed resolution
Find a way to store the parent file alongside with the submit handler, in order to include it before verifying that the submit handler is actually available.
Remaining tasks
- Find an agreement on the solution
- Write a patch
- Review and test the patch
User interface changes
None
API changes
None
Comments
Comment #1
plachIssue where this was discovered (real-life example): #1279372-15: Enable bulk field language updates when switching field translatability.
Comment #2
JeremyFrench CreditAttribution: JeremyFrench commentedI think that it should be possible to include files (more than one) in batch_set, that should be a fairly simple way to provide a mechanism for avoiding this bug.
I'm not sure if the framework should try and handle this automatically.
Comment #3
plach@JeremyFrench:
Not sure about this: in the case above we'd have to explicitly set the include file(s) where the submit handler(s) live in. If any piece of code added more submit handlers we would have no way to make them work, not knowing anything about them.
Comment #4
chx CreditAttribution: chx commentedComment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedYou would have the same problem with, for example, submitting the form via an AJAX request (that submits to
system/js.It's your job to define the files used for the validation / submit handlers in
$form_state['build_info']['files'].Comment #6
sun@see form_load_include()
Comment #7
plachGuys, I won't reopen this but in the ET code we had to explcitly include the core submit handler not ours: see #1279372-15: Enable bulk field language updates when switching field translatability around the end of the comment.
Comment #8
sunCan you provide an isolated test case for this?
I looked through the code you referenced and also field_ui, form and batch processing in HEAD, and can't see why field_ui.admin.inc wouldn't be included.
Comment #9
chx CreditAttribution: chx commentedComment #17
pameeela CreditAttribution: pameeela commentedConfirmed with plach this can be closed.