- Patch #634440 by fago, effulgentsia, sun: remove auto-rebuilding magic for ()['storage'].

4 files changed, 48 insertions, 21 deletions

diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index f0a5ec12a..8c043378e 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -1465,13 +1465,8 @@ function drupal_anonymous_user($session = '') {
  *
  */
 function drupal_bootstrap($phase = NULL, $new_phase = TRUE) {
-  $final_phase = &drupal_static(__FUNCTION__ . '_final_phase');
-  // When not recursing, store the phase name so it's not forgotten while
-  // recursing.
-  if ($new_phase) {
-    $final_phase = $phase;
-  }
-  $phases = &drupal_static(__FUNCTION__ . '_phases', array(
+  // Not drupal_static(), because does not depend on any run-time information.
+  static $phases = array(
     DRUPAL_BOOTSTRAP_CONFIGURATION,
     DRUPAL_BOOTSTRAP_PAGE_CACHE,
     DRUPAL_BOOTSTRAP_DATABASE,
@@ -1480,9 +1475,19 @@ function drupal_bootstrap($phase = NULL, $new_phase = TRUE) {
     DRUPAL_BOOTSTRAP_PAGE_HEADER,
     DRUPAL_BOOTSTRAP_LANGUAGE,
     DRUPAL_BOOTSTRAP_FULL,
-  ));
-  $completed_phase = &drupal_static(__FUNCTION__ . '_completed_phase', -1);
+  );
+  // Not drupal_static(), because the only legitimate API to control this is to
+  // call drupal_bootstrap() with a new phase parameter.
+  static $final_phase;
+  // Not drupal_static(), because it's impossible to roll back to an earlier
+  // bootstrap state.
+  static $completed_phase = -1;
 
+  // When not recursing, store the phase name so it's not forgotten while
+  // recursing.
+  if ($new_phase) {
+    $final_phase = $phase;
+  }
   if (isset($phase)) {
     // Call a phase if it has not been called before and is below the requested
     // phase.
diff --git a/includes/form.inc b/includes/form.inc
index 1e42f6eae..170badbb0 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -104,6 +104,21 @@ function drupal_get_form($form_id) {
  *     - file: An optional include file that contains the form and is
  *       automatically loaded by form_get_cache(). Defaults to the current menu
  *       router item's 'file' definition, if existent.
+ *   - storage: An array that may be used to store information related to the
+ *     processed data in the form, which can be accessed and used within the
+ *     same page request, but also persist across multiple steps in a multi-step
+ *     form.
+ *   - rebuild: Normally, after the entire form processing is completed and
+ *     submit handlers ran, a form is considered to be done and
+ *     drupal_redirect_form() will redirect the user to a new page using a GET
+ *     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.
  *   - 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.
@@ -224,13 +239,12 @@ function drupal_build_form($form_id, &$form_state) {
   // the form will simply be re-rendered with the values still in its
   // fields.
   //
-  // If $form_state['storage'] or $form_state['rebuild'] has been set
-  // and the form has been submitted, we know that we're in a complex
-  // 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
+  // If $form_state['rebuild'] has been set and the form has been submitted, 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 ((!empty($form_state['storage']) || $form_state['rebuild']) && $form_state['submitted'] && !form_get_errors()) {
+  if ($form_state['rebuild'] && $form_state['submitted'] && !form_get_errors()) {
     $form = drupal_rebuild_form($form_id, $form_state);
   }
 
@@ -255,7 +269,7 @@ function form_state_defaults() {
     'rebuild' => FALSE,
     'redirect' => NULL,
     'build_info' => array(),
-    'storage' => NULL,
+    'storage' => array(),
     'submitted' => FALSE,
     'programmed' => FALSE,
     'cache'=> FALSE,
@@ -758,8 +772,7 @@ function drupal_validate_form($form_id, $form, &$form_state) {
  * - If $form_state['programmed'] is TRUE, the form submission was usually
  *   invoked via drupal_form_submit(), so any redirection would break the script
  *   that invoked drupal_form_submit().
- * - If $form_state['rebuild'] is TRUE or $form_state['storage'] is populated,
- *   the form is most probably a multi-step form and needs to be rebuilt without
+ * - If $form_state['rebuild'] is TRUE, the form needs to be rebuilt without
  *   redirection.
  *
  * @param $form_state
@@ -773,8 +786,8 @@ function drupal_redirect_form($form_state) {
   if (!empty($form_state['programmed'])) {
     return;
   }
-  // Skip redirection for multi-step forms.
-  if (!empty($form_state['rebuild']) || !empty($form_state['storage'])) {
+  // Skip redirection if rebuild is activated.
+  if (!empty($form_state['rebuild'])) {
     return;
   }
   // Skip redirection if it was explicitly disallowed.
@@ -3161,7 +3174,13 @@ function batch_process($redirect = NULL, $url = 'batch', $redirect_callback = 'd
  * Retrieves the current batch.
  */
 function &batch_get() {
-  $batch = &drupal_static(__FUNCTION__, array());
+  // Not drupal_static(), because Batch API operates at a lower level than most
+  // use-cases for resetting static variables, and we specifically do not want a
+  // global drupal_static_reset() resetting the batch information. Functions
+  // that are part of the Batch API and need to reset the batch information may
+  // call batch_get() and manipulate the result by reference. Functions that are
+  // not part of the Batch API can also do this, but shouldn't.
+  static $batch = array();
   return $batch;
 }
 
diff --git a/modules/simpletest/tests/form_test.module b/modules/simpletest/tests/form_test.module
index 5e420fc4a..2a4f867bd 100644
--- a/modules/simpletest/tests/form_test.module
+++ b/modules/simpletest/tests/form_test.module
@@ -346,6 +346,7 @@ function form_storage_test_form_submit($form, &$form_state) {
   else {
     drupal_set_message("Title: ". check_plain($form_state['storage']['thing']['title']));
   }
+  $form_state['rebuild'] = TRUE;
   $form_state['storage']['step']++;
   drupal_set_message("Form constructions: ". $_SESSION['constructions']);
 }
diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc
index 8201d0c9c..a8027d575 100644
--- a/modules/system/system.admin.inc
+++ b/modules/system/system.admin.inc
@@ -992,6 +992,7 @@ function system_modules_submit($form, &$form_state) {
     // the form submission data needed later.
     if (!isset($form_state['values']['confirm'])) {
       $form_state['storage'] = array('more_modules' => $more_modules, 'modules' => $modules);
+      $form_state['rebuild'] = TRUE;
       return;
     }
     // Otherwise, install or enable the modules.
@@ -1203,6 +1204,7 @@ function system_modules_uninstall_submit($form, &$form_state) {
   }
   else {
     $form_state['storage'] = $form_state['values'];
+    $form_state['rebuild'] = TRUE;
   }
 }