Issue #2433599 by Wim Leers: Ensure every (non-views) pager automatically...

Issue #2433599 by Wim Leers: Ensure every (non-views) pager automatically associates a matching cache context

core/includes/pager.inc

@@ -73,7 +73,7 @@ function pager_find_page($element = 0) {
  *   );
  *
  *   // Finally, display the pager controls, and return.
- *   $pager = array('#theme' => 'pager');
+ *   $pager = array('#type' => 'pager');
  *   $output .= drupal_render($pager);
  *   return $output;
  * @endcode
@@ -105,7 +105,7 @@ function pager_find_page($element = 0) {
  *   $output = drupal_render($search_results);
  *
  *   // Finally, display the pager controls, and return.
- *   $pager = array('#theme' => 'pager');
+ *   $pager = array('#type' => 'pager');
  *   $output .= drupal_render($pager);
  *   return $output;
  * @endcode
@@ -158,23 +158,24 @@ function pager_get_query_parameters() {
  *
  * Default template: pager.html.twig.
  *
- * Menu callbacks that display paged query results should call _theme('pager')
+ * Menu callbacks that display paged query results should use #type => pager
  * to retrieve a pager control so that users can view other results. Format a
  * list of nearby pages with additional query results.
  *
  * @param array $variables
  *   An associative array containing:
- *   - tags: An array of labels for the controls in the pager.
- *   - element: An optional integer to distinguish between multiple pagers on
- *     one page.
- *   - parameters: An associative array of query string parameters to append to
- *     the pager links.
- *   - quantity: The number of pages in the list.
+ *   - pager: A render element containing:
+ *     - #tags: An array of labels for the controls in the pager.
+ *     - #element: An optional integer to distinguish between multiple pagers on
+ *       one page.
+ *     - #parameters: An associative array of query string parameters to append to
+ *       the pager links.
+ *     - #quantity: The number of pages in the list.
  */
 function template_preprocess_pager(&$variables) {
-  $element = $variables['element'];
-  $parameters = $variables['parameters'];
-  $quantity = $variables['quantity'];
+  $element = $variables['pager']['#element'];
+  $parameters = $variables['pager']['#parameters'];
+  $quantity = $variables['pager']['#quantity'];
   global $pager_page_array, $pager_total;
 
   // Nothing to do if there is only one page.
@@ -182,7 +183,7 @@ function template_preprocess_pager(&$variables) {
     return;
   }
 
-  $tags = $variables['tags'];
+  $tags = $variables['pager']['#tags'];
 
   // Calculate various markers within this pager piece:
   // Middle is used to "center" pages around the current page.

core/includes/theme.inc

@@ -1835,7 +1835,7 @@ function drupal_common_theme() {
     ),
     // From pager.inc.
     'pager' => array(
-      'variables' => array('tags' => array(), 'element' => 0, 'parameters' => array(), 'quantity' => 9),
+      'render element' => 'pager',
     ),
     // From menu.inc.
     'menu' => array(

core/lib/Drupal/Core/Entity/EntityListBuilder.php

@@ -225,7 +225,7 @@ public function render() {
       }
     }
     $build['pager'] = array(
-      '#theme' => 'pager',
+      '#type' => 'pager',
     );
     return $build;
   }

core/lib/Drupal/Core/Render/Element/Pager.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Render\Element\Pager.
+ */
+
+namespace Drupal\Core\Render\Element;
+
+use Drupal\Core\Render\Element;
+
+/**
+ * Provides a render element for a pager.
+ *
+ * @RenderElement("pager")
+ */
+class Pager extends RenderElement{
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getInfo() {
+    return [
+      '#pre_render' => [
+        get_class($this) . '::preRenderPager',
+      ],
+      '#theme' => 'pager',
+      // The pager ID, to distinguish between multiple pagers on the same page.
+      '#element' => 0,
+      // An associative array of query string parameters to append to the pager
+      // links.
+      '#parameters' => [],
+      // The number of pages in the list.
+      '#quantity' => 9,
+      // An array of labels for the controls in the pager.
+      '#tags' => [],
+    ];
+  }
+
+  /**
+   * #pre_render callback to associate the appropriate cache context.
+   *
+   * @param array $pager
+   *   A renderable array of #type => pager.
+   *
+   * @return array
+   */
+  public static function preRenderPager(array $pager) {
+    $pager['#cache']['contexts'][] = 'pager:' . $pager['#element'];
+    return $pager;
+  }
+
+}

core/modules/aggregator/src/Controller/AggregatorController.php

@@ -82,7 +82,7 @@ protected function buildPageList(array $items, $feed_source = '') {
     if ($items) {
       $build['items'] = $this->entityManager()->getViewBuilder('aggregator_item')
         ->viewMultiple($items, 'default');
-      $build['pager'] = array('#theme' => 'pager');
+      $build['pager'] = array('#type' => 'pager');
     }
     return $build;
   }

core/modules/aggregator/src/FeedViewBuilder.php

@@ -73,7 +73,7 @@ public function buildComponents(array &$build, array $entities, array $displays,
 
         if ($view_mode == 'full') {
           // Also add the pager.
-          $build[$id]['pager'] = array('#theme' => 'pager');
+          $build[$id]['pager'] = array('#type' => 'pager');
         }
       }
 

core/modules/comment/comment.module

@@ -195,26 +195,6 @@ function comment_field_config_delete(FieldConfigInterface $field) {
   }
 }
 
-/**
- * Implements hook_entity_build_defaults_alter().
- *
- * @todo Remove this hook implementation in https://www.drupal.org/node/2433599
- */
-function comment_entity_build_defaults_alter(array &$build, EntityInterface $entity, $view_mode = 'full', $langcode = NULL) {
-  // Get the corresponding display settings.
-  $display = EntityViewDisplay::collectRenderDisplay($entity, $view_mode);
-  // Add the comment page number to the cache key if render caching is enabled.
-  if (isset($build['#cache']) && isset($build['#cache']['keys'])) {
-    foreach ($entity->getFieldDefinitions() as $field_name => $definition) {
-      if ($definition->getType() === 'comment' && ($display_options = $display->getComponent($field_name))) {
-        $pager_id = $display_options['settings']['pager_id'];
-        $build['#cache']['contexts'][] = 'pager:' . $pager_id;
-      }
-    }
-  }
-  return $build;
-}
-
 /**
  * Implements hook_node_links_alter().
  */

core/modules/comment/src/Form/CommentAdminOverview.php

@@ -240,7 +240,7 @@ public function buildForm(array $form, FormStateInterface $form_state, $type = '
       '#empty' => $this->t('No comments available.'),
     );
 
-    $form['pager'] = array('#theme' => 'pager');
+    $form['pager'] = array('#type' => 'pager');
 
     return $form;
   }

