core/core.services.yml | 3 +- core/includes/common.inc | 99 ++++++++--- core/includes/menu.inc | 4 + core/lib/Drupal/Core/Utility/LinkGenerator.php | 96 ++++++++-- .../Drupal/Core/Utility/LinkGeneratorInterface.php | 11 +- core/misc/active-link.js | 60 +++++++ core/misc/drupal.js | 2 +- .../Drupal/image/Tests/ImageFieldDisplayTest.php | 2 +- core/modules/language/language.negotiation.inc | 1 + .../language/Tests/LanguageSwitchingTest.php | 136 +++++++++++++- .../Tests/LanguageUILanguageNegotiationTest.php | 7 +- .../Controller/LanguageTestController.php | 3 + .../Drupal/system/Tests/Common/JavaScriptTest.php | 2 +- .../Drupal/system/Tests/Theme/FunctionsTest.php | 10 +- core/modules/system/system.module | 20 +++ .../Controller/CommonTestController.php | 5 + .../Tests/Core/Utility/LinkGeneratorTest.php | 185 ++++++++++++++++---- 17 files changed, 553 insertions(+), 93 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index c3e6fee..24db257 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -268,9 +268,10 @@ 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'] calls: - [setRequest, ['@?request']] + - [setCurrentUser, ['@?current_user']] router.dynamic: class: Symfony\Cmf\Component\Routing\DynamicRouter arguments: ['@router.request_context', '@router.matcher', '@url_generator'] diff --git a/core/includes/common.inc b/core/includes/common.inc index 12ccffd..3750b4a 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -1214,12 +1214,25 @@ function drupal_http_header_attributes(array $attributes = array()) { * 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). This element is also used by url(). + * - 'set_active_class' (default FALSE): Whether l() should compare the $path, + * 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 by l() with active class processing enabled is incompatible + * with render caching elements on more than a per-page basis. + * 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(). * - Additional $options elements used by the url() function. * * @return string * An HTML string containing a link to the given path. * * @see url() + * @see system_page_build() */ function l($text, $path, array $options = array()) { // Start building a structured representation of our link to be altered later. @@ -1235,6 +1248,7 @@ function l($text, $path, array $options = 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 @@ -1243,35 +1257,56 @@ function l($text, $path, array $options = array()) { $variables['options']['attributes']['hreflang'] = $variables['options']['language']->id; } - // Because l() is called very often we statically cache values that require an - // extra function call. - static $drupal_static_fast; - if (!isset($drupal_static_fast['active'])) { - $drupal_static_fast['active'] = &drupal_static(__FUNCTION__); - } - $active = &$drupal_static_fast['active']; - if (!isset($active)) { - $active = array( - 'path' => current_path(), - 'front_page' => drupal_is_front_page(), - 'language' => language(Language::TYPE_URL)->id, - 'query' => \Drupal::service('request')->query->all(), - ); - } + // 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-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); + } - // Determine whether this link is "active', meaning that it links to the - // current page. It is important that we stop checking "active" conditions if - // we know the link is not active. This helps ensure that l() remains fast. - // An active link's path is equal to the current path. - $variables['url_is_active'] = ($path == $active['path'] || ($path == '' && $active['front_page'])) - // The language of an active link is equal to the current language. - && (empty($variables['options']['language']) || $variables['options']['language']->id == $active['language']) - // The query parameters of an active link are equal to the current parameters. - && ($variables['options']['query'] == $active['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'])) { + $variables['options']['attributes']['data-drupal-link-system-path'] = \Drupal::service('path.alias_manager.cached')->getSystemPath($path); + } + } + else { + // Because l() is called very often we statically cache values that require + // an extra function call. + static $drupal_static_fast; + if (!isset($drupal_static_fast['active'])) { + $drupal_static_fast['active'] = &drupal_static(__FUNCTION__); + } + $active = &$drupal_static_fast['active']; + if (!isset($active)) { + $active = array( + 'path' => current_path(), + 'front_page' => drupal_is_front_page(), + 'language' => language(Language::TYPE_URL)->id, + 'query' => \Drupal::service('request')->query->all(), + ); + } - // Add the "active" class if appropriate. - if ($variables['url_is_active']) { - $variables['options']['attributes']['class'][] = 'active'; + // Determine whether this link is "active', meaning that it links to the + // current page. It is important that we stop checking "active" conditions + // if we know the link is not active. This helps ensure that l() remains + // fast. + // An active link's path is equal to the current path. + $variables['url_is_active'] = ($path == $active['path'] || ($path == '' && $active['front_page'])) + // The language of an active link is equal to the current language. + && (empty($variables['options']['language']) || $variables['options']['language']->id == $active['language']) + // The query parameters of an active link are equal to the current parameters. + && ($variables['options']['query'] == $active['query']); + + // 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() @@ -2146,6 +2181,7 @@ function drupal_add_js($data = NULL, $options = NULL) { // @todo Make this less hacky: http://drupal.org/node/1547376. $scriptPath = $GLOBALS['script_path']; $pathPrefix = ''; + $current_query = \Drupal::service('request')->query->all(); url('', array('script' => &$scriptPath, 'prefix' => &$pathPrefix)); $current_path = current_path(); $current_path_is_admin = FALSE; @@ -2153,13 +2189,20 @@ function drupal_add_js($data = NULL, $options = NULL) { if (!(defined('MAINTENANCE_MODE') && MAINTENANCE_MODE === 'update')) { $current_path_is_admin = path_is_admin($current_path); } - $javascript['settings']['data'][] = array( + $path = array( 'basePath' => base_path(), 'scriptPath' => $scriptPath, 'pathPrefix' => $pathPrefix, 'currentPath' => $current_path, 'currentPathIsAdmin' => $current_path_is_admin, + 'isFront' => drupal_is_front_page(), + 'currentLanguage' => \Drupal::languageManager()->getLanguage(Language::TYPE_URL)->id, ); + if (!empty($current_query)) { + ksort($current_query); + $path['currentQuery'] = (object) $current_query; + } + $javascript['settings']['data'][] = array('path' => $path); } // All JavaScript settings are placed in the header of the page with // the library weight so that inline scripts appear afterwards. diff --git a/core/includes/menu.inc b/core/includes/menu.inc index 7afc274..388169a 100644 --- a/core/includes/menu.inc +++ b/core/includes/menu.inc @@ -1705,6 +1705,7 @@ function theme_menu_link(array $variables) { if ($element['#below']) { $sub_menu = drupal_render($element['#below']); } + $element['#localized_options']['set_active_class'] = TRUE; $output = l($element['#title'], $element['#href'], $element['#localized_options']); return '' . $output . $sub_menu . "\n"; } @@ -1740,6 +1741,8 @@ function theme_menu_local_task($variables) { $link['localized_options']['html'] = TRUE; $link_text = t('!local-task-title!active', array('!local-task-title' => $link['title'], '!active' => $active)); } + $link['localized_options']['set_active_class'] = TRUE; + if (!empty($link['href'])) { // @todo - remove this once all pages are converted to routes. $a_tag = l($link_text, $link['href'], $link['localized_options']); @@ -1771,6 +1774,7 @@ function theme_menu_local_action($variables) { ); $link['localized_options']['attributes']['class'][] = 'button'; $link['localized_options']['attributes']['class'][] = 'button-action'; + $link['localized_options']['set_active_class'] = TRUE; $output = '
  • '; // @todo Remove this check and the call to l() when all pages are converted to diff --git a/core/lib/Drupal/Core/Utility/LinkGenerator.php b/core/lib/Drupal/Core/Utility/LinkGenerator.php index 3f63620..d5300bf 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. + * + * @var \Drupal\Core\Session\AccountInterface + */ + protected $currentUser; + + /** * Constructs a LinkGenerator instance. * * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator @@ -58,11 +75,14 @@ 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. */ - 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) { $this->urlGenerator = $url_generator; $this->moduleHandler = $module_handler; $this->languageManager = $language_manager; + $this->aliasManager = $alias_manager; } /** @@ -85,6 +105,16 @@ public function setRequest(Request $request) { } /** + * Sets the current user. + * + * @param \Drupal\Core\Session\AccountInterface $current_user + * The current user service. + */ + public function setCurrentUser(AccountInterface $current_user) { + $this->currentUser = $current_user; + } + + /** * {@inheritdoc} */ public function getActive() { @@ -93,6 +123,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 +149,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/misc/active-link.js b/core/misc/active-link.js new file mode 100644 index 0000000..a3724e1 --- /dev/null +++ b/core/misc/active-link.js @@ -0,0 +1,60 @@ +/** + * @file + * Attaches behaviors for Drupal's active link marking. + */ + +(function (Drupal, drupalSettings) { + +"use strict"; + +/** + * Append active class. + * + * 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, exposed View filters). + */ +Drupal.behaviors.l = { + attach: function queryL (context) { + // Start by finding all potentially active links. + var path = drupalSettings.path; + var queryString = JSON.stringify(path.currentQuery); + var querySelector = path.currentQuery ? "[data-drupal-link-query='" + queryString + "']" : ':not([data-drupal-link-query])'; + var originalSelectors = ['[data-drupal-link-system-path="' + path.currentPath + '"]']; + var selectors; + + // If this is the front page, we have to check for the path as well. + if (path.isFront) { + originalSelectors.push('[data-drupal-link-system-path=""]'); + } + + // Add language filtering. + selectors = [].concat( + // Links without any hreflang attributes (most of them). + originalSelectors.map(function (selector) { return selector + ':not([hreflang])';}), + // Links with hreflang equals to the current language. + originalSelectors.map(function (selector) { return selector + '[hreflang="' + path.currentLanguage + '"]';}) + ); + + // Add query string selector for pagers, exposed filters. + selectors = selectors.map(function (current) { return current + querySelector; }); + + // Query the DOM. + var activeLinks = context.querySelectorAll(selectors.join(',')); + for (var i = 0, il = activeLinks.length; i < il; i += 1) { + activeLinks[i].classList.add('active'); + } + }, + detach: function (context, settings, trigger) { + if (trigger === 'unload') { + var activeLinks = context.querySelectorAll('a.active'); + for (var i = 0, il = activeLinks.length; i < il; i += 1) { + activeLinks[i].classList.remove('active'); + } + } + } +}; + +})(Drupal, drupalSettings); diff --git a/core/misc/drupal.js b/core/misc/drupal.js index 6c04130..e994ec6 100644 --- a/core/misc/drupal.js +++ b/core/misc/drupal.js @@ -267,7 +267,7 @@ Drupal.t = function (str, args, options) { * Returns the URL to a Drupal page. */ Drupal.url = function (path) { - return drupalSettings.basePath + drupalSettings.scriptPath + path; + return drupalSettings.path.basePath + drupalSettings.path.scriptPath + path; }; /** diff --git a/core/modules/image/lib/Drupal/image/Tests/ImageFieldDisplayTest.php b/core/modules/image/lib/Drupal/image/Tests/ImageFieldDisplayTest.php index 9ef2167..9844c34 100644 --- a/core/modules/image/lib/Drupal/image/Tests/ImageFieldDisplayTest.php +++ b/core/modules/image/lib/Drupal/image/Tests/ImageFieldDisplayTest.php @@ -111,7 +111,7 @@ function _testImageFieldFormatters($scheme) { '#width' => 40, '#height' => 20, ); - $default_output = l($image, 'node/' . $nid, array('html' => TRUE, 'attributes' => array('class' => 'active'))); + $default_output = l($image, 'node/' . $nid, array('html' => TRUE)); $this->drupalGet('node/' . $nid); $this->assertRaw($default_output, 'Image linked to content formatter displaying correctly on full node view.'); diff --git a/core/modules/language/language.negotiation.inc b/core/modules/language/language.negotiation.inc index f87284d..f59a94e 100644 --- a/core/modules/language/language.negotiation.inc +++ b/core/modules/language/language.negotiation.inc @@ -399,6 +399,7 @@ function language_switcher_url($type, $path) { 'title' => $language->name, 'language' => $language, 'attributes' => array('class' => array('language-link')), + 'set_active_class' => TRUE, ); } diff --git a/core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php b/core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php index 4613f43..e1ebfc0 100644 --- a/core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php +++ b/core/modules/language/lib/Drupal/language/Tests/LanguageSwitchingTest.php @@ -55,9 +55,70 @@ function testLanguageBlock() { $edit = array('language_interface[enabled][language-url]' => '1'); $this->drupalPostForm('admin/config/regional/language/detection', $edit, t('Save settings')); + $this->doTestLanguageBlockAuthenticated($block->label()); + $this->doTestLanguageBlockAnonymous($block->label()); + } + + /** + * For authenticated users, the "active" class is set by JavaScript. + * + * @param string $block_label + * The label of the language switching block. + * + * @see testLanguageBlock() + */ + protected function doTestLanguageBlockAuthenticated($block_label) { // Assert that the language switching block is displayed on the frontpage. $this->drupalGet(''); - $this->assertText($block->label(), 'Language switcher block found.'); + $this->assertText($block_label, 'Language switcher block found.'); + + // Assert that only the current language is marked as active. + list($language_switcher) = $this->xpath('//div[@id=:id]/div[contains(@class, "content")]', array(':id' => 'block-test-language-block')); + $links = array( + 'active' => array(), + 'inactive' => array(), + ); + $anchors = array(); + foreach ($language_switcher->ul->li as $link) { + $classes = explode(" ", (string) $link['class']); + list($langcode) = array_intersect($classes, array('en', 'fr')); + if (in_array('active', $classes)) { + $links['active'][] = $langcode; + } + else { + $links['inactive'][] = $langcode; + } + $anchors[] = array( + 'hreflang' => (string) $link->a['hreflang'], + 'data-drupal-link-system-path' => (string) $link->a['data-drupal-link-system-path'], + ); + } + $this->assertIdentical($links, array('active' => array('en'), 'inactive' => array('fr')), 'Only the current language list item is marked as active on the language switcher block.'); + $expected_anchors = array( + 0 => array('hreflang' => 'en', 'data-drupal-link-system-path' => 'user/2'), + 1 => array('hreflang' => 'fr', 'data-drupal-link-system-path' => 'user/2'), + ); + $this->assertIdentical($anchors, $expected_anchors, 'The anchors have the correct attributes that will allow the drupal.active-link library to mark them as active.'); + $settings = $this->drupalGetSettings(); + $this->assertIdentical($settings['path']['currentPath'], 'user/2', 'drupalSettings.path.currentPath is set correctly to allow drupal.active-link to mark the correct links as active.'); + $this->assertIdentical($settings['path']['isFront'], FALSE, 'drupalSettings.path.isFront is set correctly to allow drupal.active-link to mark the correct links as active.'); + $this->assertIdentical($settings['path']['currentLanguage'], 'en', 'drupalSettings.path.currentLanguage is set correctly to allow drupal.active-link to mark the correct links as active.'); + } + + /** + * For anonymous users, the "active" class is set by PHP. + * + * @param string $block_label + * The label of the language switching block. + * + * @see testLanguageBlock() + */ + protected function doTestLanguageBlockAnonymous($block_label) { + $this->drupalLogout(); + + // Assert that the language switching block is displayed on the frontpage. + $this->drupalGet(''); + $this->assertText($block_label, 'Language switcher block found.'); // Assert that only the current language is marked as active. list($language_switcher) = $this->xpath('//div[@id=:id]/div[contains(@class, "content")]', array(':id' => 'block-test-language-block')); @@ -104,7 +165,80 @@ function testLanguageLinkActiveClass() { $edit = array('language_interface[enabled][language-url]' => '1'); $this->drupalPostForm('admin/config/regional/language/detection', $edit, t('Save settings')); + $this->doTestLanguageLinkActiveClassAuthenticated(); + $this->doTestLanguageLinkActiveClassAnonymous(); + } + + /** + * For authenticated users, the "active" class is set by JavaScript. + * + * @see testLanguageLinkActiveClass() + */ + protected function doTestLanguageLinkActiveClassAuthenticated() { + $function_name = '#type link'; + $path = 'language_test/type-link-active-class'; + + // Test links generated by l() on an English page. + $current_language = 'English'; + $this->drupalGet($path); + + // Language code 'none' link should be active. + $langcode = 'none'; + $links = $this->xpath('//a[@id = :id and @data-drupal-link-system-path = :path]', array(':id' => 'no_lang_link', ':path' => $path)); + $this->assertTrue(isset($links[0]), t('A link generated by :function to the current :language page with langcode :langcode has the correct attributes that will allow the drupal.active-link library to mark it as active.', array(':function' => $function_name, ':language' => $current_language, ':langcode' => $langcode))); + + // Language code 'en' link should be active. + $langcode = 'en'; + $links = $this->xpath('//a[@id = :id and @hreflang = :lang and @data-drupal-link-system-path = :path]', array(':id' => 'en_link', ':lang' => 'en', ':path' => $path)); + $this->assertTrue(isset($links[0]), t('A link generated by :function to the current :language page with langcode :langcode has the correct attributes that will allow the drupal.active-link library to mark it as active.', array(':function' => $function_name, ':language' => $current_language, ':langcode' => $langcode))); + + // Language code 'fr' link should not be active. + $langcode = 'fr'; + $links = $this->xpath('//a[@id = :id and @hreflang = :lang and @data-drupal-link-system-path = :path]', array(':id' => 'fr_link', ':lang' => 'fr', ':path' => $path)); + $this->assertTrue(isset($links[0]), t('A link generated by :function to the current :language page with langcode :langcode has the correct attributes that will allow the drupal.active-link library to NOT mark it as active.', array(':function' => $function_name, ':language' => $current_language, ':langcode' => $langcode))); + + // Verify that drupalSettings contains the correct values. + $settings = $this->drupalGetSettings(); + $this->assertIdentical($settings['path']['currentPath'], $path, 'drupalSettings.path.currentPath is set correctly to allow drupal.active-link to mark the correct links as active.'); + $this->assertIdentical($settings['path']['isFront'], FALSE, 'drupalSettings.path.isFront is set correctly to allow drupal.active-link to mark the correct links as active.'); + $this->assertIdentical($settings['path']['currentLanguage'], 'en', 'drupalSettings.path.currentLanguage is set correctly to allow drupal.active-link to mark the correct links as active.'); + + // Test links generated by l() on a French page. + $current_language = 'French'; + $this->drupalGet('fr/language_test/type-link-active-class'); + + // Language code 'none' link should be active. + $langcode = 'none'; + $links = $this->xpath('//a[@id = :id and @data-drupal-link-system-path = :path]', array(':id' => 'no_lang_link', ':path' => $path)); + $this->assertTrue(isset($links[0]), t('A link generated by :function to the current :language page with langcode :langcode has the correct attributes that will allow the drupal.active-link library to mark it as active.', array(':function' => $function_name, ':language' => $current_language, ':langcode' => $langcode))); + + // Language code 'en' link should not be active. + $langcode = 'en'; + $links = $this->xpath('//a[@id = :id and @hreflang = :lang and @data-drupal-link-system-path = :path]', array(':id' => 'en_link', ':lang' => 'en', ':path' => $path)); + $this->assertTrue(isset($links[0]), t('A link generated by :function to the current :language page with langcode :langcode has the correct attributes that will allow the drupal.active-link library to NOT mark it as active.', array(':function' => $function_name, ':language' => $current_language, ':langcode' => $langcode))); + + // Language code 'fr' link should be active. + $langcode = 'fr'; + $links = $this->xpath('//a[@id = :id and @hreflang = :lang and @data-drupal-link-system-path = :path]', array(':id' => 'fr_link', ':lang' => 'fr', ':path' => $path)); + $this->assertTrue(isset($links[0]), t('A link generated by :function to the current :language page with langcode :langcode has the correct attributes that will allow the drupal.active-link library to mark it as active.', array(':function' => $function_name, ':language' => $current_language, ':langcode' => $langcode))); + + // Verify that drupalSettings contains the correct values. + $settings = $this->drupalGetSettings(); + $this->assertIdentical($settings['path']['currentPath'], $path, 'drupalSettings.path.currentPath is set correctly to allow drupal.active-link to mark the correct links as active.'); + $this->assertIdentical($settings['path']['isFront'], FALSE, 'drupalSettings.path.isFront is set correctly to allow drupal.active-link to mark the correct links as active.'); + $this->assertIdentical($settings['path']['currentLanguage'], 'fr', 'drupalSettings.path.currentLanguage is set correctly to allow drupal.active-link to mark the correct links as active.'); + } + + /** + * For anonymous users, the "active" class is set by PHP. + * + * @see testLanguageLinkActiveClass() + */ + protected function doTestLanguageLinkActiveClassAnonymous() { $function_name = '#type link'; + $path = 'language_test/type-link-active-class'; + + $this->drupalLogout(); // Test links generated by l() on an English page. $current_language = 'English'; diff --git a/core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php b/core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php index 7c037b5..19744c2 100644 --- a/core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php +++ b/core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php @@ -411,6 +411,11 @@ function testUrlLanguageFallback() { // Enable the language switcher block. $this->drupalPlaceBlock('language_block:' . Language::TYPE_INTERFACE, array('id' => 'test_language_block')); + // Log out, because for anonymous users, the "active" class is set by PHP + // (which means we can easily test it here), whereas for authenticated users + // it is set by JavaScript. + $this->drupalLogout(); + // Access the front page without specifying any valid URL language prefix // and having as browser language preference a non-default language. $http_header = array("Accept-Language: $langcode_browser_fallback;q=1"); @@ -464,7 +469,7 @@ function testLanguageDomain() { $italian_url = url('admin', array('language' => $languages['it'], 'script' => '')); $url_scheme = $this->request->isSecure() ? 'https://' : 'http://'; $correct_link = $url_scheme . $link; - $this->assertTrue($italian_url == $correct_link, format_string('The url() function returns the right URL (@url) in accordance with the chosen language', array('@url' => $italian_url))); + $this->assertEqual($italian_url, $correct_link, format_string('The url() function returns the right URL (@url) in accordance with the chosen language', array('@url' => $italian_url))); // Test HTTPS via options. $this->settingsSet('mixed_mode_sessions', TRUE); diff --git a/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.php b/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.php index 001fe39..781af06 100644 --- a/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.php +++ b/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.php @@ -58,6 +58,7 @@ public function typeLinkActiveClass() { 'attributes' => array( 'id' => 'no_lang_link', ), + 'set_active_class' => TRUE, ), ), 'fr' => array( @@ -69,6 +70,7 @@ public function typeLinkActiveClass() { 'attributes' => array( 'id' => 'fr_link', ), + 'set_active_class' => TRUE, ), ), 'en' => array( @@ -80,6 +82,7 @@ public function typeLinkActiveClass() { 'attributes' => array( 'id' => 'en_link', ), + 'set_active_class' => TRUE, ), ), ); diff --git a/core/modules/system/lib/Drupal/system/Tests/Common/JavaScriptTest.php b/core/modules/system/lib/Drupal/system/Tests/Common/JavaScriptTest.php index 3a11ccc..52d5c1c 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Common/JavaScriptTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Common/JavaScriptTest.php @@ -80,7 +80,7 @@ function testAddSetting() { drupal_add_library('system', 'drupalSettings'); $javascript = drupal_add_js(); $last_settings = reset($javascript['settings']['data']); - $this->assertTrue(array_key_exists('currentPath', $last_settings), 'The current path JavaScript setting is set correctly.'); + $this->assertTrue(array_key_exists('currentPath', $last_settings['path']), 'The current path JavaScript setting is set correctly.'); $javascript = drupal_add_js(array('drupal' => 'rocks', 'dries' => 280342800), 'setting'); $last_settings = end($javascript['settings']['data']); diff --git a/core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php b/core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php index d1930c9..24d0357 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php @@ -159,12 +159,6 @@ function testLinks() { $expected = ''; $this->assertThemeOutput('links', $variables, $expected, 'Empty %callback with heading generates no output.'); - // Set the current path to the front page path. - // Required to verify the "active" class in expected links below, and - // because the current path is different when running tests manually via - // simpletest.module ('batch') and via the testing framework (''). - _current_path(\Drupal::config('system.site')->get('page.front')); - // Verify that a list of links is properly rendered. $variables = array(); $variables['attributes'] = array('id' => 'somelinks'); @@ -191,7 +185,7 @@ function testLinks() { $expected_links .= ''; @@ -224,7 +218,7 @@ function testLinks() { $expected_links .= ''; $expected = $expected_heading . $expected_links; diff --git a/core/modules/system/system.module b/core/modules/system/system.module index 17d7fb0..434d1b8 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -910,6 +910,19 @@ function system_library_info() { ), ); + // Drupal's active link marking. + $libraries['drupal.active-link'] = array( + 'title' => 'Drupal active link marking', + 'version' => \Drupal::VERSION, + 'js' => array( + 'core/misc/active-link.js' => array(), + ), + 'dependencies' => array( + array('system', 'drupal'), + array('system', 'drupalSettings'), + ), + ); + // Drupal's Ajax framework. $libraries['drupal.ajax'] = array( 'title' => 'Drupal AJAX', @@ -2131,6 +2144,13 @@ function system_page_build(&$page) { if (path_is_admin(current_path())) { $page['#attached']['css'][$path . '/css/system.admin.css'] = array('weight' => CSS_COMPONENT - 10); } + + // 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/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php index 7b7afc4..b04dfdb 100644 --- a/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestController.php @@ -35,6 +35,9 @@ public function typeLinkActiveClass() { '#type' => 'link', '#title' => t('Link with no query string'), '#href' => current_path(), + '#options' => array( + 'set_active_class' => TRUE, + ), ), 'with_query' => array( '#type' => 'link', @@ -45,6 +48,7 @@ public function typeLinkActiveClass() { 'foo' => 'bar', 'one' => 'two', ), + 'set_active_class' => TRUE, ), ), 'with_query_reversed' => array( @@ -56,6 +60,7 @@ public function typeLinkActiveClass() { 'one' => 'two', 'foo' => 'bar', ), + 'set_active_class' => TRUE, ), ), ); diff --git a/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php b/core/tests/Drupal/Tests/Core/Utility/LinkGeneratorTest.php index 6b7334f..1301fe6 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,39 +319,119 @@ public function testGenerateWithHtml() { } /** + * Provides test data for testing the active class on the link method. + * + * @see \Drupal\Tests\Core\Utility\LinkGeneratorTest::testGenerateActive() + * + * @return array + * Returns some test data. + */ + public function providerTestGenerateActive() { + return array( + // Anonymous: generate "active" class. + array(FALSE, array( + // Render a link with a path different from the current path. + 0 => array('method' => 'assertNotTag', 'attributes' => array('class' => 'active')), + // Render a link with the same path as the current path. + 1 => array('method' => 'assertTag', 'attributes' => array('class' => 'active')), + // Render a link with the same path as the current path, but with the + // set_active_class option disabled. + 2 => array('method' => 'assertNotTag', 'attributes' => array('class' => 'active')), + // Render a link with the same path and language as the current path. + 3 => array('method' => 'assertTag', 'attributes' => array('class' => 'active')), + // Render a link with the same path but a different language than the current + // path. + 4 => array('method' => 'assertNotTag', 'attributes' => array('class' => 'active', 'hreflang' => 'de')), + // Render a link with the same path and query parameter as the current path. + 5 => array('method' => 'assertTag', 'attributes' => array('class' => 'active')), + // Render a link with the same path but a different query parameter than the + // current path. + 6 => array('method' => 'assertNotTag', 'attributes' => array('class' => 'active')), + // Render a link with the same path and query parameter as the current path. + 7 => array('method' => 'assertTag', 'attributes' => array('class' => 'active')), + )), + // Authenticated: generate data attributes for drupal.active-link library. + array(TRUE, array( + // Render a link with a path different from the current path. + 0 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-1')), + // Render a link with the same path as the current path. + 1 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-1')), + // Render a link with the same path as the current path, but with the + // set_active_class option disabled. + 2 => array('method' => 'assertNotTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-1')), + // Render a link with the same path and language as the current path. + 3 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-1')), + // Render a link with the same path but a different language than the current + // path. + 4 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-1', 'hreflang' => 'de')), + // Render a link with the same path and query parameter as the current path. + 5 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-3', 'data-drupal-link-query' => 'regexp:/.*value.*example_1.*/')), + // Render a link with the same path but a different query parameter than the + // current path. + 6 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-3', 'data-drupal-link-query' => 'regexp:/.*value.*example_2.*/')), + // Render a link with the same path and query parameter as the current path. + 7 => array('method' => 'assertTag', 'attributes' => array('data-drupal-link-system-path' => 'test-route-4/1', 'data-drupal-link-query' => 'regexp:/.*value.*example_1.*/')), + )), + ); + } + + /** * Tests the active class on the link method. * * @see \Drupal\Core\Utility\LinkGenerator::generate() + * @see \Drupal\Tests\Core\Utility\LinkGeneratorTest::providerTestGenerateActive() + * + * @dataProvider providerTestGenerateActive * * @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 testGenerateActive() { - $this->urlGenerator->expects($this->exactly(7)) + public function testGenerateActive($is_authenticated, $expected_results) { + $this->currentUser->expects($this->exactly(7)) + ->method('isAuthenticated') + ->will($this->returnValue($is_authenticated)); + + $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)) + if ($is_authenticated) { + $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(); + $this->linkGenerator->setCurrentUser($this->currentUser); + // 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'); - $this->assertNotTag(array( + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); + $this->$expected_results[0]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[0]['attributes'], ), $result); // Render a link with the same path as the current path. @@ -345,17 +441,31 @@ 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'); - $this->assertTag(array( + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); + $this->$expected_results[1]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[1]['attributes'], + ), $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->$expected_results[2]['method'](array( + 'tag' => 'a', + 'attributes' => $expected_results[2]['attributes'], ), $result); // Render a link with the same path and language as the current path. - $result = $this->linkGenerator->generate('Test', 'test_route_1'); - $this->assertTag(array( + $result = $this->linkGenerator->generate('Test', 'test_route_1', array(), array('set_active_class' => TRUE)); + $this->$expected_results[3]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[3]['attributes'], ), $result); // Render a link with the same path but a different language than the current @@ -364,11 +474,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( + $this->$expected_results[4]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[4]['attributes'], ), $result); // Render a link with the same path and query parameter as the current path. @@ -380,11 +493,14 @@ public function testGenerateActive() { 'Test', 'test_route_3', array(), - array('query' => array('value' => 'example_1') - )); - $this->assertTag(array( + array( + 'query' => array('value' => 'example_1'), + 'set_active_class' => TRUE, + ) + ); + $this->$expected_results[5]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[5]['attributes'], ), $result); // Render a link with the same path but a different query parameter than the @@ -393,12 +509,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( + $this->$expected_results[6]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[6]['attributes'], ), $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,11 +528,14 @@ 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( + $this->$expected_results[7]['method'](array( 'tag' => 'a', - 'attributes' => array('class' => 'active'), + 'attributes' => $expected_results[7]['attributes'], ), $result); }