core/core.services.yml | 2 +- core/includes/common.inc | 9 +- core/lib/Drupal/Core/Utility/LinkGenerator.php | 89 +++++-- .../Drupal/Core/Utility/LinkGeneratorInterface.php | 11 +- core/modules/system/system.module | 1 + .../Tests/Core/Utility/LinkGeneratorTest.php | 250 ++++++++++++++++++-- 6 files changed, 317 insertions(+), 45 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 20b8651..395d994 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -270,7 +270,7 @@ services: - { name: persist } link_generator: class: Drupal\Core\Utility\LinkGenerator - arguments: ['@url_generator', '@module_handler', '@language_manager'] + arguments: ['@url_generator', '@module_handler', '@language_manager', '@path.alias_manager.cached', '@current_user'] calls: - [setRequest, ['@?request']] router.dynamic: diff --git a/core/includes/common.inc b/core/includes/common.inc index 87e2858..2f64ddb 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -1329,17 +1329,16 @@ function l($text, $path, array $options = array()) { // Set the "active" class if the 'set_active_class' option is not empty. if (!empty($variables['options']['set_active_class'])) { if (\Drupal::currentUser()->isAuthenticated()) { - // Add a data-drupal-link-type attribute to let JavaScript know this link is - // used for navigation (as opposed to?) and so is potentially eligible for an - // active class. + // Add a "data-drupal-link-query" attribute to let the drupal.active-link + // library know the query in a standardized manner. if (!empty($variables['options']['query'])) { $query = $variables['options']['query']; ksort($query); $variables['options']['attributes']['data-drupal-link-query'] = Json::encode($query); } - // Add a data-drupal-link-system-path attribute to expose the system path for - // this link to JavaScript. + // Add a "data-drupal-link-system-path" attribute to let the + // drupal.active-link library know the path in a standardized manner. if (!isset($variables['options']['attributes']['data-drupal-link-system-path'])) { $variables['options']['attributes']['data-drupal-link-system-path'] = Drupal::service('path.alias_manager.cached')->getSystemPath($path); } diff --git a/core/lib/Drupal/Core/Utility/LinkGenerator.php b/core/lib/Drupal/Core/Utility/LinkGenerator.php index 3f63620..bc026c8 100644 --- a/core/lib/Drupal/Core/Utility/LinkGenerator.php +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php @@ -7,12 +7,15 @@ namespace Drupal\Core\Utility; +use Drupal\Component\Utility\Json; use Drupal\Component\Utility\String; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Language\Language; use Drupal\Core\Language\LanguageManager; +use Drupal\Core\Path\AliasManagerInterface; use Drupal\Core\Template\Attribute; use Drupal\Core\Routing\UrlGeneratorInterface; +use Drupal\Core\Session\AccountInterface; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\Request; @@ -50,6 +53,20 @@ class LinkGenerator implements LinkGeneratorInterface { protected $languageManager; /** + * The path alias manager. + * + * @var \Drupal\Core\Path\AliasManagerInterface + */ + protected $aliasManager; + + /** + * The current user service. + * + * @var \Drupal\Core\Session\AccountInterface + */ + protected $currentUser; + + /** * Constructs a LinkGenerator instance. * * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator @@ -58,11 +75,17 @@ class LinkGenerator implements LinkGeneratorInterface { * The module handler. * @param \Drupal\Core\Language\LanguageManager $language_manager * The language manager. + * @param \Drupal\Core\Path\AliasManagerInterface $alias_manager + * The path alias manager. + * @param \Drupal\Core\Session\AccountInterface $current_user + * The current user service. */ - public function __construct(UrlGeneratorInterface $url_generator, ModuleHandlerInterface $module_handler, LanguageManager $language_manager) { + public function __construct(UrlGeneratorInterface $url_generator, ModuleHandlerInterface $module_handler, LanguageManager $language_manager, AliasManagerInterface $alias_manager, AccountInterface $current_user) { $this->urlGenerator = $url_generator; $this->moduleHandler = $module_handler; $this->languageManager = $language_manager; + $this->aliasManager = $alias_manager; + $this->currentUser = $current_user; } /** @@ -93,6 +116,15 @@ public function getActive() { /** * {@inheritdoc} + * + * For anonymous users, the "active" class will be calculated on the server, + * because most sites serve each anonymous user the same cached page anyway. + * For authenticated users, the "active" class will be calculated on the + * client (through JavaScript), only data- attributes are added to links to + * prevent breaking the render cache. The JavaScript is added in + * system_page_build(). + * + * @see system_page_build() */ public function generate($text, $route_name, array $parameters = array(), array $options = array()) { // Start building a structured representation of our link to be altered later. @@ -110,30 +142,51 @@ public function generate($text, $route_name, array $parameters = array(), array 'query' => array(), 'html' => FALSE, 'language' => NULL, + 'set_active_class' => FALSE, ); + // Add a hreflang attribute if we know the language of this link's url and // hreflang has not already been set. if (!empty($variables['options']['language']) && !isset($variables['options']['attributes']['hreflang'])) { $variables['options']['attributes']['hreflang'] = $variables['options']['language']->id; } - // This is only needed for the active class. The generator also combines - // the parameters and $options['query'] and adds parameters that are not - // path slugs as query strings. - $full_parameters = $parameters + (array) $variables['options']['query']; - - // Determine whether this link is "active", meaning that it has the same - // URL path and query string as the current page. Note that this may be - // removed from l() in https://drupal.org/node/1979468 and would be removed - // or altered here also. - $variables['url_is_active'] = $route_name == $this->active['route_name'] - // The language of an active link is equal to the current language. - && (empty($variables['options']['language']) || $variables['options']['language']->id == $this->active['language']) - && $full_parameters == $this->active['parameters']; - - // Add the "active" class if appropriate. - if ($variables['url_is_active']) { - $variables['options']['attributes']['class'][] = 'active'; + // Set the "active" class if the 'set_active_class' option is not empty. + if (!empty($variables['options']['set_active_class'])) { + if ($this->currentUser->isAuthenticated()) { + // Add a "data-drupal-link-query" attribute to let the + // drupal.active-link library know the query in a standardized manner. + if (!empty($variables['options']['query'])) { + $query = $variables['options']['query']; + ksort($query); + $variables['options']['attributes']['data-drupal-link-query'] = Json::encode($query); + } + + // Add a "data-drupal-link-system-path" attribute to let the + // drupal.active-link library know the path in a standardized manner. + if (!isset($variables['options']['attributes']['data-drupal-link-system-path'])) { + $path = $this->urlGenerator->getPathFromRoute($route_name, $parameters); + $variables['options']['attributes']['data-drupal-link-system-path'] = $this->aliasManager->getSystemPath($path); + } + } + else { + // This is only needed for the active class. The generator also combines + // the parameters and $options['query'] and adds parameters that are not + // path slugs as query strings. + $full_parameters = $parameters + (array) $variables['options']['query']; + + // Determine whether this link is "active", meaning that it has the same + // URL path and query string as the current page. + $variables['url_is_active'] = $route_name == $this->active['route_name'] + // The language of an active link is equal to the current language. + && (empty($variables['options']['language']) || $variables['options']['language']->id == $this->active['language']) + && $full_parameters == $this->active['parameters']; + + // Add the "active" class if appropriate. + if ($variables['url_is_active']) { + $variables['options']['attributes']['class'][] = 'active'; + } + } } // Remove all HTML and PHP tags from a tooltip, calling expensive strip_tags() diff --git a/core/lib/Drupal/Core/Utility/LinkGeneratorInterface.php b/core/lib/Drupal/Core/Utility/LinkGeneratorInterface.php index 8bc7eb6..7772520 100644 --- a/core/lib/Drupal/Core/Utility/LinkGeneratorInterface.php +++ b/core/lib/Drupal/Core/Utility/LinkGeneratorInterface.php @@ -36,8 +36,8 @@ * @param array $options * (optional) An associative array of additional options. Defaults to an * empty array. It may contain the following elements: - * - 'query': An array of query key/value-pairs (without any URL-encoding) to - * append to the URL. + * - 'query': An array of query key/value-pairs (without any URL-encoding) + * to append to the URL. * - absolute: Whether to force the output to be an absolute link (beginning * with http:). Useful for links that will be displayed outside the site, * such as in an RSS feed. Defaults to FALSE. @@ -55,6 +55,13 @@ * internal to the site, $options['language'] is used to determine whether * the link is "active", or pointing to the current page (the language as * well as the path must match). + * - 'set_active_class' (default FALSE): Whether this method should compare + * the $route_name, $parameters, language and query options to the current + * URL to determine whether the link is "active", and if so, apply an + * "active" class to the link. It is important to use this sparingly as + * any content containing a link generated with active class processing + * enabled is incompatible with render caching elements on more than a + * per-page basis. * * @return string * An HTML string containing a link to the given route and parameters. diff --git a/core/modules/system/system.module b/core/modules/system/system.module index 6ce5f8e..e2213f8 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -2147,6 +2147,7 @@ function system_page_build(&$page) { // Load the active-link library if this is an authenticated user. // @see l() + // @see \Drupal\Core\Utility\LinkGenerator::generate() if (\Drupal::currentUser()->isAuthenticated()) { $page['#attached']['library'][] = array('system', 'drupal.active-link'); } diff --git a/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php b/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php index 6b7334f..8cd8f56 100644 --- a/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php +++ b/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php @@ -43,7 +43,6 @@ class LinkGeneratorTest extends UnitTestCase { protected $moduleHandler; /** - * * The mocked language manager. * * @var \PHPUnit_Framework_MockObject_MockObject @@ -51,12 +50,27 @@ class LinkGeneratorTest extends UnitTestCase { protected $languageManager; /** + * The mocked path alias manager. + * + * @var \Drupal\Core\Path\AliasManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $aliasManager; + + /** + * The mocked current user service. + * + * @var \Drupal\Core\Session\AccountInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $currentUser; + + /** * Contains the LinkGenerator default options. */ protected $defaultOptions = array( 'query' => array(), 'html' => FALSE, 'language' => NULL, + 'set_active_class' => FALSE, ); /** @@ -80,8 +94,10 @@ protected function setUp() { $this->urlGenerator = $this->getMock('\Drupal\Core\Routing\UrlGenerator', array(), array(), '', FALSE); $this->moduleHandler = $this->getMock('Drupal\Core\Extension\ModuleHandlerInterface'); $this->languageManager = $this->getMock('Drupal\Core\Language\LanguageManager'); + $this->aliasManager = $this->getMock('\Drupal\Core\Path\AliasManagerInterface'); + $this->currentUser = $this->getMock('\Drupal\Core\Session\AccountInterface'); - $this->linkGenerator = new LinkGenerator($this->urlGenerator, $this->moduleHandler, $this->languageManager); + $this->linkGenerator = new LinkGenerator($this->urlGenerator, $this->moduleHandler, $this->languageManager, $this->aliasManager, $this->currentUser); } /** @@ -303,7 +319,7 @@ public function testGenerateWithHtml() { } /** - * Tests the active class on the link method. + * Tests the active class on the link method for anonymous users. * * @see \Drupal\Core\Utility\LinkGenerator::generate() * @@ -311,20 +327,20 @@ public function testGenerateWithHtml() { * links to the front page when drupal_is_front_page() is converted to a * service. */ - public function testGenerateActive() { - $this->urlGenerator->expects($this->exactly(7)) + public function testGenerateActiveAnonymous() { + $this->currentUser->expects($this->exactly(7)) + ->method('isAuthenticated') + ->will($this->returnValue(FALSE)); + + $this->urlGenerator->expects($this->exactly(8)) ->method('generateFromRoute') ->will($this->returnValueMap(array( array('test_route_1', array(), FALSE, '/test-route-1'), - array('test_route_1', array(), FALSE, '/test-route-1'), - array('test_route_1', array(), FALSE, '/test-route-1'), - array('test_route_1', array(), FALSE, '/test-route-1'), - array('test_route_3', array(), FALSE, '/test-route-3'), array('test_route_3', array(), FALSE, '/test-route-3'), array('test_route_4', array('object' => '1'), FALSE, '/test-route-4/1'), ))); - $this->moduleHandler->expects($this->exactly(7)) + $this->moduleHandler->expects($this->exactly(8)) ->method('alter'); $this->setUpLanguageManager(); @@ -332,7 +348,7 @@ public function testGenerateActive() { // Render a link with a path different from the current path. $request = new Request(array(), array(), array('system_path' => 'test-route-2')); $this->linkGenerator->setRequest($request); - $result = $this->linkGenerator->generate('Test', 'test_route_1'); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); $this->assertNotTag(array( 'tag' => 'a', 'attributes' => array('class' => 'active'), @@ -345,14 +361,28 @@ public function testGenerateActive() { $raw_variables = new ParameterBag(); $request->attributes->set('_raw_variables', $raw_variables); $this->linkGenerator->setRequest($request); - $result = $this->linkGenerator->generate('Test', 'test_route_1'); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); $this->assertTag(array( 'tag' => 'a', 'attributes' => array('class' => 'active'), ), $result); + // Render a link with the same path as the current path, but with the + // set_active_class option disabled. + $request = new Request(array(), array(), array('system_path' => 'test-route-1', RouteObjectInterface::ROUTE_NAME => 'test_route_1')); + // This attribute is expected to be set in a Drupal request by + // \Drupal\Core\ParamConverter\ParamConverterManager + $raw_variables = new ParameterBag(); + $request->attributes->set('_raw_variables', $raw_variables); + $this->linkGenerator->setRequest($request); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => FALSE)); + $this->assertNotTag(array( + 'tag' => 'a', + 'attributes' => array('class' => 'active'), + ), $result); + // Render a link with the same path and language as the current path. - $result = $this->linkGenerator->generate('Test', 'test_route_1'); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); $this->assertTag(array( 'tag' => 'a', 'attributes' => array('class' => 'active'), @@ -364,11 +394,14 @@ public function testGenerateActive() { 'Test', 'test_route_1', array(), - array('language' => new Language(array('id' => 'de'))) + array( + 'language' => new Language(array('id' => 'de')), + 'set_active_class' => TRUE, + ) ); $this->assertNotTag(array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => array('class' => 'active', 'hreflang' => 'de'), ), $result); // Render a link with the same path and query parameter as the current path. @@ -380,8 +413,11 @@ public function testGenerateActive() { 'Test', 'test_route_3', array(), - array('query' => array('value' => 'example_1') - )); + array( + 'query' => array('value' => 'example_1'), + 'set_active_class' => TRUE, + ) + ); $this->assertTag(array( 'tag' => 'a', 'attributes' => array('class' => 'active'), @@ -393,12 +429,16 @@ public function testGenerateActive() { 'Test', 'test_route_3', array(), - array('query' => array('value' => 'example_2')) + array( + 'query' => array('value' => 'example_2'), + 'set_active_class' => TRUE, + ) ); $this->assertNotTag(array( 'tag' => 'a', 'attributes' => array('class' => 'active'), ), $result); + // Render a link with the same path and query parameter as the current path. $request = new Request(array('value' => 'example_1'), array(), array('system_path' => 'test-route-4/1', RouteObjectInterface::ROUTE_NAME => 'test_route_4')); $raw_variables = new ParameterBag(array('object' => '1')); @@ -408,7 +448,10 @@ public function testGenerateActive() { 'Test', 'test_route_4', array('object' => '1'), - array('query' => array('value' => 'example_1')) + array( + 'query' => array('value' => 'example_1'), + 'set_active_class' => TRUE, + ) ); $this->assertTag(array( 'tag' => 'a', @@ -416,6 +459,175 @@ public function testGenerateActive() { ), $result); } + /** + * Tests the active class on the link method for authenticated users. + * + * @see \Drupal\Core\Utility\LinkGenerator::generate() + * + * @todo Test that the active class is added on the front page when generating + * links to the front page when drupal_is_front_page() is converted to a + * service. + */ + public function testGenerateActiveAuthenticated() { + $this->currentUser->expects($this->exactly(7)) + ->method('isAuthenticated') + ->will($this->returnValue(TRUE)); + + $this->urlGenerator->expects($this->exactly(8)) + ->method('generateFromRoute') + ->will($this->returnValueMap(array( + array('test_route_1', array(), FALSE, '/test-route-1'), + array('test_route_3', array(), FALSE, '/test-route-3'), + array('test_route_4', array('object' => '1'), FALSE, '/test-route-4/1'), + ))); + + $this->urlGenerator->expects($this->exactly(7)) + ->method('getPathFromRoute') + ->will($this->returnValueMap(array( + array('test_route_1', array(), 'test-route-1'), + array('test_route_3', array(), 'test-route-3'), + array('test_route_4', array('object' => '1'), 'test-route-4/1'), + ))); + + $this->aliasManager->expects($this->exactly(7)) + ->method('getSystemPath') + ->will($this->returnValueMap(array( + array('test-route-1', NULL, 'test-route-1'), + array('test-route-3', NULL, 'test-route-3'), + array('test-route-4/1', NULL, 'test-route-4/1'), + ))); + + $this->moduleHandler->expects($this->exactly(8)) + ->method('alter'); + + $this->setUpLanguageManager(); + + // Render a link with a path different from the current path. + $request = new Request(array(), array(), array('system_path' => 'test-route-2')); + $this->linkGenerator->setRequest($request); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array('data-drupal-link-system-path' => 'test-route-1'), + ), $result); + + // Render a link with the same path as the current path. + $request = new Request(array(), array(), array('system_path' => 'test-route-1', RouteObjectInterface::ROUTE_NAME => 'test_route_1')); + // This attribute is expected to be set in a Drupal request by + // \Drupal\Core\ParamConverter\ParamConverterManager + $raw_variables = new ParameterBag(); + $request->attributes->set('_raw_variables', $raw_variables); + $this->linkGenerator->setRequest($request); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array('data-drupal-link-system-path' => 'test-route-1'), + ), $result); + + // Render a link with the same path as the current path, but with the + // set_active_class option disabled. + $request = new Request(array(), array(), array('system_path' => 'test-route-1', RouteObjectInterface::ROUTE_NAME => 'test_route_1')); + // This attribute is expected to be set in a Drupal request by + // \Drupal\Core\ParamConverter\ParamConverterManager + $raw_variables = new ParameterBag(); + $request->attributes->set('_raw_variables', $raw_variables); + $this->linkGenerator->setRequest($request); + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => FALSE)); + $this->assertNotTag(array( + 'tag' => 'a', + 'attributes' => array('data-drupal-link-system-path' => 'test-route-1'), + ), $result); + + // Render a link with the same path and language as the current path. + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array('data-drupal-link-system-path' => 'test-route-1'), + ), $result); + + // Render a link with the same path but a different language than the + // current path. + $result = $this->linkGenerator->generate( + 'Test', + 'test_route_1', + array(), + array( + 'language' => new Language(array('id' => 'de')), + 'set_active_class' => TRUE, + ) + ); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array( + 'data-drupal-link-system-path' => 'test-route-1', + 'hreflang' => 'de', + ), + ), $result); + + // Render a link with the same path and query parameter as the current path. + $request = new Request(array('value' => 'example_1'), array(), array('system_path' => 'test-route-3', RouteObjectInterface::ROUTE_NAME => 'test_route_3')); + $raw_variables = new ParameterBag(); + $request->attributes->set('_raw_variables', $raw_variables); + $this->linkGenerator->setRequest($request); + $result = $this->linkGenerator->generate( + 'Test', + 'test_route_3', + array(), + array( + 'query' => array('value' => 'example_1'), + 'set_active_class' => TRUE, + ) + ); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array( + 'data-drupal-link-system-path' => 'test-route-3', + 'data-drupal-link-query' => 'regexp:/.*value.*example_1.*/', + ), + ), $result); + + // Render a link with the same path but a different query parameter than the + // current path. + $result = $this->linkGenerator->generate( + 'Test', + 'test_route_3', + array(), + array( + 'query' => array('value' => 'example_2'), + 'set_active_class' => TRUE, + ) + ); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array( + 'data-drupal-link-system-path' => 'test-route-3', + 'data-drupal-link-query' => 'regexp:/.*value.*example_2.*/', + ), + ), $result); + + // Render a link with the same path and query parameter as the current path. + $request = new Request(array('value' => 'example_1'), array(), array('system_path' => 'test-route-4/1', RouteObjectInterface::ROUTE_NAME => 'test_route_4')); + $raw_variables = new ParameterBag(array('object' => '1')); + $request->attributes->set('_raw_variables', $raw_variables); + $this->linkGenerator->setRequest($request); + $result = $this->linkGenerator->generate( + 'Test', + 'test_route_4', + array('object' => '1'), + array( + 'query' => array('value' => 'example_1'), + 'set_active_class' => TRUE, + ) + ); + $this->assertTag(array( + 'tag' => 'a', + 'attributes' => array( + 'data-drupal-link-system-path' => 'test-route-4/1', + 'data-drupal-link-query' => 'regexp:/.*value.*example_1.*/', + ), + ), $result); + } + } }