#5371, drupal_get_destination, pager and tablesort array handling, patch by Steven

15 files changed, 90 insertions, 55 deletions

diff --git a/includes/common.inc b/includes/common.inc
index 368aa64fe..263f01ac6 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -156,6 +156,46 @@ function drupal_get_headers() {
  */
 
 /**
+ * Parse an array into a valid urlencoded query string.
+ *
+ * @param $query
+ *   The array to be processed e.g. $_GET
+ * @param $exclude
+ *   The array filled with keys to be excluded. Use parent[child] to exclude nested items.
+ * @param $urlencode
+ *   If TRUE, the keys and values are both urlencoded.
+ * @param $parent
+ *   Should not be passed, only used in recursive calls
+ * @return
+ *   urlencoded string which can be appended to/as the URL query string
+ */
+function drupal_query_string_encode($query, $exclude = array(), $parent = '') {
+  $params = array();
+
+  foreach ($query as $key => $value) {
+    if ($parent) {
+      $key = $parent .'['. urlencode($key) .']';
+    }
+    else {
+      $key = urlencode($key);
+    }
+
+    if (in_array(urldecode($key), $exclude)) {
+      continue;
+    }
+
+    if (is_array($value)) {
+      $params[] = drupal_query_string_encode($value, $exclude, $key);
+    }
+    else {
+      $params[] = $key .'='. urlencode($value);
+    }
+  }
+
+  return implode('&', $params);
+}
+
+/**
  * Prepare a destination query string for use in combination with
  * drupal_goto(). Used to direct the user back to the referring page
  * after completing a form. By default the current URL is returned.
@@ -171,17 +211,11 @@ function drupal_get_destination() {
   }
   else {
     $path = $_GET['q'];
-    $params = array();
-    foreach ($_GET as $key => $value) {
-      if ($key == 'q') {
-        continue;
-      }
-      $params[] = urlencode($key) .'='. urlencode($value);
-    }
-    if (count($params)) {
-      $path .= '?';
+    $query = drupal_query_string_encode($_GET, array('q'));
+    if ($query != '') {
+      $path .= '?'. $query;
     }
-    return 'destination='. urlencode($path . implode('&', $params));
+    return 'destination='. urlencode($path);
   }
 }
 
diff --git a/includes/pager.inc b/includes/pager.inc
index 80d80dd82..9f36f2ab8 100644
--- a/includes/pager.inc
+++ b/includes/pager.inc
@@ -84,6 +84,21 @@ function pager_query($query, $limit = 10, $element = 0, $count_query = NULL) {
 }
 
 /**
+ * Compose a query string to append to pager requests.
+ *
+ * @return
+ *   A query string that consists of all components of the current page request
+ *   except for those pertaining to paging.
+ */
+function pager_get_querystring() {
+  static $string = NULL;
+  if (!isset($string)) {
+    $string = drupal_query_string_encode($_REQUEST, array_merge(array('q', 'page'), array_keys($_COOKIE)));
+  }
+  return $string;
+}
+
+/**
  * Format a query pager.
  *
  * Menu callbacks that display paged query results should call theme('pager') to
@@ -179,6 +194,7 @@ function theme_pager_previous($text, $limit, $element = 0, $interval = 1, $param
   // If we are anywhere but the first page
   if ($pager_page_array[$element] > 0) {
     $page_new = pager_load_array($pager_page_array[$element] - $interval, $element, $pager_page_array);
+
     // If the previous page is the first page, mark the link as such.
     if ($page_new[$element] == 0) {
       $output = theme('pager_first', $text, $limit, $element, $parameters);
@@ -357,8 +373,12 @@ function theme_pager_link($text, $page_new, $element, $parameters = array(), $at
   }
 
   $query = array();
-  foreach ($parameters as $key => $value) {
-    $query[] = $key .'='. $value;
+  if (count($parameters)) {
+    $query[] = drupal_query_string_encode($parameters, array());
+  }
+  $querystring = pager_get_querystring();
+  if ($querystring != '') {
+    $query[] = $querystring;
   }
 
   // Set each pager link title
diff --git a/includes/tablesort.inc b/includes/tablesort.inc
index 911afe88c..340caf4c0 100644
--- a/includes/tablesort.inc
+++ b/includes/tablesort.inc
@@ -20,18 +20,6 @@ function tablesort_init($header) {
 }
 
 /**
- * Fetch pager link arguments.
- *
- * When producing a sortable table that presents paged data, pass the output
- * of this function into theme('pager') to preserve the current sort order.
- */
-function tablesort_pager() {
-  $cgi = $_SERVER['REQUEST_METHOD'] == 'GET' ? $_GET : $_POST;
-  unset($cgi['q'], $cgi['page']);
-  return $cgi;
-}
-
-/**
  * Create an SQL sort clause.
  *
  * This function produces the ORDER BY clause to insert in your SQL queries,
@@ -86,7 +74,7 @@ function tablesort_header($cell, $header, $ts) {
       $ts['sort'] = 'asc';
       $image = '';
     }
-    $cell['data'] = l($cell['data'] . $image, $_GET['q'], array('title' => $title), 'sort='. $ts['sort'] .'&order='. urlencode($cell['data']). $ts['query_string'], NULL, FALSE, TRUE);
+    $cell['data'] = l($cell['data'] . $image, $_GET['q'], array('title' => $title), 'sort='. $ts['sort'] .'&order='. urlencode($cell['data']) .'&'. $ts['query_string'], NULL, FALSE, TRUE);
 
     unset($cell['field'], $cell['sort']);
   }
@@ -134,14 +122,7 @@ function tablesort_cell($cell, $header, $ts, $i) {
  *   except for those pertaining to table sorting.
  */
 function tablesort_get_querystring() {
-  $cgi = $_SERVER['REQUEST_METHOD'] == 'GET' ? $_GET : $_POST;
-  $query_string = '';
-  foreach ($cgi as $key => $val) {
-    if ($key != 'order' && $key != 'sort' && $key != 'q') {
-      $query_string .= '&'. $key .'='. $val;
-    }
-  }
-  return $query_string;
+  return drupal_query_string_encode($_REQUEST, array_merge(array('q', 'sort', 'order'), array_keys($_COOKIE)));
 }
 
 /**
diff --git a/modules/comment.module b/modules/comment.module
index 2d9877a96..f81c632ce 100644
--- a/modules/comment.module
+++ b/modules/comment.module
@@ -980,7 +980,7 @@ function comment_admin_overview($type = 'new') {
     $form['operations'][$comment->cid] = array('#value' => l(t('edit'), 'comment/edit/'. $comment->cid, array(), $destination));
   }
   $form['comments'] = array('#type' => 'checkboxes', '#options' => $comments);
-  $form['pager'] = array('#value' => theme('pager', NULL, 50, 0, tablesort_pager()));
+  $form['pager'] = array('#value' => theme('pager', NULL, 50, 0));
   return drupal_get_form('comment_admin_overview', $form);
 }
 
diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index 2d9877a96..f81c632ce 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -980,7 +980,7 @@ function comment_admin_overview($type = 'new') {
     $form['operations'][$comment->cid] = array('#value' => l(t('edit'), 'comment/edit/'. $comment->cid, array(), $destination));
   }
   $form['comments'] = array('#type' => 'checkboxes', '#options' => $comments);
-  $form['pager'] = array('#value' => theme('pager', NULL, 50, 0, tablesort_pager()));
+  $form['pager'] = array('#value' => theme('pager', NULL, 50, 0));
   return drupal_get_form('comment_admin_overview', $form);
 }
 
diff --git a/modules/forum.module b/modules/forum.module
index 756b52e35..21b045651 100644
--- a/modules/forum.module
+++ b/modules/forum.module
@@ -1002,7 +1002,7 @@ function theme_forum_topic_list($tid, $topics, $sortby, $forum_per_page) {
   }
 
   $output .= theme('table', $forum_topic_list_header, $rows);
-  $output .= theme('pager', NULL, $forum_per_page, 0, tablesort_pager());
+  $output .= theme('pager', NULL, $forum_per_page, 0);
 
   return $output;
 }
diff --git a/modules/forum/forum.module b/modules/forum/forum.module
index 756b52e35..21b045651 100644
--- a/modules/forum/forum.module
+++ b/modules/forum/forum.module
@@ -1002,7 +1002,7 @@ function theme_forum_topic_list($tid, $topics, $sortby, $forum_per_page) {
   }
 
   $output .= theme('table', $forum_topic_list_header, $rows);
-  $output .= theme('pager', NULL, $forum_per_page, 0, tablesort_pager());
+  $output .= theme('pager', NULL, $forum_per_page, 0);
 
   return $output;
 }
diff --git a/modules/path.module b/modules/path.module
index 160e4a7d0..adb6c5f1d 100644
--- a/modules/path.module
+++ b/modules/path.module
@@ -307,7 +307,7 @@ function path_overview() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 50, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 50, 0);
   return $output;
 }
 
diff --git a/modules/path/path.module b/modules/path/path.module
index 160e4a7d0..adb6c5f1d 100644
--- a/modules/path/path.module
+++ b/modules/path/path.module
@@ -307,7 +307,7 @@ function path_overview() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 50, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 50, 0);
   return $output;
 }
 
diff --git a/modules/statistics.module b/modules/statistics.module
index a4d8bc628..d01983bce 100644
--- a/modules/statistics.module
+++ b/modules/statistics.module
@@ -192,7 +192,7 @@ function statistics_node_tracker() {
 
     drupal_set_title(check_plain($node->title));
     $output = theme('table', $header, $rows);
-    $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+    $output .= theme('pager', NULL, 30, 0);
     return $output;
   }
   else {
@@ -218,7 +218,7 @@ function statistics_user_tracker() {
 
     drupal_set_title($account->name);
     $output = theme('table', $header, $rows);
-    $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+    $output .= theme('pager', NULL, 30, 0);
     return $output;
   }
   else {
@@ -249,7 +249,7 @@ function statistics_recent_hits() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
@@ -275,7 +275,7 @@ function statistics_top_pages() {
 
   drupal_set_title(t('Top pages in the past %interval', array('%interval' => format_interval(variable_get('statistics_flush_accesslog_timer', 259200)))));
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
@@ -303,7 +303,7 @@ function statistics_top_visitors() {
 
   drupal_set_title(t('Top visitors in the past %interval', array('%interval' => format_interval(variable_get('statistics_flush_accesslog_timer', 259200)))));
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
@@ -329,7 +329,7 @@ function statistics_top_referrers() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
diff --git a/modules/statistics/statistics.module b/modules/statistics/statistics.module
index a4d8bc628..d01983bce 100644
--- a/modules/statistics/statistics.module
+++ b/modules/statistics/statistics.module
@@ -192,7 +192,7 @@ function statistics_node_tracker() {
 
     drupal_set_title(check_plain($node->title));
     $output = theme('table', $header, $rows);
-    $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+    $output .= theme('pager', NULL, 30, 0);
     return $output;
   }
   else {
@@ -218,7 +218,7 @@ function statistics_user_tracker() {
 
     drupal_set_title($account->name);
     $output = theme('table', $header, $rows);
-    $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+    $output .= theme('pager', NULL, 30, 0);
     return $output;
   }
   else {
@@ -249,7 +249,7 @@ function statistics_recent_hits() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
@@ -275,7 +275,7 @@ function statistics_top_pages() {
 
   drupal_set_title(t('Top pages in the past %interval', array('%interval' => format_interval(variable_get('statistics_flush_accesslog_timer', 259200)))));
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
@@ -303,7 +303,7 @@ function statistics_top_visitors() {
 
   drupal_set_title(t('Top visitors in the past %interval', array('%interval' => format_interval(variable_get('statistics_flush_accesslog_timer', 259200)))));
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
@@ -329,7 +329,7 @@ function statistics_top_referrers() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 30, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 30, 0);
   return $output;
 }
 
diff --git a/modules/user.module b/modules/user.module
index d64068b84..3e15ef3e9 100644
--- a/modules/user.module
+++ b/modules/user.module
@@ -1907,7 +1907,7 @@ function user_admin_account() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 50, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 50, 0);
   return $output;
 }
 
diff --git a/modules/user/user.module b/modules/user/user.module
index d64068b84..3e15ef3e9 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -1907,7 +1907,7 @@ function user_admin_account() {
   }
 
   $output = theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 50, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 50, 0);
   return $output;
 }
 
diff --git a/modules/watchdog.module b/modules/watchdog.module
index 0f620b2cc..f4fd83bc6 100644
--- a/modules/watchdog.module
+++ b/modules/watchdog.module
@@ -139,7 +139,7 @@ function watchdog_overview() {
   }
 
   $output .= theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 50, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 50, 0);
 
   return $output;
 }
diff --git a/modules/watchdog/watchdog.module b/modules/watchdog/watchdog.module
index 0f620b2cc..f4fd83bc6 100644
--- a/modules/watchdog/watchdog.module
+++ b/modules/watchdog/watchdog.module
@@ -139,7 +139,7 @@ function watchdog_overview() {
   }
 
   $output .= theme('table', $header, $rows);
-  $output .= theme('pager', NULL, 50, 0, tablesort_pager());
+  $output .= theme('pager', NULL, 50, 0);
 
   return $output;
 }