Index: includes/form.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/form.inc,v
retrieving revision 1.470
diff -u -p -r1.470 form.inc
--- includes/form.inc	16 Jun 2010 05:06:15 -0000	1.470
+++ includes/form.inc	24 Jun 2010 00:00:12 -0000
@@ -1342,6 +1342,46 @@ function form_error(&$element, $message 
  * data to the proper elements. Also, execute any #process handlers
  * attached to a specific element.
  *
+ * This is one of the three primary functions that recursively iterates a form
+ * array. This one does it for completing the form building process. The other
+ * two are _form_validate() (invoked via drupal_validate_form() and used to
+ * invoke validation logic for each element) and drupal_render() (for rendering
+ * each element). Each of these three pipelines provides ample opportunity for
+ * modules to customize what happens. For example, during this function's life
+ * cycle, the following functions get called for each element:
+ * - $element['#value_callback']: A function that implements how user input is
+ *   mapped to an element's #value property. This defaults to a function named
+ *   'form_type_TYPE_value' where TYPE is $element['#type'].
+ * - $element['#process']: An array of functions called after user input has
+ *   been mapped to the element's #value property. These functions can be used
+ *   to dynamically add child elements: for example, for the 'date' element
+ *   type, one of the functions in this array is form_process_date(), which adds
+ *   the individual 'year', 'month', 'day', etc. child elements. These functions
+ *   can also be used to set additional properties or implement special logic
+ *   other than adding child elements: for example, for the 'fieldset' element
+ *   type, one of the functions in this array is form_process_fieldset(), which
+ *   adds the attributes and JavaScript needed to make the fieldset collapsible
+ *   if the #collapsible property is set. The #process functions are called in
+ *   preorder traversal, meaning they are called for the parent element first,
+ *   then for the child elements.
+ * - $element['#after_build']: An array of functions called after form_builder()
+ *   is done with its processing of the element. These are called in postorder
+ *   traversal, meaning they are called for the child elements first, then for
+ *   the parent element.
+ * There are similar properties containing callback functions invoked by
+ * _form_validate() and drupal_render(), appropriate for those operations.
+ *
+ * Developers are strongly encouraged to integrate the functionality needed by
+ * their form or module within one of these three pipelines, using the
+ * appropriate callback property, rather than implementing their own recursive
+ * traversal of a form array. This facilitates proper integration between
+ * multiple modules. For example, module developers are familiar with the
+ * relative order in which hook_form_alter() implementations and #process
+ * functions run. A custom traversal function that affects the building of a
+ * form is likely to not integrate with hook_form_alter() and #process in the
+ * expected way. Also, deep recursion within PHP is both slow and memory
+ * intensive, so it is best to minimize how often it's done.
+ *
  * @param $form_id
  *   A unique string identifying the form for validation, submission,
  *   theming, and hook_form_alter functions.
Index: modules/simpletest/tests/system_test.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/system_test.module,v
retrieving revision 1.29
diff -u -p -r1.29 system_test.module
--- modules/simpletest/tests/system_test.module	26 May 2010 07:31:47 -0000	1.29
+++ modules/simpletest/tests/system_test.module	24 Jun 2010 00:00:14 -0000
@@ -305,3 +305,35 @@ function _system_test_second_shutdown_fu
   print t('Second shutdown function, arg1 : @arg1, arg2: @arg2', array('@arg1' => $arg1, '@arg2' => $arg2));
 }
 
+/**
+ * Form builder for testing system_settings_form().
+ */
+function system_test_system_settings_form($form, $form_state, $automatic = TRUE) {
+  $form['system_settings_form_test_checkbox_false'] = array(
+    '#type' => 'checkbox',
+    '#default_value' => FALSE,
+  );
+  $form['system_settings_form_test_checkbox_true'] = array(
+    '#type' => 'checkbox',
+    '#default_value' => TRUE,
+  );
+  $form['wrapper']['system_settings_form_test_nested_checkbox_false'] = array(
+    '#type' => 'checkbox',
+    '#default_value' => FALSE,
+  );
+  $form['wrapper']['system_settings_form_test_nested_checkbox_true'] = array(
+    '#type' => 'checkbox',
+    '#default_value' => TRUE,
+  );
+  $form['system_settings_form_test_checkbox_false_no_automatic'] = array(
+    '#type' => 'checkbox',
+    '#default_value' => FALSE,
+    '#system_settings_use_automatic_default' => FALSE,
+  );
+  $form['system_settings_form_test_checkbox_true_no_automatic'] = array(
+    '#type' => 'checkbox',
+    '#default_value' => TRUE,
+    '#system_settings_use_automatic_default' => FALSE,
+  );
+  return system_settings_form($form, $automatic);
+}
Index: modules/system/system.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/system/system.module,v
retrieving revision 1.939
diff -u -p -r1.939 system.module
--- modules/system/system.module	21 Jun 2010 02:27:47 -0000	1.939
+++ modules/system/system.module	24 Jun 2010 00:00:15 -0000
@@ -2568,32 +2568,65 @@ function system_default_region($theme) {
   return isset($regions[0]) ? $regions[0] : '';
 }
 
