- Patch #578520 by sun | c960657, mfb, Dries, catch, mattyoung: make in url() only accept an array. Another nice API clean-up!

22 files changed, 289 insertions, 121 deletions

diff --git a/modules/aggregator/aggregator.test b/modules/aggregator/aggregator.test
index 2439adbbc..d3162ff15 100644
--- a/modules/aggregator/aggregator.test
+++ b/modules/aggregator/aggregator.test
@@ -56,7 +56,7 @@ class AggregatorTestCase extends DrupalWebTestCase {
     $feed_name = $this->randomName(10);
     if (!$feed_url) {
       $feed_url = url('rss.xml', array(
-        'query' => 'feed=' . $feed_name,
+        'query' => array('feed' => $feed_name),
         'absolute' => TRUE,
       ));
     }
diff --git a/modules/book/book.module b/modules/book/book.module
index 5b6be5766..eed90fbaa 100644
--- a/modules/book/book.module
+++ b/modules/book/book.module
@@ -73,7 +73,7 @@ function book_node_view_link($node, $build_mode) {
         $links['book_add_child'] = array(
           'title' => t('Add child page'),
           'href' => 'node/add/' . str_replace('_', '-', $child_type),
-          'query' => 'parent=' . $node->book['mlid'],
+          'query' => array('parent' => $node->book['mlid']),
         );
       }
 
diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index c6516519b..8a8e43f94 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -445,7 +445,7 @@ function comment_new_page_count($num_comments, $new_replies, $node) {
   }
 
   if ($pageno >= 1) {
-    $pagenum = "page=" . intval($pageno);
+    $pagenum = array('page' => intval($pageno));
   }
 
   return $pagenum;
@@ -2153,10 +2153,10 @@ function theme_comment_post_forbidden($node) {
       // We cannot use drupal_get_destination() because these links
       // sometimes appear on /node and taxonomy listing pages.
       if (variable_get('comment_form_location_' . $node->type, COMMENT_FORM_BELOW) == COMMENT_FORM_SEPARATE_PAGE) {
-        $destination = 'destination=' . rawurlencode("comment/reply/$node->nid#comment-form");
+        $destination = array('destination' => "comment/reply/$node->nid#comment-form");
       }
       else {
-        $destination = 'destination=' . rawurlencode("node/$node->nid#comment-form");
+        $destination = array('destination' => "node/$node->nid#comment-form");
       }
 
       if (variable_get('user_register', 1)) {
diff --git a/modules/comment/comment.test b/modules/comment/comment.test
index bd835680c..216b41aa5 100644
--- a/modules/comment/comment.test
+++ b/modules/comment/comment.test
@@ -308,7 +308,7 @@ class CommentInterfaceTest extends CommentHelperCase {
     $this->setCommentsPerPage(2);
     $comment_new_page = $this->postComment($this->node, $this->randomName(), $this->randomName(), TRUE);
     $this->assertTrue($this->commentExists($comment_new_page), t('Page one exists. %s'));
-    $this->drupalGet('node/' . $this->node->nid, array('query' => 'page=1'));
+    $this->drupalGet('node/' . $this->node->nid, array('query' => array('page' => 1)));
     $this->assertTrue($this->commentExists($reply, TRUE), t('Page two exists. %s'));
     $this->setCommentsPerPage(50);
 
@@ -588,13 +588,13 @@ class CommentPagerTest extends CommentHelperCase {
     $this->assertFalse($this->commentExists($comments[2]), t('Comment 3 does not appear on page 1.'));
 
     // Check the second page.
-    $this->drupalGet('node/' . $node->nid, array('query' => 'page=1'));
+    $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 1)));
     $this->assertTrue($this->commentExists($comments[1]), t('Comment 2 appears on page 2.'));
     $this->assertFalse($this->commentExists($comments[0]), t('Comment 1 does not appear on page 2.'));
     $this->assertFalse($this->commentExists($comments[2]), t('Comment 3 does not appear on page 2.'));
 
     // Check the third page.
-    $this->drupalGet('node/' . $node->nid, array('query' => 'page=2'));
+    $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 2)));
     $this->assertTrue($this->commentExists($comments[2]), t('Comment 3 appears on page 3.'));
     $this->assertFalse($this->commentExists($comments[0]), t('Comment 1 does not appear on page 3.'));
     $this->assertFalse($this->commentExists($comments[1]), t('Comment 2 does not appear on page 3.'));
@@ -608,21 +608,21 @@ class CommentPagerTest extends CommentHelperCase {
     $this->setCommentsPerPage(2);
     // We are still in flat view - the replies should not be on the first page,
     // even though they are replies to the oldest comment.
-    $this->drupalGet('node/' . $node->nid, array('query' => 'page=0'));
+    $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 0)));
     $this->assertFalse($this->commentExists($reply, TRUE), t('In flat mode, reply does not appear on page 1.'));
 
     // If we switch to threaded mode, the replies on the oldest comment
     // should be bumped to the first page and comment 6 should be bumped
     // to the second page.
     $this->setCommentSettings('comment_default_mode', COMMENT_MODE_THREADED, t('Switched to threaded mode.'));
