core/core.services.yml | 8 +- core/includes/pager.inc | 16 ++- core/lib/Drupal/Core/Render/Element/Pager.php | 6 + core/lib/Drupal/Core/Render/UrlGenerator.php | 121 +++++++++++++++++++++ .../modules/comment/src/Tests/CommentPagerTest.php | 7 +- core/modules/comment/src/Tests/CommentRssTest.php | 1 + .../modules/node/src/Tests/Views/FrontPageTest.php | 2 + .../src/Tests/PageCacheTagsIntegrationTest.php | 5 +- core/modules/shortcut/shortcut.module | 2 +- core/modules/system/src/Tests/Pager/PagerTest.php | 9 +- core/modules/views/src/Tests/Plugin/PagerTest.php | 2 +- 11 files changed, 163 insertions(+), 16 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 0fe0a7f..de0aa21 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -682,9 +682,15 @@ services: arguments: ['@route_filter.lazy_collector'] tags: - { name: event_subscriber } - url_generator: + url_generator.uncacheable: class: Drupal\Core\Routing\UrlGenerator arguments: ['@router.route_provider', '@path_processor_manager', '@route_processor_manager', '@config.factory', '@request_stack'] + public: false + calls: + - [setContext, ['@?router.request_context']] + url_generator: + class: Drupal\Core\Render\UrlGenerator + arguments: ['@url_generator.uncacheable', '@renderer'] calls: - [setContext, ['@?router.request_context']] redirect.destination: diff --git a/core/includes/pager.inc b/core/includes/pager.inc index 7ce3407..a7990f2 100644 --- a/core/includes/pager.inc +++ b/core/includes/pager.inc @@ -218,7 +218,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, 0), ); - $items['first']['href'] = \Drupal::url('', [], $options); + $items['first']['href'] = ($options['query']) ? '?' . UrlHelper::buildQuery($options['query']) : ''; if (isset($tags[0])) { $items['first']['text'] = $tags[0]; } @@ -227,7 +227,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $pager_page_array[$element] - 1), ); - $items['previous']['href'] = \Drupal::url('', [], $options); + $items['previous']['href'] = ($options['query']) ? '?' . UrlHelper::buildQuery($options['query']) : ''; if (isset($tags[1])) { $items['previous']['text'] = $tags[1]; } @@ -243,7 +243,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $i - 1), ); - $items['pages'][$i]['href'] = \Drupal::url('', [], $options); + $items['pages'][$i]['href'] = ($options['query']) ? '?' . UrlHelper::buildQuery($options['query']) : ''; if ($i == $pager_current) { $variables['current'] = $i; } @@ -260,7 +260,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $pager_page_array[$element] + 1), ); - $items['next']['href'] = \Drupal::url('', [], $options); + $items['next']['href'] = ($options['query']) ? '?' . UrlHelper::buildQuery($options['query']) : ''; if (isset($tags[3])) { $items['next']['text'] = $tags[3]; } @@ -269,7 +269,7 @@ function template_preprocess_pager(&$variables) { $options = array( 'query' => pager_query_add_page($parameters, $element, $pager_max - 1), ); - $items['last']['href'] = \Drupal::url('', [], $options); + $items['last']['href'] = ($options['query']) ? '?' . UrlHelper::buildQuery($options['query']) : ''; if (isset($tags[4])) { $items['last']['text'] = $tags[4]; } @@ -311,6 +311,12 @@ function pager_query_add_page(array $query, $element, $index) { if ($current_request_query = pager_get_query_parameters()) { $query = array_merge($current_request_query, $query); } + + // This is is based on the entire current query string. We need to ensure + // cacheability is affected accordingly. + $build = ['#cache' => ['contexts' => ['url.query_args']]]; + drupal_render($build); + return $query; } diff --git a/core/lib/Drupal/Core/Render/Element/Pager.php b/core/lib/Drupal/Core/Render/Element/Pager.php index eeb0f9d..b7fe63c 100644 --- a/core/lib/Drupal/Core/Render/Element/Pager.php +++ b/core/lib/Drupal/Core/Render/Element/Pager.php @@ -40,6 +40,12 @@ public function getInfo() { /** * #pre_render callback to associate the appropriate cache context. * + * Note: the default pager theme process function template_preprocess_pager() + * also calls pager_query_add_page(), which maintains the existing query + * string. Therefore pager_query_add_page() adds the 'url.query_args' cache + * context, which causes the more specific cache context below to be optimized + * away. In other themes, however, that may not be the case. + * * @param array $pager * A renderable array of #type => pager. * diff --git a/core/lib/Drupal/Core/Render/UrlGenerator.php b/core/lib/Drupal/Core/Render/UrlGenerator.php new file mode 100644 index 0000000..f8fd708 --- /dev/null +++ b/core/lib/Drupal/Core/Render/UrlGenerator.php @@ -0,0 +1,121 @@ +urlGenerator = $url_generator; + $this->renderer = $renderer; + } + + /** + * {@inheritdoc} + */ + public function setContext(SymfonyRequestContext $context) { + $this->urlGenerator->setContext($context); + } + + /** + * {@inheritdoc} + */ + public function getContext() { + return $this->urlGenerator->getContext(); + } + + /** + * {@inheritdoc} + */ + public function getPathFromRoute($name, $parameters = array()) { + return $this->urlGenerator->getPathFromRoute($name, $parameters); + } + + /** + * Bubbles the cacheability metadata to the current render context. + * + * @param \Drupal\Core\GeneratedUrl $generated_url + * The generated URL whose bubbleable metadata to bubble. + */ + protected function bubble(GeneratedUrl $generated_url) { + $build = []; + $generated_url->applyTo($build); + $this->renderer->render($build); + } + + /** + * {@inheritdoc} + */ + public function generate($name, $parameters = array(), $absolute = FALSE) { + $options['absolute'] = $absolute; + $generated_url = $this->generateFromRoute($name, $parameters, $options, TRUE); + $this->bubble($generated_url); + return $generated_url->getGeneratedUrl(); + } + + /** + * {@inheritdoc} + */ + public function generateFromRoute($name, $parameters = array(), $options = array(), $collect_cacheability_metadata = FALSE) { + $generated_url = $this->urlGenerator->generateFromRoute($name, $parameters, $options, TRUE); + $this->bubble($generated_url); + return $collect_cacheability_metadata ? $generated_url : $generated_url->getGeneratedUrl(); + } + + /** + * {@inheritdoc} + */ + public function generateFromPath($path = NULL, $options = array(), $collect_cacheability_metadata = FALSE) { + $generated_url = $this->urlGenerator->generateFromPath($path, $options, TRUE); + $this->bubble($generated_url); + return $collect_cacheability_metadata ? $generated_url : $generated_url->getGeneratedUrl(); + } + + /** + * {@inheritDoc} + */ + public function supports($name) { + return $this->urlGenerator->supports($name); + } + + /** + * {@inheritDoc} + */ + public function getRouteDebugMessage($name, array $parameters = array()) { + return $this->urlGenerator->getRouteDebugMessage($name, $parameters); + } + +} diff --git a/core/modules/comment/src/Tests/CommentPagerTest.php b/core/modules/comment/src/Tests/CommentPagerTest.php index 92fc9ff..8995654 100644 --- a/core/modules/comment/src/Tests/CommentPagerTest.php +++ b/core/modules/comment/src/Tests/CommentPagerTest.php @@ -381,7 +381,12 @@ protected function clickLinkWithXPath($xpath, $arguments = array(), $index = 0) $url_before = $this->getUrl(); $urls = $this->xpath($xpath, $arguments); if (isset($urls[$index])) { - $url_target = $this->getAbsoluteUrl($urls[$index]['href']); + $path = $urls[$index]['href']; + // Support relative links: links that don't have a path component (and + // hence only a fragment or querystring), use the path component of the + // current page. + $path = (isset(parse_url($path)['path'])) ? $path : parse_url($this->getUrl())['path'] . $path; + $url_target = $this->getAbsoluteUrl($path); $this->pass(SafeMarkup::format('Clicked link %label (@url_target) from @url_before', array('%label' => $xpath, '@url_target' => $url_target, '@url_before' => $url_before)), 'Browser'); return $this->drupalGet($url_target); } diff --git a/core/modules/comment/src/Tests/CommentRssTest.php b/core/modules/comment/src/Tests/CommentRssTest.php index b82da48..7134a29 100644 --- a/core/modules/comment/src/Tests/CommentRssTest.php +++ b/core/modules/comment/src/Tests/CommentRssTest.php @@ -58,6 +58,7 @@ function testCommentRss() { $this->assertCacheContexts([ 'languages:language_interface', 'theme', + 'url.site', 'user.node_grants:view', 'user.permissions', 'timezone', diff --git a/core/modules/node/src/Tests/Views/FrontPageTest.php b/core/modules/node/src/Tests/Views/FrontPageTest.php index b3ab9c9..d4cca29 100644 --- a/core/modules/node/src/Tests/Views/FrontPageTest.php +++ b/core/modules/node/src/Tests/Views/FrontPageTest.php @@ -249,6 +249,8 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) { // Default cache contexts of the renderer. 'theme', 'url.query_args.pagers:0', + // Attached feed. + 'url.site', ]; // Test before there are any nodes. diff --git a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php index d8dec6a..868e360 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php @@ -71,10 +71,7 @@ function testPageCacheTags() { $cache_contexts = [ 'languages:' . LanguageInterface::TYPE_INTERFACE, - 'route.menu_active_trails:account', - 'route.menu_active_trails:footer', - 'route.menu_active_trails:main', - 'route.menu_active_trails:tools', + 'route', 'theme', 'timezone', 'user.permissions', diff --git a/core/modules/shortcut/shortcut.module b/core/modules/shortcut/shortcut.module index 975a309..be83839 100644 --- a/core/modules/shortcut/shortcut.module +++ b/core/modules/shortcut/shortcut.module @@ -313,7 +313,6 @@ function shortcut_preprocess_page(&$variables) { 'link' => $link, 'name' => $variables['title'], ); - $query += \Drupal::destination()->getAsArray(); $shortcut_set = shortcut_current_displayed_set(); @@ -341,6 +340,7 @@ function shortcut_preprocess_page(&$variables) { } if (theme_get_setting('third_party_settings.shortcut.module_link')) { + $query += \Drupal::destination()->getAsArray(); $variables['title_suffix']['add_or_remove_shortcut'] = array( '#attached' => array( 'library' => array( diff --git a/core/modules/system/src/Tests/Pager/PagerTest.php b/core/modules/system/src/Tests/Pager/PagerTest.php index 87730ff..bd78ec6 100644 --- a/core/modules/system/src/Tests/Pager/PagerTest.php +++ b/core/modules/system/src/Tests/Pager/PagerTest.php @@ -65,7 +65,7 @@ function testActiveClass() { $elements = $this->xpath('//li[contains(@class, :class)]/a', array(':class' => 'pager__item--last')); preg_match('@page=(\d+)@', $elements[0]['href'], $matches); $current_page = (int) $matches[1]; - $this->drupalGet($GLOBALS['base_root'] . $elements[0]['href'], array('external' => TRUE)); + $this->drupalGet($GLOBALS['base_root'] . parse_url($this->getUrl())['path'] . $elements[0]['href'], array('external' => TRUE)); $this->assertPagerItems($current_page); } @@ -77,18 +77,21 @@ protected function testPagerQueryParametersAndCacheContext() { $this->drupalGet('pager-test/query-parameters'); $this->assertText(t('Pager calls: 0'), 'Initial call to pager shows 0 calls.'); $this->assertText('pager.0.0'); + $this->assertCacheContext('url.query_args'); // Go to last page, the count of pager calls need to go to 1. $elements = $this->xpath('//li[contains(@class, :class)]/a', array(':class' => 'pager__item--last')); - $this->drupalGet($GLOBALS['base_root'] . $elements[0]['href'], array('external' => TRUE)); + $this->drupalGet($GLOBALS['base_root'] . parse_url($this->getUrl())['path'] . $elements[0]['href'], array('external' => TRUE)); $this->assertText(t('Pager calls: 1'), 'First link call to pager shows 1 calls.'); $this->assertText('pager.0.60'); + $this->assertCacheContext('url.query_args'); // Go back to first page, the count of pager calls need to go to 2. $elements = $this->xpath('//li[contains(@class, :class)]/a', array(':class' => 'pager__item--first')); - $this->drupalGet($GLOBALS['base_root'] . $elements[0]['href'], array('external' => TRUE)); + $this->drupalGet($GLOBALS['base_root'] . parse_url($this->getUrl())['path'] . $elements[0]['href'], array('external' => TRUE)); $this->assertText(t('Pager calls: 2'), 'Second link call to pager shows 2 calls.'); $this->assertText('pager.0.0'); + $this->assertCacheContext('url.query_args'); } /** diff --git a/core/modules/views/src/Tests/Plugin/PagerTest.php b/core/modules/views/src/Tests/Plugin/PagerTest.php index 4a75fdc..c1c6c97 100644 --- a/core/modules/views/src/Tests/Plugin/PagerTest.php +++ b/core/modules/views/src/Tests/Plugin/PagerTest.php @@ -261,7 +261,7 @@ public function testNormalPager() { // Test pager cache contexts. $this->drupalGet('test_pager_full'); - $this->assertCacheContexts(['languages:language_interface', 'theme', 'timezone', 'url.query_args.pagers:0', 'user.node_grants:view']); + $this->assertCacheContexts(['languages:language_interface', 'theme', 'timezone', 'url.query_args', 'user.node_grants:view']); } /**