-function _system_settings_form_automatic_defaults($form) {
-  // Get an array of all non-property keys
-  $keys = element_children($form);
-
-  foreach ($keys as $key) {
-    // If the property (key) '#default_value' exists, replace it.
-    if (array_key_exists('#default_value', $form[$key])) {
-      $form[$key]['#default_value'] = variable_get($key, $form[$key]['#default_value']);
-    }
-    else {
-      // Recurse through child elements
-      $form[$key] = _system_settings_form_automatic_defaults($form[$key]);
-    }
-  }
-
-  return $form;
+/**
+ * Helper function used by system_settings_form() to apply automatic defaults.
+ */
+function _system_settings_form_automatic_defaults($elements) {
+  // @todo It is strongly discouraged for any function other than form_builder()
+  //   to recursively traverse a form array while the form is being built. See
+  //   the form_builder() documentation for why. For Drupal 8, fix this code to
+  //   comply. However, do not do this for Drupal 7, as that would be an API
+  //   change with respect to the application of automatic defaults to elements
+  //   added by hook_form_alter() implementations. For more information, see
+  //   http://drupal.org/node/266246.
+  foreach (element_children($elements) as $key) {
+    // If the element both has a #default_value (even if NULL) and does not
+    // explicitly disable having it overriden with a system variable, then set
+    // the #default_value with the value of the corresponding system variable,
+    // if it exists.
+    $disable_automatic_default = isset($elements[$key]['#system_settings_use_automatic_default']) && !$elements[$key]['#system_settings_use_automatic_default'];
+    if (array_key_exists('#default_value', $elements[$key]) && !$disable_automatic_default) {
+      $elements[$key]['#default_value'] = variable_get($key, $elements[$key]['#default_value']);
+    }
+    // Recurse through child elements.
+    $elements[$key] = _system_settings_form_automatic_defaults($elements[$key]);
+  }
+  return $elements;
 }
 
 /**
- * Add default buttons to a form and set its prefix.
+ * Adds default buttons and a submission handler to a form.
+ *
+ * The submission handler saves each form element in the corresponding system
+ * variable, using variable_set(). If $automatic_defaults is TRUE (the default),
+ * the previously-saved system variable values are also read in and displayed
+ * on the form.
  *
  * @param $form
  *   An associative array containing the structure of the form.
  * @param $automatic_defaults
- *   Automatically load the saved values for each field from the system variables
- *   (defaults to TRUE).
+ *   If TRUE (default), automatically load default values for each form element
+ *   from system variables. The name of the loaded variable is the same as the
+ *   name of the element, and the element's #default_value property is used as
+ *   the default value for the variable_get() call. If $automatic_defaults is
+ *   FALSE, the #default_value property of each element is used directly as the
+ *   default value shown in the form element. When TRUE, automatic defaults are
+ *   applied only if all three of the following conditions are met:
+ *   - The element exists in $form at the time system_settings_form() is called.
+ *     Since system_settings_form() is usually called by the original form
+ *     builder function, this means that hook_form_alter() implementations and
+ *     #process functions run after system_settings_form(), and therefore,
+ *     elements that those functions add, or changes that those functions make
+ *     to an element's #default_value do not have automatic defaults applied to
+ *     them.
+ *   - The element has an explicitly defined value for its #default_value
+ *     property. This value may be NULL, but elements without a #default_value
+ *     property defined at the time system_settings_form() is called do not have
+ *     an automatic default value applied.
+ *   - The element does not have #system_settings_use_automatic_default set to
+ *     FALSE. This enables elements to be added to a system settings form that
+ *     require their own #default_value logic, rather than the simple
+ *     variable_get() call used for applying an automatic default.
  *
  * @return
  *   The form structure.
Index: modules/system/system.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/system/system.test,v
retrieving revision 1.127
diff -u -p -r1.127 system.test
--- modules/system/system.test	21 Jun 2010 02:27:47 -0000	1.127
+++ modules/system/system.test	24 Jun 2010 00:00:15 -0000
@@ -1242,64 +1242,61 @@ class SystemSettingsForm extends DrupalW
    * Implement setUp().
    */
   function setUp() {
-    parent::setUp();
-
-    variable_set('system_settings_form_test', TRUE);
-    variable_set('system_settings_form_test_4', TRUE);
-  }
-
-  /**
-   * Reset page title.
-   */
-  function tearDown() {
-    variable_del('system_settings_form_test');
-    variable_del('system_settings_form_test_4');
-
-    parent::tearDown();
+    parent::setUp('system_test');
   }
 
   /**
    * Tests the handling of automatic defaults in systems_settings_form().
    */
   function testAutomaticDefaults() {
-    $form['system_settings_form_test'] = array(
-      '#type' => 'checkbox',
-      '#default_value' => FALSE,
-    );
-
-    $form['system_settings_form_test_2'] = array(
-      '#type' => 'checkbox',
-      '#default_value' => FALSE,
-    );
-
-    $form['system_settings_form_test_3'] = array(
-      '#type' => 'checkbox',
-      '#default_value' => TRUE,
-    );
-
-    $form['has_children']['system_settings_form_test_4'] = array(
-      '#type' => 'checkbox',
-      '#default_value' => FALSE,
-    );
-
-    $form['has_children']['system_settings_form_test_5'] = array(
-      '#type' => 'checkbox',
-      '#default_value' => TRUE,
-    );
-
-    $automatic = system_settings_form($form, FALSE);
-    $this->assertFalse($automatic['system_settings_form_test']['#default_value']);
-    $this->assertFalse($automatic['system_settings_form_test_2']['#default_value']);
-    $this->assertTrue($automatic['system_settings_form_test_3']['#default_value']);
-    $this->assertFalse($automatic['has_children']['system_settings_form_test_4']['#default_value']);
-    $this->assertTrue($automatic['has_children']['system_settings_form_test_5']['#default_value']);
-
-    $no_automatic = system_settings_form($form);
-    $this->assertTrue($no_automatic['system_settings_form_test']['#default_value']);
-    $this->assertFalse($no_automatic['system_settings_form_test_2']['#default_value']);
-    $this->assertTrue($no_automatic['system_settings_form_test_3']['#default_value']);
-    $this->assertTrue($no_automatic['has_children']['system_settings_form_test_4']['#default_value']);
-    $this->assertTrue($no_automatic['has_children']['system_settings_form_test_5']['#default_value']);
+    // Ensure that when there aren't saved system variables, elements have their
+    // form builder assigned #default_value, regardless of whether
+    // system_settings_form() is invoked with automatic defaults or not.
+    $automatic = drupal_get_form('system_test_system_settings_form');
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_false']['#default_value'], FALSE);
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_true']['#default_value'], TRUE);
+    $this->assertIdentical($automatic['wrapper']['system_settings_form_test_nested_checkbox_false']['#default_value'], FALSE);
+    $this->assertIdentical($automatic['wrapper']['system_settings_form_test_nested_checkbox_true']['#default_value'], TRUE);
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_false_no_automatic']['#default_value'], FALSE);
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_true_no_automatic']['#default_value'], TRUE);
+
+    $no_automatic = drupal_get_form('system_test_system_settings_form', FALSE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_false']['#default_value'], FALSE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_true']['#default_value'], TRUE);
+    $this->assertIdentical($no_automatic['wrapper']['system_settings_form_test_nested_checkbox_false']['#default_value'], FALSE);
+    $this->assertIdentical($no_automatic['wrapper']['system_settings_form_test_nested_checkbox_true']['#default_value'], TRUE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_false_no_automatic']['#default_value'], FALSE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_true_no_automatic']['#default_value'], TRUE);
+
+    // Set the corresponding system variables to the opposite of their default.
+    variable_set('system_settings_form_test_checkbox_false', TRUE);
+    variable_set('system_settings_form_test_checkbox_true', FALSE);
+    variable_set('system_settings_form_test_nested_checkbox_false', TRUE);
+    variable_set('system_settings_form_test_nested_checkbox_true', FALSE);
+    variable_set('system_settings_form_test_checkbox_false_no_automatic', TRUE);
+    variable_set('system_settings_form_test_checkbox_true_no_automatic', FALSE);
+
+    // Ensure that when there are saved system variables, elements use those as
+    // their #default_value when system_settings_form() is called normally
+    // unless the element explicitly turns off automatic defaults, and that all
+    // elements use their form builder assigned #default_value when
+    // system_settings_form() is called with FALSE passed as the
+    // $automatic_defaults parameter.
+    $automatic = drupal_get_form('system_test_system_settings_form');
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_false']['#default_value'], TRUE);
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_true']['#default_value'], FALSE);
+    $this->assertIdentical($automatic['wrapper']['system_settings_form_test_nested_checkbox_false']['#default_value'], TRUE);
+    $this->assertIdentical($automatic['wrapper']['system_settings_form_test_nested_checkbox_true']['#default_value'], FALSE);
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_false_no_automatic']['#default_value'], FALSE);
+    $this->assertIdentical($automatic['system_settings_form_test_checkbox_true_no_automatic']['#default_value'], TRUE);
+
+    $no_automatic = drupal_get_form('system_test_system_settings_form', FALSE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_false']['#default_value'], FALSE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_true']['#default_value'], TRUE);
+    $this->assertIdentical($no_automatic['wrapper']['system_settings_form_test_nested_checkbox_false']['#default_value'], FALSE);
+    $this->assertIdentical($no_automatic['wrapper']['system_settings_form_test_nested_checkbox_true']['#default_value'], TRUE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_false_no_automatic']['#default_value'], FALSE);
+    $this->assertIdentical($no_automatic['system_settings_form_test_checkbox_true_no_automatic']['#default_value'], TRUE);
   }
 }