Index: includes/ajax.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/ajax.inc,v
retrieving revision 1.19
diff -u -p -r1.19 ajax.inc
--- includes/ajax.inc	22 Nov 2009 02:48:37 -0000	1.19
+++ includes/ajax.inc	27 Nov 2009 23:14:20 -0000
@@ -258,9 +258,11 @@ function ajax_form_callback() {
   // 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
-  // drupal_process_form() set up.
-  $form = drupal_rebuild_form($form_id, $form_state, $form_build_id);
+  // An AJAX submission implies a multi-step form regardless of
+  // $form_state['rebuild'].
+  if (!form_get_errors()) {
+    $form = drupal_rebuild_form($form_id, $form_state, $form_build_id);
+  }
 
   // $triggering_element_path in a simple form might just be 'myselect', which
   // would mean we should use the element $form['myselect']. For nested form
Index: includes/form.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/form.inc,v
retrieving revision 1.407
diff -u -p -r1.407 form.inc
--- includes/form.inc	27 Nov 2009 15:14:58 -0000	1.407
+++ includes/form.inc	27 Nov 2009 23:14:20 -0000
@@ -114,11 +114,15 @@ function drupal_get_form($form_id) {
  *     request (so a browser refresh does not re-submit the form). However, if
  *     'rebuild' has been set to TRUE, then a new copy of the form is
  *     immediately built and sent to the browser; instead of a redirect. This is
- *     used for multi-step forms, such as wizards and confirmation forms. Also,
- *     if a form validation handler has set 'rebuild' to TRUE and a validation
- *     error occurred, then the form is rebuilt prior to being returned,
- *     enabling form elements to be altered, as appropriate to the particular
- *     validation error.
+ *     used for multi-step forms, such as wizards and confirmation forms. This
+ *     flag is primarily intended for submit handlers, but it's also possible
+ *     for a validation handler to set this instead of triggering a validation
+ *     error, in which case, submit handlers are skipped and the form is rebuilt
+ *     instead. Regardless of this setting, form rebuilding is not triggered if
+ *     there are any validation errors, so it is pointless for a validation
+ *     handler to set $form_state['rebuild'] if it is also triggering a
+ *     validation error. Form building functions should not set this flag, but
+ *     can check it to know if they are running in the context of a rebuild.
  *   - input: An array of input that corresponds to $_POST or $_GET, depending
  *     on the 'method' chosen (see below).
  *   - method: The HTTP form method to use for finding the input for this form.
@@ -272,10 +276,9 @@ function drupal_build_form($form_id, &$f
  */
 function form_state_defaults() {
   return array(
-    'args' => array(),
     'rebuild' => FALSE,
     'redirect' => NULL,
-    'build_info' => array(),
+    'build_info' => array('args' => array()),
     'storage' => array(),
     'submitted' => FALSE,
     'programmed' => FALSE,
@@ -317,6 +320,19 @@ function form_state_defaults() {
  *   The newly built form.
  */
 function drupal_rebuild_form($form_id, &$form_state, $form_build_id = NULL) {
+  // The values from the previous step is the input to the next step. Groups
+  // and values for the new step need to be rebuilt. drupal_rebuild_form()
+  // should not be called if there are any validation errors, but in case it is,
+  // do not retain the values.
+  $form_state['input'] = form_get_errors() ? array() : $form_state['values'];
+  $form_state['groups'] = array();
+  $form_state['values'] = array();
+
+  // AJAX and other contexts may call drupal_rebuild_form() even if
+  // $form_state['rebuild'] has not been set, so set it here, to let form
+  // building functions know if they are running in a rebuilding context.
+  $form_state['rebuild'] = TRUE;
+
   $form = drupal_retrieve_form($form_id, $form_state);
 
   if (!isset($form_build_id)) {
@@ -333,15 +349,6 @@ function drupal_rebuild_form($form_id, &
     form_set_cache($form_build_id, $form, $form_state);
   }
 
-  // Clear out all post data, as we don't want the previous step's
-  // data to pollute this one and trigger validate/submit handling,
-  // then process the form for rendering.
-  $form_state['input'] = array();
-
-  // Also clear out all group associations as these might be different
-  // when rerendering the form.
-  $form_state['groups'] = array();
-
   // Do not call drupal_process_form(), since it would prevent the rebuilt form
   // to submit.
   $form = form_builder($form_id, $form, $form_state);
@@ -1200,22 +1207,35 @@ function _form_builder_handle_input_elem
   if (!isset($element['#value']) && !array_key_exists('#value', $element)) {
     $value_callback = !empty($element['#value_callback']) ? $element['#value_callback'] : 'form_type_' . $element['#type'] . '_value';
 
-    if ($form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access']))) {
+    // Set #value based on submitted input.
+    // If rebuilding, then #access was checked for the initial build and should
+    // not be re-checked. If the form was submitted programmatically (via
+    // drupal_form_submit()), then we can also bypass #access checking.
+    if ($form_state['rebuild'] || $form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access']))) {
       $input = $form_state['input'];
       foreach ($element['#parents'] as $parent) {
         $input = isset($input[$parent]) ? $input[$parent] : NULL;
       }
       // If we have input for the current element, assign it to the #value property.
       if (!$form_state['programmed'] || isset($input)) {
-        // Call #type_value to set the form value;
-        if (function_exists($value_callback)) {
+        // If the input has not already been filtered by a value callback, then
+        // it must be, because for some elements (for example, checkboxes),
+        // browsers submit data differently than what is desired in a #value
+        // property. The input during a rebuild is values that have already been
+        // filtered by value callbacks, and should not be re-filtered.
+        if (!$form_state['rebuild'] && function_exists($value_callback)) {
           $element['#value'] = $value_callback($element, $input, $form_state);
         }
+        // If there's no value callback or it didn't return a value, then
+        // filtering for the way browsers submit data isn't needed, and #value
+        // can be set to $input.
         if (!isset($element['#value']) && isset($input)) {
           $element['#value'] = $input;
         }
       }
-      // Mark all posted values for validation.
+      // Value callbacks perform minor filtering only; they do not validate.
+      // All input still needs validation, even if the form was submitted
+      // programmatically, or is being rebuilt.
       if (isset($element['#value']) || (!empty($element['#required']))) {
         $element['#needs_validation'] = TRUE;
       }
Index: modules/book/book.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/book/book.admin.inc,v
retrieving revision 1.27
diff -u -p -r1.27 book.admin.inc
--- modules/book/book.admin.inc	21 Nov 2009 08:58:23 -0000	1.27
+++ modules/book/book.admin.inc	27 Nov 2009 23:14:21 -0000
@@ -96,7 +96,6 @@ function book_admin_edit($form, $form_st
 function book_admin_edit_validate($form, &$form_state) {
   if ($form_state['values']['tree_hash'] != $form_state['values']['tree_current_hash']) {
     form_set_error('', t('This book has been modified by another user, the changes could not be saved.'));
-    $form_state['rebuild'] = TRUE;
   }
 }
 
Index: modules/field/field.form.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/field/field.form.inc,v
retrieving revision 1.35
diff -u -p -r1.35 field.form.inc
--- modules/field/field.form.inc	20 Nov 2009 04:51:27 -0000	1.35
+++ modules/field/field.form.inc	27 Nov 2009 23:14:21 -0000
@@ -351,6 +351,13 @@ function field_add_more_submit($form, &$
     if ($form_state['values'][$field_name . '_add_more']) {
       $form_state['field_item_count'][$field_name] = count($form_state['values'][$field_name][$langcode]);
     }
+
+    // @todo: See http://drupal.org/node/622922#comment-2307106. In that issue,
+    // drupal_rebuild_form() was changed to preserve values during a form
+    // rebuild. This should enable Field API to clean up some of its form
+    // building logic. However, until that is cleaned up, emulate old
+    // drupal_rebuild_form() logic by wiping the form values.
+    $form_state['values'] = array();
   }
 }
 
Index: modules/file/file.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/file/file.module,v
retrieving revision 1.11
diff -u -p -r1.11 file.module
--- modules/file/file.module	31 Oct 2009 16:06:36 -0000	1.11
+++ modules/file/file.module	27 Nov 2009 23:14:21 -0000
@@ -212,9 +212,11 @@ function file_ajax_upload() {
   // 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_build_id);
+  // An AJAX submission implies a multi-step form regardless of
+  // $form_state['rebuild'].
+  if (!form_get_errors()) {
+    $form = drupal_rebuild_form($form_id, $form_state, $form_build_id);
+  }
 
   // Retrieve the element to be rendered.
   foreach ($form_parents as $parent) {
Index: modules/image/image.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/image/image.admin.inc,v
retrieving revision 1.16
diff -u -p -r1.16 image.admin.inc
--- modules/image/image.admin.inc	8 Nov 2009 13:23:41 -0000	1.16
+++ modules/image/image.admin.inc	27 Nov 2009 23:14:21 -0000
@@ -158,7 +158,6 @@ function image_style_form($form, &$form_
 function image_style_form_add_validate($form, &$form_state) {
   if (!$form_state['values']['new']) {
     form_error($form['effects']['new']['new'], t('Select an effect to add.'));
-    $form_state['rebuild'] = TRUE;
   }
 }
 
Index: modules/simpletest/tests/ajax.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/ajax.test,v
retrieving revision 1.3
diff -u -p -r1.3 ajax.test
--- modules/simpletest/tests/ajax.test	22 Nov 2009 02:48:37 -0000	1.3
+++ modules/simpletest/tests/ajax.test	27 Nov 2009 23:14:21 -0000
@@ -141,3 +141,46 @@ class AJAXCommandsTestCase extends AJAXT
     $this->assertTrue($command['command'] == 'restripe' && $command['selector'] == '#restripe_table', "'restripe' AJAX command issued with correct selector");
   }
 }
+
+/**
+ * Test that $form_state['values'] is properly delivered to $ajax['callback'].
+ */
+class AJAXFormValuesTestCase extends AJAXTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'AJAX Form Values in command',
+      'description' => 'Creates a form, then POSTS to system/ajax to change the form via pseudo-ajax.',
+      'group' => 'AJAX',
+    );
+  }
+
+  /**
+   * Create a simple form with a select, then POST a change to it.
+   */
+  function testSimpleAJAXFormValue() {
+    $web_user = $this->drupalCreateUser(array('access content'));
+    $this->drupalLogin($web_user);
+
+    $form_path = 'ajax_forms_test_get_form';
+
+    $selects_to_test = array('red', 'green', 'blue');
+    foreach($selects_to_test as $item) {
+      $edit = array();
+      $edit['select'] = $item;
+      $commands = $this->drupalPostAJAX($form_path, $edit, 'select');
+      $data_command = $commands[2];
+      $this->assertEqual($data_command['value'], $edit['select']);
+    }
+
+    $values_to_test = array(FALSE, TRUE);
+    foreach($values_to_test as $item) {
+      $edit = array();
+      $edit['checkbox'] = $item;
+      $commands = $this->drupalPostAJAX($form_path, $edit, 'checkbox');
+      $data_command = $commands[2];
+      $value_expected = (int) $item;
+      $value_received = (int) $data_command['value'];
+      $this->assertEqual($value_received, $value_expected);
+    }
+  }
+}
Index: modules/simpletest/tests/form.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form.test,v
retrieving revision 1.25
diff -u -p -r1.25 form.test
--- modules/simpletest/tests/form.test	18 Nov 2009 18:51:11 -0000	1.25
+++ modules/simpletest/tests/form.test	27 Nov 2009 23:14:21 -0000
@@ -70,7 +70,8 @@ class FormsTestCase extends DrupalWebTes
       foreach ($data['empty_values'] as $key => $empty) {
         foreach (array(TRUE, FALSE) as $required) {
           $form_id = $this->randomName();
-          $form = $form_state = array();
+          $form = array();
+          $form_state = form_state_defaults();
           form_clear_error();
           $form['op'] = array('#type' => 'submit', '#value' => t('Submit'));
           $element = $data['element']['#title'];
@@ -131,9 +132,7 @@ class FormsTestCase extends DrupalWebTes
     $this->assertRaw(t('!name field is required.', array('!name' => 'required_checkbox')), t('A required checkbox is actually mandatory'));
 
     // Now try to submit the form correctly.
-    $this->drupalPost(NULL, array('required_checkbox' => 1), t('Submit'));
-
-    $values = json_decode($this->drupalGetContent(), TRUE);
+    $values = drupal_json_decode($this->drupalPost(NULL, array('required_checkbox' => 1), t('Submit')));
     $expected_values = array(
       'disabled_checkbox_on' => 'disabled_checkbox_on',
       'disabled_checkbox_off' => '',
@@ -413,16 +412,14 @@ class FormsFormStorageTestCase extends D
 
   function setUp() {
     parent::setUp('form_test');
+    $user = $this->drupalCreateUser(array('access content'));
+    $this->drupalLogin($user);
   }
 
   /**
    * Tests using the form in a usual way.
    */
   function testForm() {
-
-    $user = $this->drupalCreateUser(array('access content'));
-    $this->drupalLogin($user);
-
     $this->drupalPost('form_test/form-storage', array('title' => 'new', 'value' => 'value_is_set'), 'Continue');
     $this->assertText('Form constructions: 2', t('The form has been constructed two times till now.'));
 
@@ -435,9 +432,6 @@ class FormsFormStorageTestCase extends D
    * Tests using the form with an activated $form_state['cache'] property.
    */
   function testFormCached() {
-    $user = $this->drupalCreateUser(array('access content'));
-    $this->drupalLogin($user);
-
     $this->drupalPost('form_test/form-storage', array('title' => 'new', 'value' => 'value_is_set'), 'Continue', array('query' => array('cache' => 1)));
     $this->assertText('Form constructions: 1', t('The form has been constructed one time till now.'));
 
@@ -450,9 +444,6 @@ class FormsFormStorageTestCase extends D
    * Tests validation when form storage is used.
    */
   function testValidation() {
-    $user = $this->drupalCreateUser(array('access content'));
-    $this->drupalLogin($user);
-
     $this->drupalPost('form_test/form-storage', array('title' => '', 'value' => 'value_is_set'), 'Continue');
     $this->assertPattern('/value_is_set/', t("The input values have been kept."));
   }
@@ -504,8 +495,7 @@ class FormStateValuesCleanTestCase exten
    * Tests form_state_values_clean().
    */
   function testFormStateValuesClean() {
-    $this->drupalPost('form_test/form-state-values-clean', array(), t('Submit'));
-    $values = json_decode($this->content, TRUE);
+    $values = drupal_json_decode($this->drupalPost('form_test/form-state-values-clean', array(), t('Submit')));
 
     // Setup the expected result.
     $result = array(
@@ -533,3 +523,37 @@ class FormStateValuesCleanTestCase exten
   }
 }
 
+/**
+ * Tests form rebuilding.
+ */
+class FormsRebuildTestCase extends DrupalWebTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name'  => 'Form rebuilding',
+      'description'  => 'Tests proper functioning of drupal_rebuild_form().',
+      'group' => 'Form API',
+    );
+  }
+
+  function setUp() {
+    parent::setUp('form_test');
+    $user = $this->drupalCreateUser(array('access content'));
+    $this->drupalLogin($user);
+  }
+
+  /**
+   * Tests preservation of values during an "add another item" operation.
+   */
+  function testRebuildPreservesValues() {
+    $this->drupalPost('form-test/form-rebuild-preserve-values', array('movies[0]' => 'foo', 'groceries[0]' => 'bar'), 'Add grocery item');
+    $this->assertFieldByName('movies[0]', 'foo', t('Submitted data for a form element unrelated to the clicked button was preserved.'));
+    $this->assertNoField('movies[1]', t('"Add another item" button did not add an item for an unrelated form element.'));
+    $this->assertFieldByName('groceries[0]', 'bar', t('Submitted data for a form element related to the clicked button was preserved.'));
+    $this->assertFieldByName('groceries[1]', 'DEFAULT', t('"Add another item" button added an item with the correct default value.'));
+  }
+
+  /**
+   * @todo Add tests for other aspects of form rebuilding.
+   */
+}
Index: modules/simpletest/tests/form_test.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form_test.module,v
retrieving revision 1.17
diff -u -p -r1.17 form_test.module
--- modules/simpletest/tests/form_test.module	21 Nov 2009 17:01:31 -0000	1.17
+++ modules/simpletest/tests/form_test.module	27 Nov 2009 23:14:21 -0000
@@ -78,6 +78,14 @@ function form_test_menu() {
     'type' => MENU_CALLBACK,
   );
 
+  $items['form-test/form-rebuild-preserve-values'] = array(
+    'title' => 'Form values preservation during rebuild test',
+    'page callback' => 'drupal_get_form',
+    'page arguments' => array('form_test_form_rebuild_preserve_values_form'),
+    'access arguments' => array('access content'),
+    'type' => MENU_CALLBACK,
+  );
+
   return $items;
 }
 
@@ -477,3 +485,66 @@ function _form_test_checkbox_submit($for
   drupal_json_output($form_state['values']);
   exit();
 }
+
+/**
+ * Form builder for testing preservation of values during a rebuild.
+ *
+ * Creates a form with two "add another item" buttons, each controlling a
+ * different element. Used to test that a rebuilt form has the expected values.
+ */
+function form_test_form_rebuild_preserve_values_form($form, &$form_state) {
+  $num_grocery_items = isset($form_state['num_grocery_items']) ? $form_state['num_grocery_items'] : 1;
+  $num_movies = isset($form_state['num_movies']) ? $form_state['num_movies'] : 1;
+  $form['#tree'] = TRUE;
+
+  $form['groceries'] = array();
+  for ($i=0; $i < $num_grocery_items; $i++) {
+    $form['groceries'][$i] = array(
+      '#type' => 'textfield',
+      '#title' => 'Grocery item ' . ($i + 1),
+      '#default_value' => 'DEFAULT',
+    );
+  }
+  $form['add_grocery_item'] = array(
+    '#type' => 'submit',
+    '#value' => 'Add grocery item',
+    '#submit' => array('form_test_form_rebuild_preserve_values_form_submit_add_item'),
+    '#_which_counter' => 'num_grocery_items',
+  );
+  $form['movies'] = array();
+  for ($i=0; $i < $num_movies; $i++) {
+    $form['movies'][$i] = array(
+      '#type' => 'textfield',
+      '#title' => 'Movie to rent ' . ($i + 1),
+      '#default_value' => 'DEFAULT',
+    );
+  }
+  $form['add_movie'] = array(
+    '#type' => 'submit',
+    '#value' => 'Add movie',
+    '#submit' => array('form_test_form_rebuild_preserve_values_form_submit_add_item'),
+    '#_which_counter' => 'num_movies',
+  );
+  $form['submit'] = array(
+    '#type' => 'submit',
+    '#value' => 'Submit',
+  );
+  return $form;
+}
+
+/**
+ * Button submit handler for form_test_form_rebuild_preserve_values_form().
+ */
+function form_test_form_rebuild_preserve_values_form_submit_add_item($form, &$form_state) {
+  $form_state['num_grocery_items'] = count($form_state['values']['groceries']);
+  $form_state['num_movies'] = count($form_state['values']['movies']);
+  $form_state[$form_state['clicked_button']['#_which_counter']]++;
+  $form_state['rebuild'] = TRUE;
+}
+
+/**
+ * Form submit handler for form_test_form_rebuild_preserve_values_form().
+ */
+function form_test_form_rebuild_preserve_values_form_submit($form, &$form_state) {
+  drupal_set_message(t("Your grocery items: %groceries, Your movies: %movies", array('%groceries' => implode(', ', $form_state['values']['groceries']), '%movies' => implode(', ', $form_state['values']['movies']))));
+}
Index: modules/taxonomy/taxonomy.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/taxonomy/taxonomy.admin.inc,v
retrieving revision 1.84
diff -u -p -r1.84 taxonomy.admin.inc
--- modules/taxonomy/taxonomy.admin.inc	27 Nov 2009 00:06:10 -0000	1.84
+++ modules/taxonomy/taxonomy.admin.inc	27 Nov 2009 23:14:21 -0000
@@ -111,8 +111,8 @@ function taxonomy_form_vocabulary($form,
   );
   $form['#vocabulary'] = (object) $edit;
   // Check whether we need a deletion confirmation form.
-  if (isset($form_state['confirm_delete']) && isset($form_state['values']['vid'])) {
-    return taxonomy_vocabulary_confirm_delete($form, $form_state, $form_state['values']['vid']);
+  if (isset($form_state['confirm_delete']) && isset($form['#vocabulary']['vid'])) {
+    return taxonomy_vocabulary_confirm_delete($form, $form_state, $form['#vocabulary']['vid']);
   }
   $form['name'] = array(
     '#type' => 'textfield',