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

includes/common.inc

@@ -155,6 +155,46 @@ function drupal_get_headers() {
  * Functions to properly handle HTTP responses.
  */
 
+/**
+ * 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
@@ -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);
   }
 }
 

includes/pager.inc

@@ -83,6 +83,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.
  *
@@ -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

includes/tablesort.inc

@@ -19,18 +19,6 @@ function tablesort_init($header) {
   return $ts;
 }
 
-/**
- * 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.
  *
@@ -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)));
 }
 
 /**

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);
 }
 

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);
 }
 

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;
 }

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;
 }

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }
 

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;
 }

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;
 }