From ee8aa910b9db2f90bfb46ba852eaa349343d5e04 Mon Sep 17 00:00:00 2001
From: Dries Buytaert <dries@buytaert.net>
Date: Mon, 21 Sep 2009 06:44:14 +0000
Subject: [PATCH] Patch #579366 by sun, litwol | chx, Dries: simplified form
API redirection handling. I can actually understand it now. ;-).
---
includes/ajax.inc | 2 +-
includes/batch.inc | 27 +++-----
includes/common.inc | 17 +++--
includes/form.inc | 102 ++++++++++++++++++----------
modules/field_ui/field_ui.admin.inc | 2 +-
modules/node/node.pages.inc | 4 +-
modules/openid/openid.module | 4 +-
modules/openid/openid.pages.inc | 2 +-
modules/search/search.module | 4 +-
modules/system/system.admin.inc | 4 +-
modules/system/system.test | 8 +--
modules/taxonomy/taxonomy.admin.inc | 7 +-
modules/user/user.module | 2 +-
modules/user/user.pages.inc | 12 ++--
14 files changed, 103 insertions(+), 94 deletions(-)
diff --git a/includes/ajax.inc b/includes/ajax.inc
index 478adf572fdf..6bfd86d77554 100644
--- a/includes/ajax.inc
+++ b/includes/ajax.inc
@@ -184,7 +184,7 @@ function ajax_get_form() {
}
// Since some of the submit handlers are run, redirects need to be disabled.
- $form['#redirect'] = FALSE;
+ $form_state['no_redirect'] = TRUE;
// The form needs to be processed; prepare for that by setting a few internal
// variables.
diff --git a/includes/batch.inc b/includes/batch.inc
index b136ac99906c..5fe876f9d518 100644
--- a/includes/batch.inc
+++ b/includes/batch.inc
@@ -426,29 +426,24 @@ function _batch_finished() {
if ($_batch['progressive']) {
// Revert the 'destination' that was saved in batch_process().
if (isset($_batch['destination'])) {
- $_REQUEST['destination'] = $_batch['destination'];
+ $_GET['destination'] = $_batch['destination'];
}
// Determine the target path to redirect to.
- if (isset($_batch['form_state']['redirect'])) {
- $redirect = $_batch['form_state']['redirect'];
- }
- elseif (isset($_batch['redirect'])) {
- $redirect = $_batch['redirect'];
- }
- else {
- $redirect = $_batch['source_page'];
+ if (!isset($_batch['form_state']['redirect'])) {
+ if (isset($_batch['redirect'])) {
+ $_batch['form_state']['redirect'] = $_batch['redirect'];
+ }
+ else {
+ $_batch['form_state']['redirect'] = $_batch['source_page'];
+ }
}
// Use drupal_redirect_form() to handle the redirection logic.
- $form = isset($batch['form']) ? $batch['form'] : array();
- if (empty($_batch['form_state']['rebuild']) && empty($_batch['form_state']['storage'])) {
- drupal_redirect_form($form, $redirect);
- }
+ drupal_redirect_form($_batch['form_state']);
- // We get here if $form['#redirect'] was FALSE, or if the form is a
- // multi-step form. We save the final $form_state value to be retrieved
- // by drupal_get_form(), and redirect to the originating page.
+ // 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'];
drupal_goto($_batch['source_page']);
}
diff --git a/includes/common.inc b/includes/common.inc
index 2fee010a4ed5..479d0952eaae 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -364,8 +364,8 @@ function drupal_query_string_encode($query, $exclude = array(), $parent = '') {
* @see drupal_goto()
*/
function drupal_get_destination() {
- if (isset($_REQUEST['destination'])) {
- return 'destination=' . urlencode($_REQUEST['destination']);
+ if (isset($_GET['destination'])) {
+ return 'destination=' . urlencode($_GET['destination']);
}
else {
// Use $_GET here to retrieve the original path in source form.
@@ -418,9 +418,8 @@ function drupal_get_destination() {
* @see drupal_get_destination()
*/
function drupal_goto($path = '', $query = NULL, $fragment = NULL, $http_response_code = 302) {
-
- if (isset($_REQUEST['destination'])) {
- extract(parse_url(urldecode($_REQUEST['destination'])));
+ if (isset($_GET['destination'])) {
+ extract(parse_url(urldecode($_GET['destination'])));
}
$args = array(
@@ -471,8 +470,8 @@ function drupal_not_found() {
watchdog('page not found', check_plain($_GET['q']), NULL, WATCHDOG_WARNING);
// Keep old path for reference, and to allow forms to redirect to it.
- if (!isset($_REQUEST['destination'])) {
- $_REQUEST['destination'] = $_GET['q'];
+ if (!isset($_GET['destination'])) {
+ $_GET['destination'] = $_GET['q'];
}
$path = drupal_get_normal_path(variable_get('site_404', ''));
@@ -502,8 +501,8 @@ function drupal_access_denied() {
watchdog('access denied', check_plain($_GET['q']), NULL, WATCHDOG_WARNING);
// Keep old path for reference, and to allow forms to redirect to it.
- if (!isset($_REQUEST['destination'])) {
- $_REQUEST['destination'] = $_GET['q'];
+ if (!isset($_GET['destination'])) {
+ $_GET['destination'] = $_GET['q'];
}
$path = drupal_get_normal_path(variable_get('site_403', ''));
diff --git a/includes/form.inc b/includes/form.inc
index 30df84c54978..d92e2a7ee761 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -122,8 +122,13 @@ function drupal_get_form($form_id) {
* structure, which is passed on to the actual form builder function.
* Such forms cannot use drupal_get_form() and need to prepare $form_state
* on their own.
+ * Further $form_state properties controlling the redirection behavior after
+ * form submission may be found in drupal_redirect_form().
+ *
* @return
* The rendered form or NULL, depending upon the $form_state flags that were set.
+ *
+ * @see drupal_redirect_form()
*/
function drupal_build_form($form_id, &$form_state) {
// Ensure some defaults; if already set they will not be overridden.
@@ -503,6 +508,8 @@ function drupal_retrieve_form($form_id, &$form_state) {
}
/**
+ * Processes a form submission.
+ *
* This function is the heart of form API. The form gets built, validated and in
* appropriate cases, submitted.
*
@@ -579,20 +586,8 @@ function drupal_process_form($form_id, &$form, &$form_state) {
// Set a flag to indicate the the form has been processed and executed.
$form_state['executed'] = TRUE;
- // The form is executed. By default, we're finished now and redirect to a
- // new destination page. The path of the destination page can be set in
- // $form['#redirect'] or $form_state['redirect']. If neither of the two is
- // set, the user is redirect to the current page, which means a fresh,
- // unpopulated copy of the form.
- // Redirection is skipped, though, if
- // - the form was called by drupal_form_submit(),
- // - the form has to be rebuilt because either $form_state['rebuild'] was
- // set to TRUE or $form_state['storage'] was populated by a submit handler.
- // - $form_state['no_redirect'] is set to TRUE,
- // - $form_state['redirect'] or $form['#redirect'] is set to FALSE.
- if (!$form_state['programmed'] && empty($form_state['rebuild']) && empty($form_state['storage']) && empty($form_state['no_redirect'])) {
- drupal_redirect_form($form, $form_state['redirect']);
- }
+ // Redirect the form based on values in $form_state.
+ drupal_redirect_form($form_state);
}
}
}
@@ -732,33 +727,67 @@ function drupal_validate_form($form_id, $form, &$form_state) {
}
/**
- * Redirect the user to a URL after a form has been processed.
+ * Redirects the user to a URL after a form has been processed.
+ *
+ * After a form was executed, the data in $form_state controls whether the form
+ * is redirected. By default, we redirect to a new destination page. The path of
+ * the destination page can be set in $form_state['redirect']. If that is not
+ * set, the user is redirected to the current page to display a fresh,
+ * unpopulated copy of the form.
+ *
+ * There are several triggers that may prevent a redirection though:
+ * - If $form_state['redirect'] is FALSE, a form builder function or form
+ * validation/submit handler does not want a user to be redirected, which
+ * means that drupal_goto() is not invoked. For most forms, the redirection
+ * logic will be the same regardless of whether $form_state['redirect'] is
+ * undefined or FALSE. However, in case it was not defined and the current
+ * request contains a 'destination' query string, drupal_goto() will redirect
+ * to that given destination instead. Only setting $form_state['redirect'] to
+ * FALSE will prevent any redirection.
+ * - If $form_state['no_redirect'] is TRUE, then the callback that originally
+ * built the form explicitly disallows any redirection, regardless of the
+ * redirection value in $form_state['redirect']. For example, ajax_get_form()
+ * defines $form_state['no_redirect'] when building a form in an AJAX
+ * callback to prevent any redirection. $form_state['no_redirect'] should NOT
+ * be altered by form builder functions or form validation/submit handlers.
+ * - 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
+ * redirection.
*
- * @param $form
- * An associative array containing the structure of the form.
- * @param $redirect
- * An optional value containing the destination path to redirect
- * to if none is specified by the form.
+ * @param $form_state
+ * A keyed array containing the current state of the form.
+ *
+ * @see drupal_process_form()
+ * @see drupal_build_form()
*/
-function drupal_redirect_form($form, $redirect = NULL) {
- $goto = NULL;
- if (isset($redirect)) {
- $goto = $redirect;
+function drupal_redirect_form($form_state) {
+ // Skip redirection for form submissions invoked via drupal_form_submit().
+ if (!empty($form_state['programmed'])) {
+ return;
}
- if ($goto !== FALSE && isset($form['#redirect'])) {
- $goto = $form['#redirect'];
+ // Skip redirection for multi-step forms.
+ if (!empty($form_state['rebuild']) || !empty($form_state['storage'])) {
+ return;
+ }
+ // Skip redirection if it was explicitly disallowed.
+ if (!empty($form_state['no_redirect'])) {
+ return;
}
- if (!isset($goto) || ($goto !== FALSE)) {
- if (isset($goto)) {
- if (is_array($goto)) {
- call_user_func_array('drupal_goto', $goto);
+ // Only invoke drupal_goto() if redirect value was not set to FALSE.
+ if (!isset($form_state['redirect']) || $form_state['redirect'] !== FALSE) {
+ if (isset($form_state['redirect'])) {
+ if (is_array($form_state['redirect'])) {
+ call_user_func_array('drupal_goto', $form_state['redirect']);
}
else {
// This function can be called from the installer, which guarantees
// that $redirect will always be a string, so catch that case here
// and use the appropriate redirect function.
$function = drupal_installation_attempted() ? 'install_goto' : 'drupal_goto';
- $function($goto);
+ $function($form_state['redirect']);
}
}
drupal_goto($_GET['q']);
@@ -2922,12 +2951,11 @@ function batch_process($redirect = NULL, $url = NULL) {
$batch += $process_info;
if ($batch['progressive']) {
- // Clear the way for the drupal_goto redirection to the batch processing
- // page, by saving and unsetting the 'destination' if any, on both places
- // drupal_goto looks for it.
- if (isset($_REQUEST['destination'])) {
- $batch['destination'] = $_REQUEST['destination'];
- unset($_REQUEST['destination']);
+ // Clear the way for the drupal_goto() redirection to the batch processing
+ // page, by saving and unsetting the 'destination', if there is any.
+ if (isset($_GET['destination'])) {
+ $batch['destination'] = $_GET['destination'];
+ unset($_GET['destination']);
}
// Initiate db storage in order to get a batch id. We have to provide
diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc
index 6cdf699d449e..9335ecc8ae34 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -540,7 +540,7 @@ function field_ui_field_overview_form_submit($form, &$form_state) {
if ($destinations) {
$destinations[] = urldecode(substr(drupal_get_destination(), 12));
- unset($_REQUEST['destination']);
+ unset($_GET['destination']);
$form_state['redirect'] = field_ui_get_destinations($destinations);
}
}
diff --git a/modules/node/node.pages.inc b/modules/node/node.pages.inc
index a0bbf92b0ba4..e794fd0b24aa 100644
--- a/modules/node/node.pages.inc
+++ b/modules/node/node.pages.inc
@@ -301,9 +301,9 @@ function node_form($form, &$form_state, $node) {
*/
function node_form_delete_submit($form, &$form_state) {
$destination = '';
- if (isset($_REQUEST['destination'])) {
+ if (isset($_GET['destination'])) {
$destination = drupal_get_destination();
- unset($_REQUEST['destination']);
+ unset($_GET['destination']);
}
$node = $form['#node'];
$form_state['redirect'] = array('node/' . $node->nid . '/delete', $destination);
diff --git a/modules/openid/openid.module b/modules/openid/openid.module
index 0facb8ebde63..61edf053a774 100644
--- a/modules/openid/openid.module
+++ b/modules/openid/openid.module
@@ -445,7 +445,7 @@ function openid_authentication($response) {
$_SESSION['openid']['values'] = $form_state['values'];
// We'll want to redirect back to the same place.
$destination = drupal_get_destination();
- unset($_REQUEST['destination']);
+ unset($_GET['destination']);
drupal_goto('user/register', $destination);
}
else {
@@ -463,7 +463,7 @@ function openid_authentication($response) {
// Let other modules act on OpenID login
module_invoke_all('openid_response', $response, $account);
}
- drupal_redirect_form($form, $form_state['redirect']);
+ drupal_redirect_form($form_state);
}
else {
drupal_set_message(t('Only site administrators can create new user accounts.'), 'error');
diff --git a/modules/openid/openid.pages.inc b/modules/openid/openid.pages.inc
index b900c413c903..0091e33fa778 100644
--- a/modules/openid/openid.pages.inc
+++ b/modules/openid/openid.pages.inc
@@ -111,5 +111,5 @@ function openid_user_delete_form_submit($form, &$form_state) {
if ($query) {
drupal_set_message(t('OpenID deleted.'));
}
- $form_state['#redirect'] = 'user/' . $form_state['args'][0]->uid . '/openid';
+ $form_state['redirect'] = 'user/' . $form_state['args'][0]->uid . '/openid';
}
diff --git a/modules/search/search.module b/modules/search/search.module
index 35b817d3f0bd..9d15edb5e448 100644
--- a/modules/search/search.module
+++ b/modules/search/search.module
@@ -917,8 +917,8 @@ function search_box_form_submit($form, &$form_state) {
// functionality, so we override any static destination set in the request,
// for example by drupal_access_denied() or drupal_not_found()
// (see http://drupal.org/node/292565).
- if (isset($_REQUEST['destination'])) {
- unset($_REQUEST['destination']);
+ if (isset($_GET['destination'])) {
+ unset($_GET['destination']);
}
$form_id = $form['form_id']['#value'];
diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc
index 48806cdb41ba..2d8c81713aae 100644
--- a/modules/system/system.admin.inc
+++ b/modules/system/system.admin.inc
@@ -1822,7 +1822,7 @@ function system_site_maintenance_mode() {
* @ingroup forms
* @see system_settings_form()
*/
-function system_clean_url_settings($form) {
+function system_clean_url_settings($form, &$form_state) {
global $base_url;
// When accessing this form using a non-clean URL, allow a re-check to make
@@ -1852,7 +1852,7 @@ function system_clean_url_settings($form) {
else {
drupal_add_js(drupal_get_path('module', 'system') . '/system.js');
- $form['#redirect'] = $base_url . '/admin/config/search/clean-urls';
+ $form_state['redirect'] = $base_url . '/admin/config/search/clean-urls';
$form['clean_url_description'] = array(
'#type' => 'markup',
'#markup' => '<p>' . t('Use URLs like <code>example.com/user</code> instead of <code>example.com/?q=user</code>.') . ' ' . t('If you are directed to a <em>Page not found (404)</em> error after testing for clean URLs, see the <a href="@handbook">online handbook</a>.', array('@handbook' => 'http://drupal.org/node/15365')) . '</p>',
diff --git a/modules/system/system.test b/modules/system/system.test
index d6fd1a98e87b..6d1e4d14255a 100644
--- a/modules/system/system.test
+++ b/modules/system/system.test
@@ -512,20 +512,14 @@ class AdminMetaTagTestCase extends DrupalWebTestCase {
class AccessDeniedTestCase extends DrupalWebTestCase {
protected $admin_user;
- /**
- * Implement getInfo().
- */
public static function getInfo() {
return array(
'name' => '403 functionality',
- 'description' => "Tests page access denied functionality, including custom 403 pages.",
+ 'description' => 'Tests page access denied functionality, including custom 403 pages.',
'group' => 'System'
);
}
- /**
- * Implement setUp().
- */
function setUp() {
parent::setUp();
diff --git a/modules/taxonomy/taxonomy.admin.inc b/modules/taxonomy/taxonomy.admin.inc
index ee32be4b0824..b7cd4aba60bd 100644
--- a/modules/taxonomy/taxonomy.admin.inc
+++ b/modules/taxonomy/taxonomy.admin.inc
@@ -469,10 +469,7 @@ function taxonomy_overview_terms($form, &$form_state, $vocabulary) {
'#type' => 'submit',
'#value' => t('Reset to alphabetical')
);
- $form['destination'] = array(
- '#type' => 'hidden',
- '#value' => $_GET['q'] . (isset($_GET['page']) ? '?page=' . $_GET['page'] : '')
- );
+ $form_state['redirect'] = array($_GET['q'], (isset($_GET['page']) ? array('page' => $_GET['page']) : ''));
}
return $form;
@@ -780,7 +777,7 @@ function taxonomy_form_term($form, &$form_state, $vocabulary, $edit = array()) {
'#value' => $edit['tid']);
}
else {
- $form['destination'] = array('#type' => 'hidden', '#value' => $_GET['q']);
+ $form_state['redirect'] = $_GET['q'];
}
return $form;
diff --git a/modules/user/user.module b/modules/user/user.module
index 9cb594c73dde..cf41f9d7e007 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -3078,7 +3078,7 @@ function user_register($form, &$form_state) {
);
// Redirect back to page which initiated the create request;
// usually admin/people/create.
- $form['destination'] = array('#type' => 'hidden', '#value' => $_GET['q']);
+ $form_state['redirect'] = $_GET['q'];
}
// Create a dummy variable for pass-by-reference parameters.
diff --git a/modules/user/user.pages.inc b/modules/user/user.pages.inc
index ac1115a80fac..80e00ae49f14 100644
--- a/modules/user/user.pages.inc
+++ b/modules/user/user.pages.inc
@@ -292,9 +292,9 @@ function user_profile_form_submit($form, &$form_state) {
*/
function user_edit_cancel_submit($form, &$form_state) {
$destination = '';
- if (isset($_REQUEST['destination'])) {
+ if (isset($_GET['destination'])) {
$destination = drupal_get_destination();
- unset($_REQUEST['destination']);
+ unset($_GET['destination']);
}
// Note: We redirect from user/uid/edit to user/uid/cancel to make the tabs disappear.
$form_state['redirect'] = array("user/" . $form_state['values']['_account']->uid . "/cancel", $destination);
@@ -391,9 +391,7 @@ function user_cancel_confirm_form_submit($form, &$form_state) {
if (user_access('administer users') && empty($form_state['values']['user_cancel_confirm']) && $account->uid != $user->uid) {
user_cancel($form_state['values'], $account->uid, $form_state['values']['user_cancel_method']);
- if (!isset($_REQUEST['destination'])) {
- $form_state['redirect'] = 'admin/people';
- }
+ $form_state['redirect'] = 'admin/people';
}
else {
// Store cancelling method and whether to notify the user in $account for
@@ -407,9 +405,7 @@ function user_cancel_confirm_form_submit($form, &$form_state) {
drupal_set_message(t('A confirmation request to cancel your account has been sent to your e-mail address.'));
watchdog('user', 'Sent account cancellation request to %name %email.', array('%name' => $account->name, '%email' => '<' . $account->mail . '>'), WATCHDOG_NOTICE);
- if (!isset($_REQUEST['destination'])) {
- $form_state['redirect'] = "user/$account->uid";
- }
+ $form_state['redirect'] = "user/$account->uid";
}
}
--
GitLab