Issue #1410574 by tim.plunkett, sun, bleen18, danillonunes: Make l() respect...

Issue #1410574 by tim.plunkett, sun, bleen18, danillonunes: Make l() respect the URL query string before adding the 'active' CSS class.

core/includes/common.inc

@@ -2320,12 +2320,20 @@ function l($text, $path, array $options = array()) {
   // Merge in defaults.
   $options += array(
     'attributes' => array(),
+    'query' => array(),
     'html' => FALSE,
   );
 
   // Append active class.
-  if (($path == current_path() || ($path == '<front>' && drupal_is_front_page())) &&
-      (empty($options['language']) || $options['language']->langcode == language(LANGUAGE_TYPE_URL)->langcode)) {
+  // The link is only active, if its path corresponds to the current path, the
+  // language of the linked path is equal to the current language, and if the
+  // query parameters of the link equal those of the current request, since the
+  // same request with different query parameters may yield a different page
+  // (e.g., pagers).
+  $is_active = ($path == current_path() || ($path == '<front>' && drupal_is_front_page()));
+  $is_active = $is_active && (empty($options['language']) || $options['language']->langcode == language(LANGUAGE_TYPE_URL)->langcode);
+  $is_active = $is_active && (drupal_container()->get('request')->query->all() == $options['query']);
+  if ($is_active) {
     $options['attributes']['class'][] = 'active';
   }
 

core/includes/pager.inc

@@ -377,13 +377,7 @@ function theme_pager_link($variables) {
     }
   }
 
-  // @todo l() cannot be used here, since it adds an 'active' class based on the
-  //   path only (which is always the current path for pager links). Apparently,
-  //   none of the pager links is active at any time - but it should still be
-  //   possible to use l() here.
-  // @see http://drupal.org/node/1410574
-  $attributes['href'] = url(current_path(), array('query' => $query));
-  return '<a' . new Attribute($attributes) . '>' . check_plain($text) . '</a>';
+  return l($text, current_path(), array('query' => $query, 'attributes' => $attributes));
 }
 
 /**

core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php

@@ -16,6 +16,9 @@
  * inheriting from a web test case rather than a unit test case.
  */
 class UrlTest extends WebTestBase {
+
+  public static $modules = array('common_test');
+
   public static function getInfo() {
     return array(
       'name' => 'URL generation tests',
@@ -35,12 +38,28 @@ function testLXSS() {
     $this->assertTrue(strpos($link, $sanitized_path) !== FALSE, format_string('XSS attack @path was filtered', array('@path' => $path)));
   }
 
-  /*
+  /**
    * Tests for active class in l() function.
    */
   function testLActiveClass() {
-    $link = l($this->randomName(), current_path());
-    $this->assertTrue($this->hasClass($link, 'active'), format_string('Class @class is present on link to the current page', array('@class' => 'active')));
+    $path = 'common-test/l-active-class';
+    $options = array();
+
+    $this->drupalGet($path, $options);
+    $links = $this->xpath('//a[@href = :href and contains(@class, :class)]', array(':href' => url($path, $options), ':class' => 'active'));
+    $this->assertTrue(isset($links[0]), 'A link to the current page is marked active.');
+
+    $options = array('query' => array('foo' => 'bar'));
+    $links = $this->xpath('//a[@href = :href and not(contains(@class, :class))]', array(':href' => url($path, $options), ':class' => 'active'));
+    $this->assertTrue(isset($links[0]), 'A link to the current page with a query string when the current page has no query string is not marked active.');
+
+    $this->drupalGet($path, $options);
+    $links = $this->xpath('//a[@href = :href and contains(@class, :class)]', array(':href' => url($path, $options), ':class' => 'active'));
+    $this->assertTrue(isset($links[0]), 'A link to the current page with a query string that matches the current query string is marked active.');
+
+    $options = array();
+    $links = $this->xpath('//a[@href = :href and not(contains(@class, :class))]', array(':href' => url($path, $options), ':class' => 'active'));
+    $this->assertTrue(isset($links[0]), 'A link to the current page without a query string when the current page has a query string is not marked active.');
   }
 
   /**
@@ -50,7 +69,6 @@ function testLCustomClass() {
     $class = $this->randomName();
     $link = l($this->randomName(), current_path(), array('attributes' => array('class' => array($class))));
     $this->assertTrue($this->hasClass($link, $class), format_string('Custom class @class is present on link when requested', array('@class' => $class)));
-    $this->assertTrue($this->hasClass($link, 'active'), format_string('Class @class is present on link to the current page', array('@class' => 'active')));
   }
 
   /**

core/modules/system/tests/modules/common_test/common_test.module

@@ -64,6 +64,12 @@ function common_test_menu() {
     'access arguments' => array('access content'),
     'type' => MENU_CALLBACK,
   );
+  $items['common-test/l-active-class'] = array(
+    'title' => 'Test l() adding of active class',
+    'page callback' => 'common_test_l_active_class',
+    'access arguments' => array('access content'),
+    'type' => MENU_CALLBACK,
+  );
   return $items;
 }
 
@@ -311,3 +317,26 @@ function common_test_js_and_css_querystring() {
 function common_test_cron() {
   throw new Exception(t('Uncaught exception'));
 }
+
+/**
+ * Page callback: Displays links to the current page, one with a query string.
+ */
+function common_test_l_active_class() {
+  return array(
+    'no_query' => array(
+      '#type' => 'link',
+      '#title' => t('Link with no query string'),
+      '#href' => current_path(),
+    ),
+    'with_query' => array(
+      '#type' => 'link',
+      '#title' => t('Link with a query string'),
+      '#href' => current_path(),
+      '#options' => array(
+        'query' => array(
+          'foo' => 'bar',
+        ),
+      ),
+    ),
+  );
+}

core/update.php

@@ -399,6 +399,8 @@ function update_check_requirements($skip_warnings = FALSE) {
 // @todo These two lines were copied from index.php which has its own todo about
 // a change required here. Revisit this when that change has been made.
 $request = Request::createFromGlobals();
+drupal_container()
+  ->set('request', $request);
 
 // There can be conflicting 'op' parameters because both update and batch use
 // this parameter name. We need the 'op' coming from a POST request to trump