Index: includes/ajax.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/ajax.inc,v retrieving revision 1.29 diff -u -p -r1.29 ajax.inc --- includes/ajax.inc 31 Mar 2010 19:34:56 -0000 1.29 +++ includes/ajax.inc 3 Apr 2010 19:14:24 -0000 @@ -168,7 +168,7 @@ * $commands[] = ajax_command_changed('#object-1'); * // Menu 'page callback' and #ajax['callback'] functions are supposed to * // return render arrays. If returning an AJAX commands array, it must be - * // encapsulated in a render array structure. + * // encapsulated in a render array structure. * return array('#type' => 'ajax', '#commands' => $commands); * @endcode * @@ -239,6 +239,11 @@ function ajax_get_form() { // Since some of the submit handlers are run, redirects need to be disabled. $form_state['no_redirect'] = TRUE; + // When a form is rebuilt after AJAX processing, its #build_id and #action + // should not change. + $form_state['retain_during_rebuild']['#build_id'] = TRUE; + $form_state['retain_during_rebuild']['#action'] = TRUE; + // The form needs to be processed; prepare for that by setting a few internal // variables. $form_state['input'] = $_POST; @@ -263,18 +268,15 @@ function ajax_get_form() { * enhanced function. */ function ajax_form_callback() { - list($form, $form_state, $form_id, $form_build_id) = ajax_get_form(); - - // Build, validate and if possible, submit the form. - drupal_process_form($form_id, $form, $form_state); + list($form, $form_state) = ajax_get_form(); + drupal_process_form($form['#form_id'], $form, $form_state); - // This call recreates the form relying solely on the $form_state that - // drupal_process_form() set up. - $form = drupal_rebuild_form($form_id, $form_state, $form); - - // As part of drupal_process_form(), the element that triggered the form - // submission is determined, and in the case of AJAX, it might not be a - // button. This lets us route to the appropriate callback. + // We need to return the part of the form (or some other content) that needs + // to be re-rendered so the browser can update the page with changed content. + // Since this is the generic menu callback used by many AJAX elements, it is + // up to the #ajax['callback'] function of the element (may or may not be a + // button) that triggered the AJAX request to determine what needs to be + // rendered. if (!empty($form_state['triggering_element'])) { $callback = $form_state['triggering_element']['#ajax']['callback']; } Index: includes/batch.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/batch.inc,v retrieving revision 1.50 diff -u -p -r1.50 batch.inc --- includes/batch.inc 17 Feb 2010 22:44:51 -0000 1.50 +++ includes/batch.inc 3 Apr 2010 19:14:24 -0000 @@ -495,9 +495,11 @@ function _batch_finished() { // Use drupal_redirect_form() to handle the redirection logic. drupal_redirect_form($_batch['form_state']); - // If no redirection happened, save the final $form_state value to be - // retrieved by drupal_get_form() and redirect to the originating page. - $_SESSION['batch_form_state'] = $_batch['form_state']; + // If no redirection happened redirect to the originating page. In case we + // need to rebuild save the final $form_state for drupal_build_form(). + if (!empty($_batch['form_state']['rebuild'])) { + $_SESSION['batch_form_state'] = $_batch['form_state']; + } $function = $_batch['redirect_callback']; if (function_exists($function)) { $function($_batch['source_url'], array('query' => array('op' => 'finish', 'id' => $_batch['id']))); Index: includes/form.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/form.inc,v retrieving revision 1.447 diff -u -p -r1.447 form.inc --- includes/form.inc 31 Mar 2010 19:34:56 -0000 1.447 +++ includes/form.inc 3 Apr 2010 19:14:25 -0000 @@ -168,102 +168,66 @@ function drupal_build_form($form_id, &$f $form_state['input'] = $form_state['method'] == 'get' ? $_GET : $_POST; } + // We've been redirected here after a batch processing: The form has already + // been processed, but needs to be rebuilt. See _batch_finished(). if (isset($_SESSION['batch_form_state'])) { - // We've been redirected here after a batch processing : the form has - // already been processed, so we grab the post-process $form_state value - // and move on to form display. See _batch_finished() function. $form_state = $_SESSION['batch_form_state']; unset($_SESSION['batch_form_state']); + return drupal_rebuild_form($form_id, $form_state); } - else { - // If the incoming input contains a form_build_id, we'll check the - // cache for a copy of the form in question. If it's there, we don't - // have to rebuild the form to proceed. In addition, if there is stored - // form_state data from a previous step, we'll retrieve it so it can - // be passed on to the form processing code. - if (isset($form_state['input']['form_id']) && $form_state['input']['form_id'] == $form_id && !empty($form_state['input']['form_build_id'])) { - $form_build_id = $form_state['input']['form_build_id']; - $form = form_get_cache($form_build_id, $form_state); - } - - // If the previous bit of code didn't result in a populated $form - // object, we're hitting the form for the first time and we need - // to build it from scratch. - if (!isset($form)) { - // Record the filepath of the include file containing the original form, - // so the form builder callbacks can be loaded when the form is being - // rebuilt from cache on a different path (such as 'system/ajax'). See - // form_get_cache(). - // $menu_get_item() is not available at installation time. - if (!isset($form_state['build_info']['file']) && !defined('MAINTENANCE_MODE')) { - $item = menu_get_item(); - if (!empty($item['file'])) { - $form_state['build_info']['file'] = $item['file']; - } - } - $form = drupal_retrieve_form($form_id, $form_state); - $form_build_id = 'form-' . md5(uniqid(mt_rand(), TRUE)); - $form['#build_id'] = $form_build_id; - - // Fix the form method, if it is 'get' in $form_state, but not in $form. - if ($form_state['method'] == 'get' && !isset($form['#method'])) { - $form['#method'] = 'get'; - } - - drupal_prepare_form($form_id, $form, $form_state); - // Store a copy of the unprocessed form to cache in case - // $form_state['cache'] is set. - $original_form = $form; - } - - // Now that we know we have a form, we'll process it (validating, - // submitting, and handling the results returned by its submission - // handlers. Submit handlers accumulate data in the form_state by - // altering the $form_state variable, which is passed into them by - // reference. - drupal_process_form($form_id, $form, $form_state); - } - - // Most simple, single-step forms will be finished by this point -- - // drupal_process_form() usually redirects to another page (or to - // a 'fresh' copy of the form) once processing is complete. If one - // of the form's handlers has set $form_state['redirect'] to FALSE, - // the form will simply be re-rendered with the values still in its - // fields. - // - // If $form_state['rebuild'] has been set and input has been processed, we - // know that we're in a multi-part process of some sort and the form's - // workflow is not complete. We need to construct a fresh copy of the form, - // passing in the latest $form_state in addition to any other variables passed - // into drupal_get_form(). - if ($form_state['rebuild'] && $form_state['process_input'] && !form_get_errors()) { - $form = drupal_rebuild_form($form_id, $form_state); + // If the incoming input contains a form_build_id, we'll check the cache for a + // copy of the form in question. If it's there, we don't have to rebuild the + // form to proceed. In addition, if there is stored form_state data from a + // previous step, we'll retrieve it so it can be passed on to the form + // processing code. + if (isset($form_state['input']['form_id']) && $form_state['input']['form_id'] == $form_id && !empty($form_state['input']['form_build_id'])) { + $form = form_get_cache($form_state['input']['form_build_id'], $form_state); } - // After processing the form, the form builder or a #process callback may - // have set $form_state['cache'] to indicate that the original form and the - // $form_state shall be cached. But the form may only be cached if the - // special 'no_cache' property is not set to TRUE and we are not rebuilding. - elseif (isset($form_build_id) && $form_state['cache'] && empty($form_state['no_cache'])) { - // Cache the original, unprocessed form upon initial build of the form. - if (isset($original_form)) { - form_set_cache($form_build_id, $original_form, $form_state); - } - // After processing a cached form, only update the cached form state. - else { - form_set_cache($form_build_id, NULL, $form_state); + + // If the previous bit of code didn't result in a populated $form object, + // we're hitting the form for the first time and we need to build it from + // scratch. + if (!isset($form)) { + // Record the filepath of the include file containing the original form, + // so the form builder callbacks can be loaded when the form is being + // rebuilt from cache on a different path (such as 'system/ajax'). See + // form_get_cache(). + // $menu_get_item() is not available at installation time. + if (!isset($form_state['build_info']['file']) && !defined('MAINTENANCE_MODE')) { + $item = menu_get_item(); + if (!empty($item['file'])) { + $form_state['build_info']['file'] = $item['file']; + } } - } - // Don't override #theme if someone already set it. - if (!isset($form['#theme'])) { - drupal_theme_initialize(); - $registry = theme_get_registry(); - if (isset($registry[$form_id])) { - $form['#theme'] = $form_id; + $form = drupal_retrieve_form($form_id, $form_state); + $form['#build_id'] = 'form-' . md5(uniqid(mt_rand(), TRUE)); + + // Fix the form method, if it is 'get' in $form_state, but not in $form. + if ($form_state['method'] == 'get' && !isset($form['#method'])) { + $form['#method'] = 'get'; } + + drupal_prepare_form($form_id, $form, $form_state); + // Store a copy of the unprocessed form to cache in case + // $form_state['cache'] gets set. + $original_form = $form; } + // Now that we know we have a form, we'll process it (validating, submitting, + // and handling the results returned by its submission handlers. Submit + // handlers accumulate data in the form_state by altering the $form_state + // variable, which is passed into them by reference. + drupal_process_form($form_id, $form, $form_state); + + // After processing the form, the form builder or a #process callback may + // have set $form_state['cache'] to indicate that the original form and the + // $form_state shall be cached. But the form may only be cached if the special + // 'no_cache' property is not set to TRUE. + if (isset($original_form) && $form_state['cache'] && empty($form_state['no_cache'])) { + form_set_cache($form['#build_id'], $original_form, $form_state); + } return $form; } @@ -282,6 +246,7 @@ function form_state_defaults() { 'method' => 'post', 'groups' => array(), 'buttons' => array(), + 'retain_during_rebuild' => array(), ); } @@ -308,21 +273,15 @@ function form_state_defaults() { * A keyed array containing the current state of the form. * @param $old_form * (optional) A previously built $form. Used to retain the #build_id and - * #action properties in AJAX callbacks and similar partial form rebuilds. - * Should not be passed for regular rebuilds, for which the entire $form - * should be rebuilt freshly. + * #action properties in AJAX callbacks and similar partial form rebuilds. The + * only properties copied from $old_form are the ones which both exist in + * $old_form and for which $form_state['retain_during_rebuild'][PROPERTY] is + * TRUE. If $old_form is not passed, the entire $form is rebuilt freshly. * * @return * The newly built form. */ function drupal_rebuild_form($form_id, &$form_state, $old_form = NULL) { - // AJAX and other contexts may call drupal_rebuild_form() even when - // $form_state['rebuild'] isn't set, but _form_builder_handle_input_element() - // needs to distinguish a rebuild from an initial build in order to process - // user input correctly. Form constructors and form processing functions may - // also need to handle a rebuild differently than an initial build. - $form_state['rebuild'] = TRUE; - $form = drupal_retrieve_form($form_id, $form_state); // If only parts of the form will be returned to the browser (e.g. AJAX or @@ -331,12 +290,12 @@ function drupal_rebuild_form($form_id, & // Otherwise, a new #build_id is generated, to not clobber the previous // build's data in the form cache; also allowing the user to go back to an // earlier build, make changes, and re-submit. - $form['#build_id'] = isset($old_form['#build_id']) ? $old_form['#build_id'] : 'form-' . md5(mt_rand()); + $form['#build_id'] = (isset($old_form['#build_id']) && !empty($form_state['retain_during_rebuild']['#build_id'])) ? $old_form['#build_id'] : 'form-' . md5(mt_rand()); // #action defaults to request_uri(), but in case of AJAX and other partial // rebuilds, the form is submitted to an alternate URL, and the original // #action needs to be retained. - if (isset($old_form['#action'])) { + if (isset($old_form['#action']) && !empty($form_state['retain_during_rebuild']['#action'])) { $form['#action'] = $old_form['#action']; } @@ -415,6 +374,7 @@ function form_state_keys_no_cache() { 'redirect', 'no_redirect', 'temporary', + 'retain_during_rebuild', // Internal properties defined by form processing. 'buttons', 'triggering_element', @@ -588,7 +548,7 @@ function drupal_retrieve_form($form_id, * Processes a form submission. * * This function is the heart of form API. The form gets built, validated and in - * appropriate cases, submitted. + * appropriate cases, submitted and rebuilt. * * @param $form_id * The unique string identifying the current form. @@ -677,6 +637,33 @@ function drupal_process_form($form_id, & // Redirect the form based on values in $form_state. drupal_redirect_form($form_state); } + + // Don't rebuild or cache form submissions invoked via drupal_form_submit(). + if (!empty($form_state['programmed'])) { + return; + } + + // Most simple, single-step forms will be finished by this point -- + // drupal_process_form() usually redirects to another page (or to + // a 'fresh' copy of the form) once processing is complete. If one + // of the form's handlers has set $form_state['redirect'] to FALSE, + // the form will simply be re-rendered with the values still in its + // fields. + // + // If $form_state['rebuild'] has been set and input has been processed, we + // know that we're in a multi-part process of some sort and the form's + // workflow is not complete. We need to construct a fresh copy of the form, + // passing in the latest $form_state in addition to any other variables + // passed into drupal_get_form(). + if ($form_state['rebuild'] && !form_get_errors()) { + $form = drupal_rebuild_form($form_id, $form_state, $form); + $form_state['no_cache'] = TRUE; + } + // After processing a cached form, only update the cached form state. + elseif ($form_state['cache'] && empty($form_state['no_cache'])) { + form_set_cache($form['#build_id'], NULL, $form_state); + $form_state['no_cache'] = TRUE; + } } } @@ -761,6 +748,14 @@ function drupal_prepare_form($form_id, & } } + if (!isset($form['#theme'])) { + drupal_theme_initialize(); + $registry = theme_get_registry(); + if (isset($registry[$form_id])) { + $form['#theme'] = $form_id; + } + } + // Invoke hook_form_FORM_ID_alter() implementations. drupal_alter('form_' . $form_id, $form, $form_state); Index: modules/file/file.module =================================================================== RCS file: /cvs/drupal/drupal/modules/file/file.module,v retrieving revision 1.24 diff -u -p -r1.24 file.module --- modules/file/file.module 31 Mar 2010 19:34:56 -0000 1.24 +++ modules/file/file.module 3 Apr 2010 19:14:27 -0000 @@ -217,7 +217,7 @@ function file_ajax_upload() { return array('#type' => 'ajax', '#commands' => $commands, '#header' => FALSE); } - list($form, $form_state, $form_id, $form_build_id) = ajax_get_form(); + list($form, $form_state) = ajax_get_form(); if (!$form) { // Invalid form_build_id. @@ -234,12 +234,8 @@ function file_ajax_upload() { } $current_file_count = isset($current_element['#file_upload_delta']) ? $current_element['#file_upload_delta'] : 0; - // Build, validate and if possible, submit the form. - drupal_process_form($form_id, $form, $form_state); - - // This call recreates the form relying solely on the form_state that the - // drupal_process_form() set up. - $form = drupal_rebuild_form($form_id, $form_state, $form); + // Process user input and rebuild the updated form. + drupal_process_form($form['#form_id'], $form, $form_state); // Retrieve the element to be rendered. foreach ($form_parents as $parent) { Index: modules/simpletest/tests/form.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form.test,v retrieving revision 1.44 diff -u -p -r1.44 form.test --- modules/simpletest/tests/form.test 31 Mar 2010 19:34:56 -0000 1.44 +++ modules/simpletest/tests/form.test 3 Apr 2010 19:14:28 -0000 @@ -837,6 +837,10 @@ class FormsRebuildTestCase extends Drupa // what happens to the form action after a validation error. $this->assertText('Title field is required.', t('Non-AJAX submission correctly triggered a validation error.')); + // Ensure that the form contains two items in the multi-valued field, so we + // know we're testing a form that was correctly retrieved from cache. + $this->assert(count($this->xpath('//form[@id="page-node-form"]//div[contains(@class, "field-name-field-ajax-test")]//input[@type="text"]')) == 2, t('Form retained its state from cache.')); + // Ensure that the form's action is correct. $this->assertFieldByXPath('//form[@id="page-node-form" and @action="' . url('node/add/page') . '"]', NULL, t('Re-rendered form contains the correct action value.')); }