-    $this->drupalGet('node/' . $node->nid, array('query' => 'page=0'));
+    $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 0)));
     $this->assertTrue($this->commentExists($reply, TRUE), t('In threaded mode, reply appears on page 1.'));
     $this->assertFalse($this->commentExists($comments[1]), t('In threaded mode, comment 2 has been bumped off of page 1.'));
 
     // If (# replies > # comments per page) in threaded expanded view,
     // the overage should be bumped.
     $reply2 = $this->postComment(NULL, $this->randomName(), $this->randomName(), FALSE, TRUE);
-    $this->drupalGet('node/' . $node->nid, array('query' => 'page=0'));
+    $this->drupalGet('node/' . $node->nid, array('query' => array('page' => 0)));
     $this->assertFalse($this->commentExists($reply2, TRUE), t('In threaded mode where # replies > # comments per page, the newest reply does not appear on page 1.'));
 
     $this->drupalLogout();
diff --git a/modules/contact/contact.pages.inc b/modules/contact/contact.pages.inc
index 319a34eb6..de1a20caf 100644
--- a/modules/contact/contact.pages.inc
+++ b/modules/contact/contact.pages.inc
@@ -155,7 +155,7 @@ function contact_personal_page($account) {
   global $user;
 
   if (!valid_email_address($user->mail)) {
-    $output = t('You need to provide a valid e-mail address to contact other users. Please update your <a href="@url">user information</a> and try again.', array('@url' => url("user/$user->uid/edit", array('query' => 'destination=' . drupal_get_destination()))));
+    $output = t('You need to provide a valid e-mail address to contact other users. Please update your <a href="@url">user information</a> and try again.', array('@url' => url("user/$user->uid/edit", array('query' => drupal_get_destination()))));
   }
   elseif (!flood_is_allowed('contact', variable_get('contact_threshold_limit', 5), variable_get('contact_threshold_window', 3600)) && !user_access('administer site-wide contact form')) {
     $output = t("You cannot send more than %number messages in @interval. Please try again later.", array('%number' => variable_get('contact_threshold_limit', 3), '@interval' => format_interval(variable_get('contact_threshold_window', 3600))));
diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc
index ae5ce6368..3096eb602 100644
--- a/modules/field_ui/field_ui.admin.inc
+++ b/modules/field_ui/field_ui.admin.inc
@@ -539,7 +539,8 @@ function field_ui_field_overview_form_submit($form, &$form_state) {
   }
 
   if ($destinations) {
-    $destinations[] = urldecode(substr(drupal_get_destination(), 12));
+    $destination = drupal_get_destination();
+    $destinations[] = $destination['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 a2643ee5f..969cc357a 100644
--- a/modules/node/node.pages.inc
+++ b/modules/node/node.pages.inc
@@ -300,7 +300,7 @@ function node_form($form, &$form_state, $node) {
  * Button submit function: handle the 'Delete' button on the node form.
  */
 function node_form_delete_submit($form, &$form_state) {
-  $destination = '';
+  $destination = array();
   if (isset($_GET['destination'])) {
     $destination = drupal_get_destination();
     unset($_GET['destination']);
diff --git a/modules/openid/tests/openid_test.module b/modules/openid/tests/openid_test.module
index 731233009..6fb503c43 100644
--- a/modules/openid/tests/openid_test.module
+++ b/modules/openid/tests/openid_test.module
@@ -229,5 +229,5 @@ function _openid_test_endpoint_authenticate() {
   // Put the signed message into the query string of a URL supplied by the
   // Relying Party, and redirect the user.
   drupal_set_header('Content-Type', 'text/plain');
-  header('Location: ' . url($_REQUEST['openid_return_to'], array('query' => http_build_query($response, '', '&'), 'external' => TRUE)));
+  header('Location: ' . url($_REQUEST['openid_return_to'], array('query' => $response, 'external' => TRUE)));
 }
diff --git a/modules/search/search.test b/modules/search/search.test
index 6472dc14d..7ffcbf2ab 100644
--- a/modules/search/search.test
+++ b/modules/search/search.test
@@ -499,7 +499,7 @@ class SearchCommentTestCase extends DrupalWebTestCase {
 
     // Invoke search index update.
     $this->drupalLogout();
-    $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . variable_get('cron_key', 'drupal')));
+    $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal'))));
 
     // Search for $title.
     $edit = array(
@@ -521,7 +521,7 @@ class SearchCommentTestCase extends DrupalWebTestCase {
 
     // Invoke search index update.
     $this->drupalLogout();
-    $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . variable_get('cron_key', 'drupal')));
+    $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal'))));
 
     // Search for $title.
     $this->drupalPost('', $edit, t('Search'));
diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php
index 1a49c9b5b..37c5964bc 100644
--- a/modules/simpletest/drupal_web_test_case.php
+++ b/modules/simpletest/drupal_web_test_case.php
@@ -1005,7 +1005,7 @@ class DrupalWebTestCase extends DrupalTestCase {
     // Make a request to the logout page, and redirect to the user page, the
     // idea being if you were properly logged out you should be seeing a login
     // screen.
-    $this->drupalGet('user/logout', array('query' => 'destination=user'));
+    $this->drupalGet('user/logout', array('query' => array('destination' => 'user')));
     $pass = $this->assertField('name', t('Username field found.'), t('Logout'));
     $pass = $pass && $this->assertField('pass', t('Password field found.'), t('Logout'));
 
@@ -1779,7 +1779,7 @@ class DrupalWebTestCase extends DrupalTestCase {
 
     $this->assertTrue(isset($urls[$index]), t('Clicked link %label (@url_target) from @url_before', array('%label' => $label, '@url_target' => $url_target, '@url_before' => $url_before)), t('Browser'));
 
-    if (isset($urls[$index])) {
+    if (isset($url_target)) {
       return $this->drupalGet($url_target);
     }
     return FALSE;
@@ -1793,12 +1793,16 @@ class DrupalWebTestCase extends DrupalTestCase {
    *   query, too. Can even be an absolute path which is just passed through.
    * @return
    *   An absolute path.
+   *
+   * @todo What is the intention of this function? It is only invoked from
+   *   locations, where URLs from the *output* are turned into absolute URLs,
+   *   so why do we pass that through url() again?
    */
   protected function getAbsoluteUrl($path) {
-    $options = array('absolute' => TRUE);
     $parts = parse_url($path);
-    // This is more crude than the menu_is_external but enough here.
+    // This is more crude than menu_path_is_external() but enough here.
     if (empty($parts['host'])) {
+      $options = array('absolute' => TRUE);
       $path = $parts['path'];
       $base_path = base_path();
       $n = strlen($base_path);
@@ -1806,7 +1810,19 @@ class DrupalWebTestCase extends DrupalTestCase {
         $path = substr($path, $n);
       }
       if (isset($parts['query'])) {
-        $options['query'] = $parts['query'];
+        parse_str($parts['query'], $options['query']);
+        // Let's make it a bit more crude. It's not clear why we invoke url() on
+        // a URL contained in the returned page output again, but since $path is
+        // FALSE (see $path = $parts['path'] above) with Clean URLs disabled,
+        // and url() now encodes the passed in query parameter array, we get a
+        // double-encoded 'q' query string in the resulting absolute URL
+        // generated by url(). This cannot be avoided in url(). But it could be
+        // completely avoided if this function wouldn't be calling url().
+        // @see SimpleTestURLTestCase->testGetAbsoluteUrl()
+        if (isset($options['query']['q'])) {
+          $path = $options['query']['q'];
+          unset($options['query']['q']);
+        }
       }
       $path = url($path, $options);
     }
@@ -2499,7 +2515,8 @@ class DrupalWebTestCase extends DrupalTestCase {
    */
   protected function verbose($message) {
     if ($id = simpletest_verbose($message)) {
-      $this->pass(l(t('Verbose message'), $this->originalFileDirectory . '/simpletest/verbose/' . get_class($this) . '-' . $id . '.html', array('attributes' => array('target' => '_blank'))), 'Debug');
+      $url = file_create_url($this->originalFileDirectory . '/simpletest/verbose/' . get_class($this) . '-' . $id . '.html');
+      $this->pass(l(t('Verbose message'), $url, array('attributes' => array('target' => '_blank'))), 'Debug');
     }
   }
 
diff --git a/modules/simpletest/simpletest.test b/modules/simpletest/simpletest.test
index 9d14abee7..f6d0f56c6 100644
--- a/modules/simpletest/simpletest.test
+++ b/modules/simpletest/simpletest.test
@@ -270,6 +270,43 @@ class SimpleTestFunctionalTest extends DrupalWebTestCase {
   }
 }
 
+/**
+ * Test internal testing framework URL handling.
+ */
+class SimpleTestURLTestCase extends DrupalWebTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'SimpleTest URL handling',
+      'description' => 'Test the URL handling in the testing framework.',
+      'group' => 'SimpleTest',
+    );
+  }
+
+  /**
+   * Test DrupalWebTestCase::getAbsoluteUrl().
+   */
+  function testGetAbsoluteUrl() {
+    // Testbed runs with Clean URLs disabled, so disable it here.
+    $GLOBALS['conf']['clean_url'] = 0;
+    $url = 'user/login';
+
+    $this->drupalGet($url);
+    $absolute = url($url, array('absolute' => TRUE));
+    $this->assertEqual($absolute, $this->url, t('Passed and requested URL are equal.'));
+    $this->assertEqual($this->url, $this->getAbsoluteUrl($url), t('Requested and returned absolute URL are equal.'));
+
+    $this->drupalPost(NULL, array(), t('Log in'));
+    $this->assertEqual($absolute, $this->url, t('Passed and requested URL are equal.'));
+    $this->assertEqual($this->url, $this->getAbsoluteUrl($url), t('Requested and returned absolute URL are equal.'));
+
+    $this->clickLink('Create new account');
+    $url = 'user/register';
+    $absolute = url($url, array('absolute' => TRUE));
+    $this->assertEqual($absolute, $this->url, t('Passed and requested URL are equal.'));
+    $this->assertEqual($this->url, $this->getAbsoluteUrl($url), t('Requested and returned absolute URL are equal.'));
+  }
+}
+
 class SimpleTestMailCaptureTestCase extends DrupalWebTestCase {
   /**
    * Implement getInfo().
diff --git a/modules/simpletest/tests/browser_test.module b/modules/simpletest/tests/browser_test.module
index 4d4fd24a3..cf107d850 100644
--- a/modules/simpletest/tests/browser_test.module
+++ b/modules/simpletest/tests/browser_test.module
@@ -58,7 +58,7 @@ function browser_test_print_post_form_submit($form, &$form_state) {
 
 function browser_test_refresh_meta() {
   if (!isset($_GET['refresh'])) {
-    $url = url('browser_test/refresh/meta', array('absolute' => TRUE, 'query' => 'refresh=true'));
+    $url = url('browser_test/refresh/meta', array('absolute' => TRUE, 'query' => array('refresh' => 'true')));
     drupal_add_html_head('<meta http-equiv="Refresh" content="0; URL=' . $url . '">');
     return '';
   }
@@ -68,7 +68,7 @@ function browser_test_refresh_meta() {
 
 function browser_test_refresh_header() {
   if (!isset($_GET['refresh'])) {
-    $url = url('browser_test/refresh/header', array('absolute' => TRUE, 'query' => 'refresh=true'));
+    $url = url('browser_test/refresh/header', array('absolute' => TRUE, 'query' => array('refresh' => 'true')));
     drupal_set_header('Location', $url);
     return '';
   }
diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test
index d32369441..32857bf11 100644
--- a/modules/simpletest/tests/common.test
+++ b/modules/simpletest/tests/common.test
@@ -2,14 +2,13 @@
 // $Id$
 
 /**
- * Tests for the l() function.
+ * Tests for URL generation functions.
  */
-class CommonLUnitTest extends DrupalUnitTestCase {
-
+class CommonURLUnitTest extends DrupalUnitTestCase {
   public static function getInfo() {
     return array(
       'name' => 'URL generation tests',
-      'description' => 'Confirm that url(), drupal_query_string_encode(), and l() work correctly with various input.',
+      'description' => 'Confirm that url(), drupal_get_query_parameters(), drupal_http_build_query(), and l() work correctly with various input.',
       'group' => 'System',
     );
   }
@@ -26,48 +25,196 @@ class CommonLUnitTest extends DrupalUnitTestCase {
   }
 
   /**
-    * Test drupal_query_string_encode().
-    */
-   function testDrupalQueryStringEncode() {
-     $this->assertEqual(drupal_query_string_encode(array('a' => ' &#//+%20@۞')), 'a=%20%26%23%2F%2F%2B%2520%40%DB%9E', t('Value was properly encoded.'));
-     $this->assertEqual(drupal_query_string_encode(array(' &#//+%20@۞' => 'a')), '%20%26%23%2F%2F%2B%2520%40%DB%9E=a', t('Key was properly encoded.'));
-     $this->assertEqual(drupal_query_string_encode(array('a' => '1', 'b' => '2', 'c' => '3'), array('b')), 'a=1&c=3', t('Value was properly excluded.'));
-     $this->assertEqual(drupal_query_string_encode(array('a' => array('b' => '2', 'c' => '3')), array('b', 'a[c]')), 'a[b]=2', t('Nested array was properly encoded.'));
-   }
-   
+   * Test drupal_get_query_parameters().
+   */
+  function testDrupalGetQueryParameters() {
+    $original = array(
+      'a' => 1,
+      'b' => array(
+        'd' => 4,
+        'e' => array(
+          'f' => 5,
+        ),
+      ),
+      'c' => 3,
+      'q' => 'foo/bar',
+    );
+
+    // Default arguments.
+    $result = $_GET;
+    unset($result['q']);
+    $this->assertEqual(drupal_get_query_parameters(), $result, t("\$_GET['q'] was removed."));
+
+    // Default exclusion.
+    $result = $original;
+    unset($result['q']);
+    $this->assertEqual(drupal_get_query_parameters($original), $result, t("'q' was removed."));
+
+    // First-level exclusion.
+    $result = $original;
+    unset($result['b']);
+    $this->assertEqual(drupal_get_query_parameters($original, array('b')), $result, t("'b' was removed."));
+
+    // Second-level exclusion.
+    $result = $original;
+    unset($result['b']['d']);
+    $this->assertEqual(drupal_get_query_parameters($original, array('b[d]')), $result, t("'b[d]' was removed."));
+
+    // Third-level exclusion.
+    $result = $original;
+    unset($result['b']['e']['f']);
+    $this->assertEqual(drupal_get_query_parameters($original, array('b[e][f]')), $result, t("'b[e][f]' was removed."));
+
+    // Multiple exclusions.
+    $result = $original;
+    unset($result['a'], $result['b']['e'], $result['c']);
+    $this->assertEqual(drupal_get_query_parameters($original, array('a', 'b[e]', 'c')), $result, t("'a', 'b[e]', 'c' were removed."));
+  }
+
+  /**
+   * Test drupal_http_build_query().
+   */
+  function testDrupalHttpBuildQuery() {
+    $this->assertEqual(drupal_http_build_query(array('a' => ' &#//+%20@۞')), 'a=%20%26%23//%2B%2520%40%DB%9E', t('Value was properly encoded.'));
+    $this->assertEqual(drupal_http_build_query(array(' &#//+%20@۞' => 'a')), '%20%26%23%2F%2F%2B%2520%40%DB%9E=a', t('Key was properly encoded.'));
+    $this->assertEqual(drupal_http_build_query(array('a' => '1', 'b' => '2', 'c' => '3')), 'a=1&b=2&c=3', t('Multiple values were properly concatenated.'));
+    $this->assertEqual(drupal_http_build_query(array('a' => array('b' => '2', 'c' => '3'), 'd' => 'foo')), 'a[b]=2&a[c]=3&d=foo', t('Nested array was properly encoded.'));
+  }
+
+  /**
+   * Test drupal_parse_url().
+   */
+  function testDrupalParseUrl() {
+    // Relative URL.
+    $url = 'foo/bar?foo=bar&bar=baz&baz#foo';
+    $result = array(
+      'path' => 'foo/bar',
+      'query' => array('foo' => 'bar', 'bar' => 'baz', 'baz' => ''),
+      'fragment' => 'foo',
+    );
+    $this->assertEqual(drupal_parse_url($url), $result, t('Relative URL parsed correctly.'));
+
+    // Absolute URL.
+    $url = '/foo/bar?foo=bar&bar=baz&baz#foo';
+    $result = array(
+      'path' => '/foo/bar',
+      'query' => array('foo' => 'bar', 'bar' => 'baz', 'baz' => ''),
+      'fragment' => 'foo',
+    );
+    $this->assertEqual(drupal_parse_url($url), $result, t('Absolute URL parsed correctly.'));
+
+    // External URL.
+    $url = 'http://drupal.org/foo/bar?foo=bar&bar=baz&baz#foo';
+    $result = array(
+      'path' => 'http://drupal.org/foo/bar',
+      'query' => array('foo' => 'bar', 'bar' => 'baz', 'baz' => ''),
+      'fragment' => 'foo',
+    );
+    $this->assertEqual(drupal_parse_url($url), $result, t('External URL parsed correctly.'));
+  }
+
   /**
    * Test url() with/without query, with/without fragment, absolute on/off and
    * assert all that works when clean URLs are on and off.
    */
   function testUrl() {
     global $base_url;
-    
+
     foreach (array(FALSE, TRUE) as $absolute) {
       // Get the expected start of the path string.
       $base = $absolute ? $base_url . '/' : base_path();
       $absolute_string = $absolute ? 'absolute' : NULL;
-      
-      // Run tests with clean urls disabled.
+
+      // Disable Clean URLs.
       $GLOBALS['conf']['clean_url'] = 0;
-      $clean_urls = 'disabled';
-      
-      $this->assertTrue(url('node', array('absolute' => $absolute)) == $base . '?q=node', t('Creation of @absolute internal url with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('node', array('fragment' => 'foo', 'absolute' => $absolute)) == $base . '?q=node#foo', t('Creation of @absolute internal url with fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('node', array('query' => 'foo', 'absolute' => $absolute)) == $base . '?q=node&foo', t('Creation of @absolute internal url with query option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('node', array('query' => 'foo', 'fragment' => 'bar', 'absolute' => $absolute)) == $base . '?q=node&foo#bar', t('Creation of @absolute internal url with query and fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('<front>', array('absolute' => $absolute)) == $base, t('Creation of @absolute internal url using front with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-  
-      // Run tests again with clean urls enabled.
+
+      $url = $base . '?q=node/123';
+      $result = url('node/123', array('absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . '?q=node/123#foo';
+      $result = url('node/123', array('fragment' => 'foo', 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . '?q=node/123&foo';
+      $result = url('node/123', array('query' => array('foo' => NULL), 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . '?q=node/123&foo=bar&bar=baz';
+      $result = url('node/123', array('query' => array('foo' => 'bar', 'bar' => 'baz'), 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . '?q=node/123&foo#bar';
+      $result = url('node/123', array('query' => array('foo' => NULL), 'fragment' => 'bar', 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base;
+      $result = url('<front>', array('absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      // Enable Clean URLs.
       $GLOBALS['conf']['clean_url'] = 1;
-      $clean_urls = 'enabled';
-      
-      $this->assertTrue(url('node', array('absolute' => $absolute)) == $base . 'node', t('Creation of @absolute internal url with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('node', array('fragment' => 'foo', 'absolute' => $absolute)) == $base . 'node#foo', t('Creation of @absolute internal url with fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('node', array('query' => 'foo', 'absolute' => $absolute)) == $base . 'node?foo', t('Creation of @absolute internal url with query option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('node', array('query' => 'foo', 'fragment' => 'bar', 'absolute' => $absolute)) == $base . 'node?foo#bar', t('Creation of @absolute internal url with query and fragment option with clean urls @clean_urls.', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
-      $this->assertTrue(url('<front>', array('absolute' => $absolute)) == $base, t('Creation of @absolute internal url using front with clean urls @clean_urls', array('@absolute' => $absolute_string, '@clean_urls' => $clean_urls)));
+
+      $url = $base . 'node/123';
+      $result = url('node/123', array('absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . 'node/123#foo';
+      $result = url('node/123', array('fragment' => 'foo', 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . 'node/123?foo';
+      $result = url('node/123', array('query' => array('foo' => NULL), 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . 'node/123?foo=bar&bar=baz';
+      $result = url('node/123', array('query' => array('foo' => 'bar', 'bar' => 'baz'), 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base . 'node/123?foo#bar';
+      $result = url('node/123', array('query' => array('foo' => NULL), 'fragment' => 'bar', 'absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
+
+      $url = $base;
+      $result = url('<front>', array('absolute' => $absolute));
+      $this->assertEqual($url, $result, "$url == $result");
     }
   }
+
+  /**
+   * Test external URL handling.
+   */
+  function testExternalUrls() {
+    $test_url = 'http://drupal.org/';
+
+    // Verify external URL can contain a fragment.
+    $url = $test_url . '#drupal';
+    $result = url($url);
+    $this->assertEqual($url, $result, t('External URL with fragment works without a fragment in $options.'));
+
+    // Verify fragment can be overidden in an external URL.
+    $url = $test_url . '#drupal';
+    $fragment = $this->randomName(10);
+    $result = url($url, array('fragment' => $fragment));
+    $this->assertEqual($test_url . '#' . $fragment, $result, t('External URL fragment is overidden with a custom fragment in $options.'));
+
+    // Verify external URL can contain a query string.
+    $url = $test_url . '?drupal=awesome';
+    $result = url($url);
+    $this->assertEqual($url, $result, t('External URL with query string works without a query string in $options.'));
+
+    // Verify external URL can be extended with a query string.
+    $url = $test_url;
+    $query = array($this->randomName(5) => $this->randomName(5));
+    $result = url($url, array('query' => $query));
+    $this->assertEqual($url . '?' . http_build_query($query, '', '&'), $result, t('External URL can be extended with a query string in $options.'));
+
+    // Verify query string can be extended in an external URL.
+    $url = $test_url . '?drupal=awesome';
+    $query = array($this->randomName(5) => $this->randomName(5));
+    $result = url($url, array('query' => $query));
+    $this->assertEqual($url . '&' . http_build_query($query, '', '&'), $result, t('External URL query string can be extended with a custom query string in $options.'));
+  }
 }
 
 class CommonSizeTestCase extends DrupalUnitTestCase {
@@ -225,54 +372,6 @@ class DrupalTagsHandlingTestCase extends DrupalWebTestCase {
 }
 
 /**
- * Tests url().
- */
-class UrlTestCase extends DrupalWebtestCase {
-
-  public static function getInfo() {
-    return array(
-      'name' => 'Tests for the url() function',
-      'description' => 'Performs tests on the url() function.',
-      'group' => 'System',
-    );
-  }
-
-  /**
-   * Test the url() function's $options array.
-   *
-   * Assert that calling url() with an external URL
-   *   1. containing a fragment works with and without a fragment in $options.
-   *   2. containing or not containing a query works with a query in $options.
-   */
-  function testUrlOptions() {
-    // Testing the fragment handling.
-    $fragment = $this->randomName(10);
-    $test_url = 'http://www.drupal.org/#' . $fragment;
-
-    $result_url = url($test_url);
-    $this->assertEqual($test_url, $result_url, t("External URL containing a fragment works without a fragment in options. url('http://drupal.org/#frag1');"));
-
-    $result_url = url($test_url, array('fragment' => $fragment));
-    $this->assertEqual($test_url, $result_url, t("External URL containing a fragment works with a fragment in options. url('http://drupal.org/#frag1', array('fragment' => 'frag1'));"));
-
-    // Testing the query handling.
-    $query = $this->randomName(10);
-    $query2 = $this->randomName(10);
-    $test_url = 'http://www.drupal.org/?' . $query;
-
-    // The external URL contains a query.
-    $result_url = url($test_url, array('query' => $query2));
-    $this->assertEqual($test_url . '&' . $query2, $result_url, t("External URL with a query passed in the path paramater. url('http://drupal.org/?param1', array('query' => 'param2'));"));
-
-    // The external URL does not contain a query.
-    $test_url = 'http://www.drupal.org';
-    $result_url = url($test_url, array('query' => $query2));
-    $this->assertEqual($test_url . '?' . $query2, $result_url, t("External URL without a query passed in the path paramater. url('http://drupal.org', array('query' => 'param2'));"));
-  }
-
-}
-
-/**
  * Test the Drupal CSS system.
  */
 class CascadingStylesheetsTestCase extends DrupalWebTestCase {
@@ -563,9 +662,15 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
 
   function testDrupalGetDestination() {
     $query = $this->randomName(10);
-    $url = url('system-test/destination', array('absolute' => TRUE, 'query' => $query));
-    $this->drupalGet($url);
-    $this->assertText($query, t('The query passed to the page is correctly represented by drupal_get_detination().'));
+
+    // Verify that a 'destination' query string is used as destination.
+    $this->drupalGet('system-test/destination', array('query' => array('destination' => $query)));
+    $this->assertText('The destination: ' . $query, t('The given query string destination is determined as destination.'));
+
+    // Verify that the current path is used as destination.
+    $this->drupalGet('system-test/destination', array('query' => array($query => NULL)));
+    $url = 'system-test/destination?' . $query;
+    $this->assertText('The destination: ' . $url, t('The current path is determined as destination.'));
   }
 }
 
diff --git a/modules/simpletest/tests/form.test b/modules/simpletest/tests/form.test
index b8a10cf56..855fb3d08 100644
--- a/modules/simpletest/tests/form.test
+++ b/modules/simpletest/tests/form.test
@@ -447,10 +447,10 @@ class FormsFormStorageTestCase extends DrupalWebTestCase {
     $user = $this->drupalCreateUser(array('access content'));
     $this->drupalLogin($user);
 
-    $this->drupalPost('form_test/form-storage', array('title' => 'new', 'value' => 'value_is_set'), 'Continue', array('query' => 'cache=1'));
+    $this->drupalPost('form_test/form-storage', array('title' => 'new', 'value' => 'value_is_set'), 'Continue', array('query' => array('cache' => 1)));
     $this->assertText('Form constructions: 1', t('The form has been constructed one time till now.'));
 
-    $this->drupalPost(NULL, array(), 'Save', array('query' => 'cache=1'));
+    $this->drupalPost(NULL, array(), 'Save', array('query' => array('cache' => 1)));
     $this->assertText('Form constructions: 2', t('The form has been constructed two times till now.'));
     $this->assertText('Title: new', t('The form storage has stored the values.'));
   }
diff --git a/modules/simpletest/tests/system_test.module b/modules/simpletest/tests/system_test.module
index 90a8f28a0..73f5fbfb3 100644
--- a/modules/simpletest/tests/system_test.module
+++ b/modules/simpletest/tests/system_test.module
@@ -116,7 +116,8 @@ function system_test_redirect_invalid_scheme() {
 }
 
 function system_test_destination() {
-  return 'The destination: ' . drupal_get_destination();
+  $destination = drupal_get_destination();
+  return 'The destination: ' . $destination['destination'];
 }
 
 /**
diff --git a/modules/statistics/statistics.admin.inc b/modules/statistics/statistics.admin.inc
index dc92ba126..70137a7bb 100644
--- a/modules/statistics/statistics.admin.inc
+++ b/modules/statistics/statistics.admin.inc
@@ -130,9 +130,9 @@ function statistics_top_visitors() {
 
   $result = $query->execute();
   $rows = array();
+  $destination = drupal_get_destination();
   foreach ($result as $account) {
-    $qs = drupal_get_destination();
-    $ban_link =  $account->iid ? l(t('unblock IP address'), "admin/config/people/ip-blocking/delete/$account->iid", array('query' => $qs)) : l(t('block IP address'), "admin/config/people/ip-blocking/$account->hostname", array('query' => $qs));
+    $ban_link = $account->iid ? l(t('unblock IP address'), "admin/config/people/ip-blocking/delete/$account->iid", array('query' => $destination)) : l(t('block IP address'), "admin/config/people/ip-blocking/$account->hostname", array('query' => $destination));
     $rows[] = array($account->hits, ($account->uid ? theme('username', $account) : $account->hostname), format_interval(round($account->total / 1000)), (user_access('block IP addresses') && !$account->uid) ? $ban_link : '');
   }
 
diff --git a/modules/system/system.install b/modules/system/system.install
index 0f337467e..f49788a2f 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -172,7 +172,7 @@ function system_requirements($phase) {
     }
 
     $description .= ' ' . $t('You can <a href="@cron">run cron manually</a>.', array('@cron' => url('admin/reports/status/run-cron')));
-    $description .= '<br />' . $t('To run cron from outside the site, go to <a href="!cron">!cron</a>', array('!cron' => url($base_url . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . variable_get('cron_key', 'drupal')))));
+    $description .= '<br />' . $t('To run cron from outside the site, go to <a href="!cron">!cron</a>', array('!cron' => url($base_url . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal'))))));
 
     $requirements['cron'] = array(
       'title' => $t('Cron maintenance tasks'),
diff --git a/modules/system/system.module b/modules/system/system.module
index b8ff23d77..a4cc60ed1 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -2336,7 +2336,7 @@ function system_admin_compact_mode() {
 function system_admin_compact_page($mode = 'off') {
   global $user;
   user_save($user, array('admin_compact_mode' => ($mode == 'on')));
-  drupal_goto(drupal_get_destination());
+  drupal_goto();
 }
 
 /**
diff --git a/modules/system/system.test b/modules/system/system.test
index 6d1e4d142..3738cd2f5 100644
--- a/modules/system/system.test
+++ b/modules/system/system.test
@@ -379,12 +379,12 @@ class CronRunTestCase extends DrupalWebTestCase {
 
     // Run cron anonymously with a random cron key.
     $key = $this->randomName(16);
-    $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . $key));
+    $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => $key)));
     $this->assertResponse(403);
 
     // Run cron anonymously with the valid cron key.
     $key = variable_get('cron_key', 'drupal');
-    $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => 'cron_key=' . $key));
+    $this->drupalGet($base_url . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => $key)));
     $this->assertResponse(200);
 
     // Execute cron directly.
diff --git a/modules/translation/translation.pages.inc b/modules/translation/translation.pages.inc
index e80a301bb..50d8849ee 100644
--- a/modules/translation/translation.pages.inc
+++ b/modules/translation/translation.pages.inc
@@ -47,7 +47,7 @@ function translation_node_overview($node) {
       // No such translation in the set yet: help user to create it.
       $title = t('n/a');
       if (node_access('create', $node)) {
-        $options[] = l(t('add translation'), 'node/add/' . str_replace('_', '-', $node->type), array('query' => "translation=$node->nid&language=$language->language"));
+        $options[] = l(t('add translation'), 'node/add/' . str_replace('_', '-', $node->type), array('query' => array('translation' => $node->nid, 'language' => $language->language)));
       }
       $status = t('Not translated');
     }
diff --git a/modules/user/user.module b/modules/user/user.module
index 2de52e237..ad3690c03 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -3139,9 +3139,16 @@ function user_modules_uninstalled($modules) {
 }
 
 /**
- * Rewrite the destination to prevent redirecting to login page after login.
+ * Helper function to rewrite the destination to avoid redirecting to login page after login.
+ *
+ * Third-party authentication modules may use this function to determine the
+ * proper destination after a user has been properly logged in.
  */
 function user_login_destination() {
   $destination = drupal_get_destination();
-  return $destination == 'destination=user%2Flogin' ? 'destination=user' : $destination;
+  if ($destination['destination'] == 'user/login') {
+    $destination['destination'] = 'user';
+  }
+  return $destination;
 }
+
diff --git a/modules/user/user.pages.inc b/modules/user/user.pages.inc
index 25f7401bd..8bbb41a53 100644
--- a/modules/user/user.pages.inc
+++ b/modules/user/user.pages.inc
@@ -292,7 +292,7 @@ function user_profile_form_submit($form, &$form_state) {
  * Submit function for the 'Cancel account' button on the user edit form.
  */
 function user_edit_cancel_submit($form, &$form_state) {
-  $destination = '';
+  $destination = array();
   if (isset($_GET['destination'])) {
     $destination = drupal_get_destination();
     unset($_GET['destination']);