core/modules/comment/src/Plugin/Field/FieldFormatter/CommentDefaultFormatter.php

@@ -170,7 +170,7 @@ public function viewElements(FieldItemListInterface $items) {
           if ($comments) {
             comment_prepare_thread($comments);
             $build = $this->viewBuilder->viewMultiple($comments);
-            $build['pager']['#theme'] = 'pager';
+            $build['pager']['#type'] = 'pager';
             if ($this->getSetting('pager_id')) {
               $build['pager']['#element'] = $this->getSetting('pager_id');
             }

core/modules/dblog/src/Controller/DbLogController.php

@@ -216,7 +216,7 @@ public function overview() {
         'library' => array('dblog/drupal.dblog'),
       ),
     );
-    $build['dblog_pager'] = array('#theme' => 'pager');
+    $build['dblog_pager'] = array('#type' => 'pager');
 
     return $build;
 
@@ -411,7 +411,7 @@ public function topLogMessages($type) {
         'library' => array('dblog/drupal.dblog'),
       ),
     );
-    $build['dblog_top_pager'] = array('#theme' => 'pager');
+    $build['dblog_top_pager'] = array('#type' => 'pager');
 
     return $build;
   }

core/modules/forum/forum.module

@@ -515,7 +515,7 @@ function template_preprocess_forums(&$variables) {
 
       $variables['topics'] = $table;
       $variables['topics_pager'] = array(
-        '#theme' => 'pager',
+        '#type' => 'pager',
       );
     }
   }
@@ -567,7 +567,7 @@ function template_preprocess_forum_list(&$variables) {
   }
 
   $variables['pager'] = array(
-   '#theme' => 'pager',
+   '#type' => 'pager',
   );
 
   // Give meaning to $tid for themers. $tid actually stands for term ID.

core/modules/locale/src/Form/TranslateEditForm.php

@@ -161,7 +161,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
         );
       }
     }
-    $form['pager']['#theme'] = 'pager';
+    $form['pager']['#type'] = 'pager';
     return $form;
   }
 

core/modules/path/src/Controller/PathController.php

@@ -130,7 +130,7 @@ public function adminOverview(Request $request) {
       '#rows' => $rows,
       '#empty' => $this->t('No URL aliases available. <a href="@link">Add URL alias</a>.', array('@link' => $this->url('path.admin_add'))),
     );
-    $build['path_pager'] = array('#theme' => 'pager');
+    $build['path_pager'] = array('#type' => 'pager');
 
     return $build;
   }

core/modules/search/src/Controller/SearchController.php

@@ -128,7 +128,7 @@ public function view(Request $request, SearchPageInterface $entity) {
     );
 
     $build['pager'] = array(
-      '#theme' => 'pager',
+      '#type' => 'pager',
     );
 
     $build['#attached']['library'][] = 'search/drupal.search.results';

core/modules/search/tests/modules/search_extra_type/src/Plugin/Search/SearchExtraTypeSearch.php

@@ -84,7 +84,7 @@ public function buildResults() {
       );
     }
     $pager = array(
-      '#theme' => 'pager',
+      '#type' => 'pager',
     );
     $output['suffix']['#markup'] = '</ol>' . drupal_render($pager);
 

core/modules/system/src/Tests/Pager/PagerTest.php

@@ -63,22 +63,25 @@ function testActiveClass() {
   }
 
   /**
-   * Test proper functioning of the query parameters.
+   * Test proper functioning of the query parameters and the pager cache context.
    */
-  public function testPagerQueryParameters() {
+  protected function testPagerQueryParametersAndCacheContext() {
     // First page.
     $this->drupalGet('pager-test/query-parameters');
     $this->assertText(t('Pager calls: 0'), 'Initial call to pager shows 0 calls.');
+    $this->assertText('pager.0.0');
 
     // 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->assertText(t('Pager calls: 1'), 'First link call to pager shows 1 calls.');
+    $this->assertText('pager.0.60');
 
     // 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->assertText(t('Pager calls: 2'), 'Second link call to pager shows 2 calls.');
+    $this->assertText('pager.0.0');
   }
 
   /**

core/modules/system/tests/modules/pager_test/src/Controller/PagerTestController.php

@@ -52,13 +52,25 @@ public function queryParameters() {
 
     // Pager.
     $build['pager_pager_0'] = array(
-      '#theme' => 'pager',
+      '#type' => 'pager',
       '#element' => 0,
       '#parameters' => array(
         'pager_calls' => ++$pager_calls,
       ),
+      '#pre_render' => [
+        'Drupal\pager_test\Controller\PagerTestController::showPagerCacheContext',
+      ]
     );
 
     return $build;
   }
+
+  /**
+   * #pre_render callback for #type => pager that shows the pager cache context.
+   */
+  public static function showPagerCacheContext(array $pager) {
+    drupal_set_message(\Drupal::service('cache_contexts')->convertTokensToKeys(['pager:' . $pager['#element']])[0]);
+    return $pager;
+  }
+
 }

core/modules/tracker/tracker.pages.inc

@@ -128,7 +128,7 @@ function tracker_page($account = NULL) {
     '#empty' => t('No content available.'),
   );
   $page['pager'] = array(
-    '#theme' => 'pager',
+    '#type' => 'pager',
     '#weight' => 10,
   );
   $page['#sorted'] = TRUE;

core/modules/user/src/UserListBuilder.php

@@ -164,7 +164,7 @@ public function getOperations(EntityInterface $entity) {
   public function render() {
     $build['accounts'] = parent::render();
     $build['accounts']['#empty'] = $this->t('No people available.');
-    $build['pager']['#theme'] = 'pager';
+    $build['pager']['#type'] = 'pager';
     return $build;
   }