diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module index ef12cc1..58c92e0 100644 --- a/core/modules/comment/comment.module +++ b/core/modules/comment/comment.module @@ -537,14 +537,15 @@ function comment_node_view(EntityInterface $node, EntityDisplay $display, $view_ 'fragment' => 'comments', 'html' => TRUE, ); - // Show a link to the first new comment. - if ($new = comment_num_new($node->id())) { + if (module_exists('history')) { $links['comment-new-comments'] = array( - 'title' => format_plural($new, '1 new comment', '@count new comments'), - 'href' => 'node/' . $node->id(), - 'query' => comment_new_page_count($node->comment_count, $new, $node), - 'attributes' => array('title' => t('Jump to the first new comment of this posting.')), - 'fragment' => 'new', + 'title' => '', + 'href' => '', + 'attributes' => array( + 'class' => 'hidden', + 'title' => t('Jump to the first new comment of this posting.'), + 'data-history-node-last-comment-timestamp' => $node->last_comment_timestamp, + ), 'html' => TRUE, ); } @@ -615,6 +616,9 @@ function comment_node_view(EntityInterface $node, EntityDisplay $display, $view_ '#links' => $links, '#attributes' => array('class' => array('links', 'inline')), ); + if ($view_mode == 'teaser' && module_exists('history')) { + $node->content['links']['#attached']['library'][] = array('comment', 'drupal.node-new-comments-link'); + } // Only append comments when we are building a node on its own node detail // page. We compare $node and $page_node to ensure that comments are not @@ -627,6 +631,15 @@ function comment_node_view(EntityInterface $node, EntityDisplay $display, $view_ } /** + * Implements hook_node_view_alter(). + */ +function comment_node_view_alter(&$build, EntityInterface $node, EntityDisplay $display) { + if (module_exists('history')) { + $build['#attributes']['data-history-node-id'] = $node->id(); + } +} + +/** * Builds the comment-related elements for node detail pages. * * @param \Drupal\Core\Entity\EntityInterface $node @@ -809,20 +822,10 @@ function comment_get_thread(EntityInterface $node, $mode, $comments_per_page) { * An array of comment objects, keyed by comment ID. */ function comment_prepare_thread(&$comments) { - // A flag stating if we are still searching for first new comment on the thread. - $first_new = TRUE; - // A counter that helps track how indented we are. $divs = 0; foreach ($comments as $key => $comment) { - if ($first_new && $comment->new->value != MARK_READ) { - // Assign the anchor only for the first new comment. This avoids duplicate - // id attributes on a page. - $first_new = FALSE; - $comment->first_new = TRUE; - } - // The $divs element instructs #prefix whether to add an indent div or // close existing divs (a negative value). $comment->depth = count(explode('.', $comment->thread->value)) - 1; @@ -1511,7 +1514,7 @@ function template_preprocess_comment(&$variables) { '#account' => $account, ); $variables['author'] = drupal_render($username); - $variables['new'] = $comment->new->value ? t('new') : ''; + $variables['new_indicator_timestamp'] = $comment->changed->value; $variables['created'] = format_date($comment->created->value); // Avoid calling format_date() twice on the same timestamp. if ($comment->changed->value == $comment->created->value) { @@ -1603,9 +1606,6 @@ function template_preprocess_comment(&$variables) { if ($variables['status'] != 'published') { $variables['attributes']['class'][] = $variables['status']; } - if ($variables['new']) { - $variables['attributes']['class'][] = 'new'; - } if (!$comment->uid->target_id) { $variables['attributes']['class'][] = 'by-anonymous'; } @@ -1613,13 +1613,13 @@ function template_preprocess_comment(&$variables) { if ($comment->uid->target_id == $variables['node']->getAuthorId()) { $variables['attributes']['class'][] = 'by-node-author'; } - if ($comment->uid->target_id == $variables['user']->id()) { - $variables['attributes']['class'][] = 'by-viewer'; - } } // Add clearfix class. $variables['attributes']['class'][] = 'clearfix'; + // Add comment author user ID. Necessary for the comment-by-viewer library. + $variables['attributes']['data-comment-user-id'] = $comment->uid->value; + $variables['content_attributes']['class'][] = 'content'; } @@ -1784,11 +1784,12 @@ function comment_file_download_access($field, EntityInterface $entity, File $fil * Implements hook_library_info(). */ function comment_library_info() { + $path = drupal_get_path('module', 'comment'); $libraries['drupal.comment'] = array( 'title' => 'Comment', 'version' => VERSION, 'js' => array( - drupal_get_path('module', 'comment') . '/comment-node-form.js' => array(), + $path . '/comment-node-form.js' => array(), ), 'dependencies' => array( array('system', 'jquery'), @@ -1796,6 +1797,42 @@ function comment_library_info() { array('system', 'drupal.form'), ), ); - + $libraries['drupal.comment-by-viewer'] = array( + 'title' => 'Annotate comments by the current viewer for targeted styling', + 'version' => VERSION, + 'js' => array( + $path . '/js/comment-by-viewer.js' => array(), + ), + 'dependencies' => array( + array('system', 'jquery'), + array('system', 'drupal'), + array('system', 'drupalSettings'), + ), + ); + $libraries['drupal.comment-new-indicator'] = array( + 'title' => 'New comment indicator', + 'version' => VERSION, + 'js' => array( + $path . '/js/comment-new-indicator.js' => array(), + ), + 'dependencies' => array( + array('system', 'jquery'), + array('system', 'drupal'), + array('history', 'drupal.history'), + array('system', 'drupal.displace'), + ), + ); + $libraries['drupal.node-new-comments-link'] = array( + 'title' => 'New comments link', + 'version' => VERSION, + 'js' => array( + $path . '/js/node-new-comments-link.js' => array(), + ), + 'dependencies' => array( + array('system', 'jquery'), + array('system', 'drupal'), + array('history', 'drupal.history'), + ), + ); return $libraries; } diff --git a/core/modules/comment/comment.routing.yml b/core/modules/comment/comment.routing.yml index b62c118..1815a0c 100644 --- a/core/modules/comment/comment.routing.yml +++ b/core/modules/comment/comment.routing.yml @@ -19,3 +19,10 @@ comment_permalink: _controller: '\Drupal\comment\Controller\CommentController::commentPermalink' requirements: _entity_access: 'comment.view' + +comment_new_comments_node_links: + pattern: '/comments/render_new_comments_node_links' + defaults: + _controller: '\Drupal\comment\Controller\CommentController::renderNewCommentsNodeLinks' + requirements: + _permission: 'access content' diff --git a/core/modules/comment/js/comment-by-viewer.js b/core/modules/comment/js/comment-by-viewer.js new file mode 100644 index 0000000..06d5b82 --- /dev/null +++ b/core/modules/comment/js/comment-by-viewer.js @@ -0,0 +1,22 @@ +/** + * Attaches behaviors for the Comment module's "by-viewer" class. + */ +(function ($, Drupal, drupalSettings) { + +"use strict"; + +/** + * Add 'by-viewer' class to comments written by the current user. + */ +Drupal.behaviors.commentByViewer = { + attach: function (context) { + var currentUserID = parseInt(drupalSettings.user.uid, 10); + $('[data-comment-user-id]') + .filter(function () { + return parseInt(this.getAttribute('data-comment-user-id'), 10) === currentUserID; + }) + .addClass('by-viewer'); + } +}; + +})(jQuery, Drupal, drupalSettings); diff --git a/core/modules/comment/js/comment-new-indicator.js b/core/modules/comment/js/comment-new-indicator.js new file mode 100644 index 0000000..72c419e --- /dev/null +++ b/core/modules/comment/js/comment-new-indicator.js @@ -0,0 +1,82 @@ +/** + * Attaches behaviors for the Comment module's "new" indicator. + * + * May only be loaded for authenticated users, with the History module enabled. + */ +(function ($, Drupal, window) { + +"use strict"; + +/** + * Render "new" comment indicators wherever necessary. + */ +Drupal.behaviors.commentNewIndicator = { + attach: function (context) { + // Collect all "new" comment indicator placeholders (and their corresponding + // node IDs) newer than 30 days ago that have not already been read after + // their last comment timestamp. + var nodeIDs = []; + var $placeholders = $(context) + .find('[data-comment-timestamp]') + .once('history') + .filter(function () { + var $placeholder = $(this); + var commentTimestamp = parseInt($placeholder.attr('data-comment-timestamp'), 10); + var nodeID = $placeholder.closest('[data-history-node-id]').attr('data-history-node-id'); + if (Drupal.history.needsServerCheck(nodeID, commentTimestamp)) { + nodeIDs.push(nodeID); + return true; + } + else { + return false; + } + }); + + if ($placeholders.length === 0) { + return; + } + + // Fetch the node read timestamps from the server. + Drupal.history.fetchTimestamps(nodeIDs, function () { + processCommentNewIndicators($placeholders); + }); + } +}; + +function processCommentNewIndicators($placeholders) { + var isFirstNewComment = true; + var newCommentString = Drupal.t('new'); + var $placeholder; + + $placeholders.each(function (index, placeholder) { + $placeholder = $(placeholder); + var timestamp = parseInt($placeholder.attr('data-comment-timestamp'), 10); + var $node = $placeholder.closest('[data-history-node-id]'); + var nodeID = $node.attr('data-history-node-id'); + var lastViewTimestamp = Drupal.history.getLastRead(nodeID); + + if (timestamp > lastViewTimestamp) { + // Turn the placeholder into an actual "new" indicator. + var $comment = $(placeholder) + .removeClass('hidden') + .text(newCommentString) + .closest('.comment') + // Add 'new' class to the comment, so it can be styled. + .addClass('new'); + + // Insert "new" anchor just before the "comment-" anchor if + // this is the first new comment in the DOM. + if (isFirstNewComment) { + isFirstNewComment = false; + $comment.prev().before(''); + // If the URL points to the first new comment, then scroll to that + // comment. + if (window.location.hash === '#new') { + window.scrollTo(0, $comment.offset().top - Drupal.displace().top); + } + } + } + }); +} + +})(jQuery, Drupal, window); diff --git a/core/modules/comment/js/node-new-comments-link.js b/core/modules/comment/js/node-new-comments-link.js new file mode 100644 index 0000000..3b74a54 --- /dev/null +++ b/core/modules/comment/js/node-new-comments-link.js @@ -0,0 +1,119 @@ +/** + * Attaches behaviors for the Comment module's "X new comments" link. + * + * May only be loaded for authenticated users, with the History module enabled. + */ +(function ($, Drupal) { + +"use strict"; + +/** + * Render "X new comments" links wherever necessary. + */ +Drupal.behaviors.nodeNewCommentsLink = { + attach: function (context) { + // Collect all "X new comments" node link placeholders (and their + // corresponding node IDs) newer than 30 days ago that have not already been + // read after their last comment timestamp. + var nodeIDs = []; + var $placeholders = $(context) + .find('[data-history-node-last-comment-timestamp]') + .once('history') + .filter(function () { + var $placeholder = $(this); + var lastCommentTimestamp = parseInt($placeholder.attr('data-history-node-last-comment-timestamp'), 10); + var nodeID = $placeholder.closest('[data-history-node-id]').attr('data-history-node-id'); + if (Drupal.history.needsServerCheck(nodeID, lastCommentTimestamp)) { + nodeIDs.push(nodeID); + // Hide this placeholder link until it is certain we'll need it. + hide($placeholder); + return true; + } + else { + // Remove this placeholder link from the DOM because we won't need it. + remove($placeholder); + return false; + } + }); + + if ($placeholders.length === 0) { + return; + } + + // Perform an AJAX request to retrieve node read timestamps. + Drupal.history.fetchTimestamps(nodeIDs, function () { + processNodeNewCommentLinks($placeholders); + }); + } +}; + +function hide($placeholder) { + return $placeholder + // Find the parent
  • . + .closest('.comment-new-comments') + // Find the preceding
  • , if any, and give it the 'last' class. + .prev().addClass('last') + // Go back to the parent
  • and hide it. + .end().hide(); +} + +function remove($placeholder) { + hide($placeholder).remove(); +} + +function show($placeholder) { + return $placeholder + // Find the parent
  • . + .closest('.comment-new-comments') + // Find the preceding
  • , if any, and remove its 'last' class, if any. + .prev().removeClass('last') + // Go back to the parent
  • and show it. + .end().show(); +} + +function processNodeNewCommentLinks($placeholders) { + // Figure out which placeholders need the "x new comments" links. + var $placeholdersToUpdate = {}; + var $placeholder; + $placeholders.each(function (index, placeholder) { + $placeholder = $(placeholder); + var timestamp = parseInt($placeholder.attr('data-history-node-last-comment-timestamp'), 10); + var nodeID = $placeholder.closest('[data-history-node-id]').attr('data-history-node-id'); + var lastViewTimestamp = Drupal.history.getLastRead(nodeID); + + // Queue this placeholder's "X new comments" link to be downloaded from the + // server. + if (timestamp > lastViewTimestamp) { + $placeholdersToUpdate[nodeID] = $placeholder; + } + // No "X new comments" link necessary; remove it from the DOM. + else { + remove($placeholder); + } + }); + + // Perform an AJAX request to retrieve node view timestamps. + var nodeIDs = Object.keys($placeholdersToUpdate); + if (nodeIDs.length === 0) { + return; + } + $.ajax({ + url: Drupal.url('comments/render_new_comments_node_links'), + type: 'POST', + data: { 'node_ids[]' : nodeIDs }, + dataType: 'json', + success: function (results) { + for (var nodeID in results) { + if (results.hasOwnProperty(nodeID) && $placeholdersToUpdate.hasOwnProperty(nodeID)) { + $placeholdersToUpdate[nodeID] + .attr('href', results[nodeID].first_new_comment_link) + .text(Drupal.formatPlural(results[nodeID].new_comment_count, '1 new comment', '@count new comments')) + .removeClass('hidden'); + show($placeholdersToUpdate[nodeID]); + } + } + } + }); +} + +})(jQuery, Drupal); diff --git a/core/modules/comment/lib/Drupal/comment/CommentNewItem.php b/core/modules/comment/lib/Drupal/comment/CommentNewItem.php deleted file mode 100644 index b9dc313..0000000 --- a/core/modules/comment/lib/Drupal/comment/CommentNewItem.php +++ /dev/null @@ -1,41 +0,0 @@ - 'integer', - 'label' => t('Integer value'), - 'class' => '\Drupal\comment\CommentNewValue', - 'computed' => TRUE, - ); - } - return static::$propertyDefinitions; - } -} diff --git a/core/modules/comment/lib/Drupal/comment/CommentNewValue.php b/core/modules/comment/lib/Drupal/comment/CommentNewValue.php deleted file mode 100644 index 1295776..0000000 --- a/core/modules/comment/lib/Drupal/comment/CommentNewValue.php +++ /dev/null @@ -1,44 +0,0 @@ -value)) { - if (!isset($this->parent)) { - throw new InvalidArgumentException('Computed properties require context for computation.'); - } - $field = $this->parent->getParent(); - $entity = $field->getParent(); - $this->value = node_mark($entity->nid->target_id, $entity->changed->value); - } - return $this->value; - } - - /** - * Implements \Drupal\Core\TypedData\TypedDataInterface::setValue(). - */ - public function setValue($value, $notify = TRUE) { - if (isset($value)) { - throw new ReadOnlyException('Unable to set a computed property.'); - } - } -} diff --git a/core/modules/comment/lib/Drupal/comment/CommentRenderController.php b/core/modules/comment/lib/Drupal/comment/CommentRenderController.php index 641c566..71097b6 100644 --- a/core/modules/comment/lib/Drupal/comment/CommentRenderController.php +++ b/core/modules/comment/lib/Drupal/comment/CommentRenderController.php @@ -44,6 +44,8 @@ public function buildContent(array $entities, array $displays, $view_mode, $lang } $nodes = node_load_multiple($nids); + global $user; + foreach ($entities as $entity) { if (isset($nodes[$entity->nid->target_id])) { $node = $nodes[$entity->nid->target_id]; @@ -66,6 +68,14 @@ public function buildContent(array $entities, array $displays, $view_mode, $lang '#attributes' => array('class' => array('links', 'inline')), ); } + + if (!isset($entity->content['#attached'])) { + $entity->content['#attached'] = array(); + } + $entity->content['#attached']['library'][] = array('comment', 'drupal.comment-by-viewer'); + if (module_exists('history') && $user->isAuthenticated()) { + $entity->content['#attached']['library'][] = array('comment', 'drupal.comment-new-indicator'); + } } } @@ -79,11 +89,6 @@ protected function alterBuild(array &$build, EntityInterface $comment, EntityDis $is_threaded = isset($comment->divs) && variable_get('comment_default_mode_' . $comment->bundle(), COMMENT_MODE_THREADED) == COMMENT_MODE_THREADED; - // Add 'new' anchor if needed. - if (!empty($comment->first_new)) { - $prefix .= "\n"; - } - // Add indentation div or close open divs as needed. if ($is_threaded) { $build['#attached']['css'][] = drupal_get_path('module', 'comment') . '/css/comment.theme.css'; diff --git a/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php b/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php index a75dbca..652997f 100644 --- a/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php +++ b/core/modules/comment/lib/Drupal/comment/Controller/CommentController.php @@ -11,12 +11,14 @@ use Drupal\comment\Entity\Comment; use Drupal\Core\Controller\ControllerInterface; use Drupal\Core\Routing\UrlGeneratorInterface; +use Drupal\Core\Session\AccountInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\HttpFoundation\JsonResponse; /** * Controller for the comment entity. @@ -33,6 +35,13 @@ class CommentController implements ControllerInterface { protected $urlGenerator; /** + * The current user service. + * + * @var \Drupal\Core\Session\AccountInterface + */ + protected $currentUser; + + /** * The HTTP kernel. * * @var \Symfony\Component\HttpKernel\HttpKernelInterface @@ -44,11 +53,14 @@ class CommentController implements ControllerInterface { * * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator * The url generator service. + * @param \Drupal\Core\Session\AccountInterface $current_user + * The current user service. * @param \Symfony\Component\HttpKernel\HttpKernelInterface $httpKernel * HTTP kernel to handle requests. */ - public function __construct(UrlGeneratorInterface $url_generator, HttpKernelInterface $httpKernel) { + public function __construct(UrlGeneratorInterface $url_generator, AccountInterface $current_user, HttpKernelInterface $httpKernel) { $this->urlGenerator = $url_generator; + $this->currentUser = $current_user; $this->httpKernel = $httpKernel; } /** @@ -57,6 +69,7 @@ public function __construct(UrlGeneratorInterface $url_generator, HttpKernelInte public static function create(ContainerInterface $container) { return new static( $container->get('url_generator'), + $container->get('current_user'), $container->get('http_kernel') ); } @@ -132,4 +145,39 @@ public function commentPermalink(Request $request, CommentInterface $comment) { throw new NotFoundHttpException(); } + /** + * Returns a set of nodes' last read timestamps. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request of the page. + * + * @return Symfony\Component\HttpFoundation\JsonResponse + * The JSON response. + */ + public function renderNewCommentsNodeLinks(Request $request) { + if ($this->currentUser->isAnonymous()) { + throw new AccessDeniedHttpException(); + } + + $nids = $request->request->get('node_ids'); + if (!isset($nids)) { + throw new NotFoundHttpException(); + } + // Only handle up to 100 nodes. + $nids = array_slice($nids, 0, 100); + + $links = array(); + foreach ($nids as $nid) { + $node = node_load($nid); + $new = comment_num_new($node->id()); + $query = comment_new_page_count($node->comment_count, $new, $node); + $links[$nid] = array( + 'new_comment_count' => (int)$new, + 'first_new_comment_link' => url('node/' . $node->id(), array('query' => $query, 'fragment' => 'new')), + ); + } + + return new JsonResponse($links); + } + } diff --git a/core/modules/comment/lib/Drupal/comment/Entity/Comment.php b/core/modules/comment/lib/Drupal/comment/Entity/Comment.php index 0dcf37d..bff2a45 100644 --- a/core/modules/comment/lib/Drupal/comment/Entity/Comment.php +++ b/core/modules/comment/lib/Drupal/comment/Entity/Comment.php @@ -174,13 +174,6 @@ class Comment extends EntityNG implements CommentInterface { public $node_type; /** - * The comment 'new' marker for the current user. - * - * @var \Drupal\Core\Entity\Field\FieldInterface - */ - public $new; - - /** * Initialize the object. Invoked upon construction and wake up. */ protected function init() { @@ -201,7 +194,6 @@ protected function init() { unset($this->status); unset($this->thread); unset($this->node_type); - unset($this->new); } /** @@ -450,13 +442,6 @@ public static function baseFieldDefinitions($entity_type) { 'type' => 'string_field', 'queryable' => FALSE, ); - $properties['new'] = array( - 'label' => t('Comment new marker'), - 'description' => t("The comment 'new' marker for the current user (0 read, 1 new, 2 updated)."), - 'type' => 'integer_field', - 'computed' => TRUE, - 'class' => '\Drupal\comment\CommentNewItem', - ); return $properties; } } diff --git a/core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php b/core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php index 3948995..160daf1 100644 --- a/core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentCSSTest.php @@ -82,6 +82,11 @@ function testCommentClasses() { } // Request the node with the comment. $this->drupalGet('node/' . $node->id()); + $settings = $this->drupalGetSettings(); + + // Verify the data-history-node-id attribute, which is necessary for the + // by-viewer class and the "new" indicator, see below. + $this->assertIdentical(1, count($this->xpath('//*[@data-history-node-id="' . $node->id() . '"]')), 'data-history-node-id attribute is set on node.'); // Verify classes if the comment is visible for the current user. if ($case['comment_status'] == COMMENT_PUBLISHED || $case['user'] == 'admin') { @@ -103,14 +108,12 @@ function testCommentClasses() { $this->assertFalse(count($comments), 'by-node-author class not found.'); } - // Verify the by-viewer class. - $comments = $this->xpath('//*[contains(@class, "comment") and contains(@class, "by-viewer")]'); - if ($case['comment_uid'] > 0 && $case['comment_uid'] == $case['user_uid']) { - $this->assertTrue(count($comments) == 1, 'by-viewer class found.'); - } - else { - $this->assertFalse(count($comments), 'by-viewer class not found.'); - } + // Verify the data-comment-user-id attribute, which is used by the + // drupal.comment-by-viewer library to add a by-viewer when the current + // user (the viewer) was the author of the comment. We do this in Java- + // Script to prevent breaking the render cache. + $this->assertIdentical(1, count($this->xpath('//*[contains(@class, "comment") and @data-comment-user-id="' . $case['comment_uid'] . '"]')), 'data-comment-user-id attribute is set on comment.'); + $this->assertTrue(isset($settings['ajaxPageState']['js']['core/modules/comment/js/comment-by-viewer.js']), 'drupal.comment-by-viewer library is present.'); } // Verify the unpublished class. @@ -122,20 +125,14 @@ function testCommentClasses() { $this->assertFalse(count($comments), 'unpublished class not found.'); } - // Verify the new class. + // Verify the data-comment-timestamp attribute, which is used by the + // drupal.comment-new-indicator library to add a "new" indicator to each + // comment that was created or changed after the last time the current + // user read the corresponding node. if ($case['comment_status'] == COMMENT_PUBLISHED || $case['user'] == 'admin') { - $comments = $this->xpath('//*[contains(@class, "comment") and contains(@class, "new")]'); - if ($case['user'] != 'anonymous') { - $this->assertTrue(count($comments) == 1, 'new class found.'); - - // Request the node again. The new class should disappear. - $this->drupalGet('node/' . $node->id()); - $comments = $this->xpath('//*[contains(@class, "comment") and contains(@class, "new")]'); - $this->assertFalse(count($comments), 'new class not found.'); - } - else { - $this->assertFalse(count($comments), 'new class not found.'); - } + $this->assertIdentical(1, count($this->xpath('//*[contains(@class, "comment")]/*[@data-comment-timestamp="' . $comment->changed->value . '"]')), 'data-comment-timestamp attribute is set on comment'); + $expectedJS = ($case['user'] !== 'anonymous'); + $this->assertIdentical($expectedJS, isset($settings['ajaxPageState']['js']['core/modules/comment/js/comment-new-indicator.js']), 'drupal.comment-new-indicator library is present.'); } } } diff --git a/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php b/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php index bf7cdb5..7bb0458 100644 --- a/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php @@ -154,10 +154,6 @@ function setEnvironment(array $info) { )); $comment->save(); $this->comment = $comment; - - // comment_num_new() relies on history_read(), so ensure that no one has - // seen the node of this comment. - db_delete('history')->condition('nid', $this->node->id())->execute(); } else { $cids = db_query("SELECT cid FROM {comment}")->fetchCol(); diff --git a/core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php b/core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php index 55deb61..d777cd6 100644 --- a/core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php +++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php @@ -32,6 +32,43 @@ public static function getInfo() { } /** + * Get node "x new comments" metadata from the server for the current user. + * + * @param array $node_ids + * An array of node IDs. + * + * @return string + * The response body. + */ + protected function renderNewCommentsNodeLinks(array $node_ids) { + // Build POST values. + $post = array(); + for ($i = 0; $i < count($node_ids); $i++) { + $post['node_ids[' . $i . ']'] = $node_ids[$i]; + } + + // Serialize POST values. + foreach ($post as $key => $value) { + // Encode according to application/x-www-form-urlencoded + // Both names and values needs to be urlencoded, according to + // http://www.w3.org/TR/html4/interact/forms.html#h-17.13.4.1 + $post[$key] = urlencode($key) . '=' . urlencode($value); + } + $post = implode('&', $post); + + // Perform HTTP request. + return $this->curlExec(array( + CURLOPT_URL => url('comments/render_new_comments_node_links', array('absolute' => TRUE)), + CURLOPT_POST => TRUE, + CURLOPT_POSTFIELDS => $post, + CURLOPT_HTTPHEADER => array( + 'Accept: application/json', + 'Content-Type: application/x-www-form-urlencoded', + ), + )); + } + + /** * Tests new comment marker. */ public function testCommentNewCommentsIndicator() { @@ -41,8 +78,12 @@ public function testCommentNewCommentsIndicator() { $this->node = $this->drupalCreateNode(array('type' => 'article', 'promote' => 1, 'comment' => COMMENT_NODE_OPEN)); $this->drupalGet('node'); $this->assertNoLink(t('@count comments', array('@count' => 0))); - $this->assertNoLink(t('@count new comments', array('@count' => 0))); $this->assertLink(t('Read more')); + // Verify the data-history-node-last-comment-timestamp attribute, which is + // used by the drupal.node-new-comments-link library to determine whether + // a "x new comments" link might be necessary or not. We do this in + // JavaScript to prevent breaking the render cache. + $this->assertIdentical(0, count($this->xpath('//*[@data-history-node-last-comment-timestamp]')), 'data-history-node-last-comment-timestamp attribute is not set.'); // Create a new comment. This helper function may be run with different // comment settings so use $comment->save() to avoid complex setup. @@ -64,17 +105,30 @@ public function testCommentNewCommentsIndicator() { // Log in with 'web user' and check comment links. $this->drupalLogin($this->web_user); $this->drupalGet('node'); - $this->assertLink(t('1 new comment')); - $this->clickLink(t('1 new comment')); - $this->assertRaw('', 'Found "new" marker.'); - $this->assertTrue($this->xpath('//a[@id=:new]/following-sibling::a[1][@id=:comment_id]', array(':new' => 'new', ':comment_id' => 'comment-1')), 'The "new" anchor is positioned at the right comment.'); + // Verify the data-history-node-last-comment-timestamp attribute. Given its + // value, the drupal.node-new-comments-link library would determine that the + // node received a comment after the user last viewed it, and hence it would + // perform an HTTP request to render the "new comments" node link. + $this->assertIdentical(1, count($this->xpath('//*[@data-history-node-last-comment-timestamp="' . $comment->changed->value . '"]')), 'data-history-node-last-comment-timestamp attribute is set to the correct value.'); + $response = $this->renderNewCommentsNodeLinks(array($this->node->id())); + $this->assertResponse(200); + $json = drupal_json_decode($response); + $expected = array($this->node->id() => array( + 'new_comment_count' => 1, + 'first_new_comment_link' => url('node/' . $this->node->id(), array('fragment' => 'new')), + )); + $this->assertIdentical($expected, $json); - // Test if "new comment" link is correctly removed. - $this->drupalGet('node'); - $this->assertLink(t('1 comment')); - $this->assertLink(t('Read more')); - $this->assertNoLink(t('1 new comment')); - $this->assertNoLink(t('@count new comments', array('@count' => 0))); + // Failing to specify node IDs for the endpoint should return a 404. + $this->renderNewCommentsNodeLinks(array()); + $this->assertResponse(404); + + // Accessing the endpoint as the anonymous user should return a 403. + $this->drupalLogout(); + $this->renderNewCommentsNodeLinks(array($this->node->id())); + $this->assertResponse(403); + $this->renderNewCommentsNodeLinks(array()); + $this->assertResponse(403); } } diff --git a/core/modules/comment/templates/comment.html.twig b/core/modules/comment/templates/comment.html.twig index 11dab84..c47ffb6 100644 --- a/core/modules/comment/templates/comment.html.twig +++ b/core/modules/comment/templates/comment.html.twig @@ -15,7 +15,6 @@ * - changed: Formatted date and time for when the comment was last changed. * Preprocess functions can reformat it by calling format_date() with the * desired parameters on the 'comment.changed' variable. - * - new: New comment marker. * - permalink: Comment permalink. * - submitted: Submission information created from author and created * during template_preprocess_comment(). @@ -32,8 +31,6 @@ * - preview: When previewing a new or edited comment. * The following applies only to viewers who are registered users: * - unpublished: An unpublished comment visible only to administrators. - * - by-viewer: Comment by the user currently viewing the page. - * - new: New comment since the last visit. * - title_prefix: Additional output populated by modules, intended to be * displayed in front of the main title tag that appears in the template. * - title_suffix: Additional output populated by modules, intended to be @@ -66,9 +63,12 @@ {{ title_prefix }} - {% if new %} - {{ new }} - {% endif %} + {# + Hide the "new" indicator by default, let a piece of JavaScript ask + the server which comments are new for the user. Rendering the final + "new" indicator here would break the render cache. + #} + {{ title }} diff --git a/core/modules/history/history.module b/core/modules/history/history.module index 38dd8ab..297b2fa 100644 --- a/core/modules/history/history.module +++ b/core/modules/history/history.module @@ -106,3 +106,24 @@ function history_user_delete($account) { ->condition('uid', $account->id()) ->execute(); } + +/** + * Implements hook_library_info(). + */ +function history_library_info() { + $libraries['drupal.history'] = array( + 'title' => 'History', + 'version' => VERSION, + 'js' => array( + drupal_get_path('module', 'history') . '/js/history.js' => array(), + ), + 'dependencies' => array( + array('system', 'jquery'), + array('system', 'drupalSettings'), + array('system', 'drupal'), + array('system', 'drupal.ajax'), + ), + ); + + return $libraries; +} diff --git a/core/modules/history/history.routing.yml b/core/modules/history/history.routing.yml new file mode 100644 index 0000000..4069c30 --- /dev/null +++ b/core/modules/history/history.routing.yml @@ -0,0 +1,13 @@ +history_get_last_node_view: + pattern: '/history/get_node_read_timestamps' + defaults: + _controller: '\Drupal\history\Controller\HistoryController::getNodeReadTimestamps' + requirements: + _permission: 'access content' + +history_read_node: + pattern: '/history/{node}/read' + defaults: + _controller: '\Drupal\history\Controller\HistoryController::readNode' + requirements: + _entity_access: 'node.view' diff --git a/core/modules/history/js/history.js b/core/modules/history/js/history.js new file mode 100644 index 0000000..de9c745 --- /dev/null +++ b/core/modules/history/js/history.js @@ -0,0 +1,98 @@ +/** + * JavaScript API for the History module, with client-side caching. + * + * May only be loaded for authenticated users, with the History module enabled. + */ +(function ($, Drupal, drupalSettings, storage) { + +"use strict"; + +var currentUserID = parseInt(drupalSettings.user.uid, 10); + +// Any comment that is older than 30 days is automatically considered read, +// so for these we don't need to perform a request at all! +var thirtyDaysAgo = Math.round(new Date().getTime() / 1000) - 30 * 24 * 60 * 60; + +Drupal.history = { + + /** + * Fetch "last read" timestamps for the given nodes. + * + * @param Array nodeIDs + * An array of node IDs. + * @param Function callback + * A callback that is called after the requested timestamps were fetched. + */ + fetchTimestamps: function (nodeIDs, callback) { + $.ajax({ + url: Drupal.url('history/get_node_read_timestamps'), + type: 'POST', + data: { 'node_ids[]' : nodeIDs }, + dataType: 'json', + success: function (results) { + for (var nodeID in results) { + if (results.hasOwnProperty(nodeID)) { + storage.setItem('Drupal.history.' + currentUserID + '.' + nodeID, results[nodeID]); + } + } + callback(); + } + }); + }, + + /** + * Get the last read timestamp for the given node. + * + * @param Number|String nodeID + * A node ID. + * + * @return Number + * A UNIX timestamp. + */ + getLastRead: function (nodeID) { + return parseInt(storage.getItem('Drupal.history.' + currentUserID + '.' + nodeID) || 0, 10); + }, + + /** + * Marks a node as read, store the last read timestamp in client-side storage. + * + * @param Number|String nodeID + * A node ID. + */ + markAsRead: function (nodeID) { + $.ajax({ + url: Drupal.url('history/' + nodeID + '/read'), + type: 'POST', + dataType: 'json', + success: function (timestamp) { + storage.setItem('Drupal.history.' + currentUserID + '.' + nodeID, timestamp); + } + }); + }, + + /** + * Determines whether a server check is necessary. + * + * Any content that is >30 days old never gets a "new" or "updated" indicator. + * Any content that was published before the oldest known reading also never + * gets a "new" or "updated" indicator, because it must've been read already. + * + * @param Number|String nodeID + * A node ID. + * @param Number contentTimestamp + * The time at which some content (e.g. a comment) was published. + * + * @return Boolean + * Whether a server check is necessary for the given node and its timestamp. + */ + needsServerCheck: function (nodeID, contentTimestamp) { + // First check if the content is older than 30 days, then we can bail early. + if (contentTimestamp < thirtyDaysAgo) { + return false; + } + var minLastReadTimestamp = parseInt(storage.getItem('Drupal.history.' + currentUserID + '.' + nodeID) || 0, 10); + return contentTimestamp > minLastReadTimestamp; + } +}; + +})(jQuery, Drupal, drupalSettings, window.localStorage); diff --git a/core/modules/history/lib/Drupal/history/Controller/HistoryController.php b/core/modules/history/lib/Drupal/history/Controller/HistoryController.php new file mode 100644 index 0000000..1c4849c --- /dev/null +++ b/core/modules/history/lib/Drupal/history/Controller/HistoryController.php @@ -0,0 +1,69 @@ +currentUser()->isAnonymous()) { + throw new AccessDeniedHttpException(); + } + + $nids = $request->request->get('node_ids'); + if (!isset($nids)) { + throw new NotFoundHttpException(); + } + // Only handle up to 100 nodes. + $nids = array_slice($nids, 0, 100); + + $timestamps = array(); + foreach ($nids as $nid) { + $timestamps[$nid] = (int) history_read($nid); + } + return new JsonResponse($timestamps); + } + + /** + * Marks a node as read by the current user right now. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request of the page. + * @param \Drupal\node\NodeInterface $node + * The node whose "last read" timestamp should be updated. + */ + public function readNode(Request $request, NodeInterface $node) { + if ($this->currentUser()->isAnonymous()) { + throw new AccessDeniedHttpException(); + } + + // Update the history table, stating that this user viewed this node. + history_write($node->id()); + + return new JsonResponse((int)history_read($node->id())); + } + +} diff --git a/core/modules/history/lib/Drupal/history/Tests/HistoryTest.php b/core/modules/history/lib/Drupal/history/Tests/HistoryTest.php new file mode 100644 index 0000000..84356f2 --- /dev/null +++ b/core/modules/history/lib/Drupal/history/Tests/HistoryTest.php @@ -0,0 +1,155 @@ + 'History endpoints', + 'description' => 'Tests the History endpoints', + 'group' => 'History' + ); + } + + function setUp() { + parent::setUp(); + + $this->drupalCreateContentType(array('type' => 'page', 'name' => 'Basic page')); + + $this->user = $this->drupalCreateUser(array('create page content', 'access content')); + $this->drupalLogin($this->user); + $this->test_node = $this->drupalCreateNode(array('type' => 'page', 'uid' => $this->user->id())); + } + + /** + * Get node read timestamps from the server for the current user. + * + * @param array $node_ids + * An array of node IDs. + * + * @return string + * The response body. + */ + protected function getNodeReadTimestamps(array $node_ids) { + // Build POST values. + $post = array(); + for ($i = 0; $i < count($node_ids); $i++) { + $post['node_ids[' . $i . ']'] = $node_ids[$i]; + } + + // Serialize POST values. + foreach ($post as $key => $value) { + // Encode according to application/x-www-form-urlencoded + // Both names and values needs to be urlencoded, according to + // http://www.w3.org/TR/html4/interact/forms.html#h-17.13.4.1 + $post[$key] = urlencode($key) . '=' . urlencode($value); + } + $post = implode('&', $post); + + // Perform HTTP request. + return $this->curlExec(array( + CURLOPT_URL => url('history/get_node_read_timestamps', array('absolute' => TRUE)), + CURLOPT_POST => TRUE, + CURLOPT_POSTFIELDS => $post, + CURLOPT_HTTPHEADER => array( + 'Accept: application/json', + 'Content-Type: application/x-www-form-urlencoded', + ), + )); + } + + /** + * Mark a node as read for the current user. + * + * @param int $node_id + * A node ID. + * + * @return string + * The response body. + */ + protected function markNodeAsRead($node_id) { + return $this->curlExec(array( + CURLOPT_URL => url('history/' . $node_id . '/read', array('absolute' => TRUE)), + CURLOPT_HTTPHEADER => array( + 'Accept: application/json', + ), + )); + } + + /** + * Verifies that the history endpoints work. + */ + function testHistory() { + $nid = $this->test_node->id(); + + // Retrieve "last read" timestamp for test node, for the current user. + $response = $this->getNodeReadTimestamps(array($nid)); + $this->assertResponse(200); + $json = drupal_json_decode($response); + $this->assertIdentical(array(1 => 0), $json, 'The node has not yet been read.'); + + // View the node. + $this->drupalGet('node/' . $nid); + // JavaScript present to record the node read. + $settings = $this->drupalGetSettings(); + $this->assertTrue(isset($settings['ajaxPageState']['js']['core/modules/history/js/history.js']), 'drupal.history library is present.'); + $this->assertRaw('Drupal.history.markAsRead(' . $nid . ')', 'History module JavaScript API call to mark node as read present on page.'); + + // Simulate JavaScript: perform HTTP request to mark node as read. + $response = $this->markNodeAsRead($nid); + $this->assertResponse(200); + $timestamp = drupal_json_decode($response); + $this->assertTrue(is_numeric($timestamp), 'Node has been marked as read. Timestamp received.'); + + // Retrieve "last read" timestamp for test node, for the current user. + $response = $this->getNodeReadTimestamps(array($nid)); + $this->assertResponse(200); + $json = drupal_json_decode($response); + $this->assertIdentical(array(1 => $timestamp), $json, 'The node has been read.'); + + // Failing to specify node IDs for the first endpoint should return a 404. + $this->getNodeReadTimestamps(array()); + $this->assertResponse(404); + + // Accessing either endpoint as the anonymous user should return a 403. + $this->drupalLogout(); + $this->getNodeReadTimestamps(array($nid)); + $this->assertResponse(403); + $this->getNodeReadTimestamps(array()); + $this->assertResponse(403); + $this->markNodeAsRead($nid); + $this->assertResponse(403); + } +} diff --git a/core/modules/node/node.module b/core/modules/node/node.module index 44f60e7..3863c25 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -585,14 +585,28 @@ function node_show(EntityInterface $node, $message = FALSE) { } // For markup consistency with other pages, use node_view_multiple() rather than node_view(). - $nodes = array('nodes' => node_view_multiple(array($node->id() => $node), 'full')); + $page = array('nodes' => node_view_multiple(array($node->id() => $node), 'full')); // Update the history table, stating that this user viewed this node. - if (module_exists('history')) { - history_write($node->id()); + global $user; + if (module_exists('history') && $user->isAuthenticated()) { + $page['#attached'] = array( + 'js' => array( + // When the window's "load" event is triggered, mark the node as read. + // This still allows for Drupal behaviors (which are triggered on the + // "DOMContentReady" event) to add "new" and "updated" indicators. + array( + 'data' => 'window.addEventListener("load",function(){Drupal.history.markAsRead(' . $node->id() . ');},false);', + 'type' => 'inline', + ), + ), + 'library' => array( + array('history', 'drupal.history'), + ) + ); } - return $nodes; + return $page; } /** diff --git a/core/modules/system/lib/Drupal/system/Tests/Theme/ThemeInfoStylesTest.php b/core/modules/system/lib/Drupal/system/Tests/Theme/ThemeInfoStylesTest.php index 47acb01..c152a27 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Theme/ThemeInfoStylesTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Theme/ThemeInfoStylesTest.php @@ -47,18 +47,19 @@ function testStylesheets() { $this->drupalGet('theme-test/info/stylesheets'); - $this->assertRaw("$base/base-add.css"); - $this->assertRaw("$base/base-override.css"); - $this->assertNoRaw("base-remove.css"); + $this->assertIdentical(1, count($this->xpath("//link[contains(@href, '$base/base-add.css')]")), "$base/base-add.css found"); + $this->assertIdentical(1, count($this->xpath("//link[contains(@href, '$base/base-override.css')]")), "$base/base-override.css found"); + $this->assertIdentical(0, count($this->xpath("//link[contains(@href, 'base-remove.css')]")), "base-remove.css not found"); - $this->assertRaw("$sub/sub-add.css"); + $this->assertIdentical(1, count($this->xpath("//link[contains(@href, '$sub/sub-add.css')]")), "$sub/sub-add.css found"); - $this->assertRaw("$sub/sub-override.css"); - $this->assertRaw("$sub/base-add.sub-override.css"); - $this->assertRaw("$sub/base-remove.sub-override.css"); + $this->assertIdentical(1, count($this->xpath("//link[contains(@href, '$sub/sub-override.css')]")), "$sub/sub-override.css found"); + $this->assertIdentical(1, count($this->xpath("//link[contains(@href, '$sub/base-add.sub-override.css')]")), "$sub/base-add.sub-override.css found"); + $this->assertIdentical(1, count($this->xpath("//link[contains(@href, '$sub/base-remove.sub-override.css')]")), "$sub/base-remove.sub-override.css found"); - $this->assertNoRaw("sub-remove.css"); - $this->assertNoRaw("base-add.sub-remove.css"); - $this->assertNoRaw("base-override.sub-remove.css"); + $this->assertIdentical(0, count($this->xpath("//link[contains(@href, 'sub-remove.css')]")), "sub-remove.css not found"); + $this->assertIdentical(0, count($this->xpath("//link[contains(@href, 'base-add.sub-remove.css')]")), "base-add.sub-remove.css not found"); + $this->assertIdentical(0, count($this->xpath("//link[contains(@href, 'base-override.sub-remove.css')]")), "base-override.sub-remove.css not found"); } + } diff --git a/core/modules/tracker/js/tracker-history.js b/core/modules/tracker/js/tracker-history.js new file mode 100644 index 0000000..df7b686 --- /dev/null +++ b/core/modules/tracker/js/tracker-history.js @@ -0,0 +1,122 @@ +/** + * Attaches behaviors for the Tracker module's History module integration. + * + * May only be loaded for authenticated users, with the History module enabled. + */ +(function ($, Drupal, window) { + +"use strict"; + +/** + * Render "new" and "updated" node indicators, as well as "X new" replies links. + */ +Drupal.behaviors.trackerHistory = { + attach: function (context) { + // Find all "new" comment indicator placeholders newer than 30 days ago that + // have not already been read after their last comment timestamp. + var nodeIDs = []; + var $nodeNewPlaceholders = $(context) + .find('[data-history-node-timestamp]') + .once('history') + .filter(function () { + var nodeTimestamp = parseInt(this.getAttribute('data-history-node-timestamp'), 10); + var nodeID = this.getAttribute('data-history-node-id'); + if (Drupal.history.needsServerCheck(nodeID, nodeTimestamp)) { + nodeIDs.push(nodeID); + return true; + } + else { + return false; + } + }); + + // Find all "new" comment indicator placeholders newer than 30 days ago that + // have not already been read after their last comment timestamp. + var $newRepliesPlaceholders = $(context) + .find('[data-history-node-last-comment-timestamp]') + .once('history') + .filter(function () { + var lastCommentTimestamp = parseInt(this.getAttribute('data-history-node-last-comment-timestamp'), 10); + var nodeTimestamp = parseInt(this.previousSibling.previousSibling.getAttribute('data-history-node-timestamp'), 10); + // Discard placeholders that have zero comments. + if (lastCommentTimestamp === nodeTimestamp) { + return false; + } + var nodeID = this.previousSibling.previousSibling.getAttribute('data-history-node-id'); + if (Drupal.history.needsServerCheck(nodeID, lastCommentTimestamp)) { + if (nodeIDs.indexOf(nodeID) === -1) { + nodeIDs.push(nodeID); + } + return true; + } + else { + return false; + } + }); + + if ($nodeNewPlaceholders.length === 0 && $newRepliesPlaceholders.length === 0) { + return; + } + + // Fetch the node read timestamps from the server. + Drupal.history.fetchTimestamps(nodeIDs, function () { + processNodeNewIndicators($nodeNewPlaceholders); + processNewRepliesIndicators($newRepliesPlaceholders); + }); + } +}; + +function processNodeNewIndicators($placeholders) { + var newNodeString = Drupal.t('new'); + var updatedNodeString = Drupal.t('updated'); + + $placeholders.each(function (index, placeholder) { + var timestamp = parseInt(placeholder.getAttribute('data-history-node-timestamp'), 10); + var nodeID = placeholder.getAttribute('data-history-node-id'); + var lastViewTimestamp = Drupal.history.getLastRead(nodeID); + + if (timestamp > lastViewTimestamp) { + var message = (lastViewTimestamp === 0) ? newNodeString : updatedNodeString; + $(placeholder).append('' + message + ''); + } + }); +} + +function processNewRepliesIndicators($placeholders) { + // Figure out which placeholders need the "x new" replies links. + var placeholdersToUpdate = {}; + $placeholders.each(function (index, placeholder) { + var timestamp = parseInt(placeholder.getAttribute('data-history-node-last-comment-timestamp'), 10); + var nodeID = placeholder.previousSibling.previousSibling.getAttribute('data-history-node-id'); + var lastViewTimestamp = Drupal.history.getLastRead(nodeID); + + // Queue this placeholder's "X new" replies link to be downloaded from the + // server. + if (timestamp > lastViewTimestamp) { + placeholdersToUpdate[nodeID] = placeholder; + } + }); + + // Perform an AJAX request to retrieve node view timestamps. + var nodeIDs = Object.keys(placeholdersToUpdate); + if (nodeIDs.length === 0) { + return; + } + $.ajax({ + url: Drupal.url('comments/render_new_comments_node_links'), + type: 'POST', + data: { 'node_ids[]' : nodeIDs }, + dataType: 'json', + success: function (results) { + for (var nodeID in results) { + if (results.hasOwnProperty(nodeID) && placeholdersToUpdate.hasOwnProperty(nodeID)) { + var url = results[nodeID].first_new_comment_link; + var text = Drupal.formatPlural(results[nodeID].new_comment_count, '1 new', '@count new'); + $(placeholdersToUpdate[nodeID]).append('
    ' + text + ''); + } + } + } + }); +} + +})(jQuery, Drupal, window); diff --git a/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerTest.php b/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerTest.php index b11df97..95495fd 100644 --- a/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerTest.php +++ b/core/modules/tracker/lib/Drupal/tracker/Tests/TrackerTest.php @@ -52,6 +52,10 @@ function setUp() { $permissions = array('access comments', 'create page content', 'post comments', 'skip comment approval'); $this->user = $this->drupalCreateUser($permissions); $this->other_user = $this->drupalCreateUser($permissions); + user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array( + 'access content', + 'access user profiles', + )); // Make node preview optional. variable_set('comment_preview_page', 0); @@ -130,70 +134,52 @@ function testTrackerUser() { } /** - * Tests for the presence of the "new" flag for nodes. + * Tests the metadata for the "new"/"updated" indicators. */ - function testTrackerNewNodes() { + function testTrackerHistoryMetadata() { $this->drupalLogin($this->user); + // Create a page node. $edit = array( 'title' => $this->randomName(8), ); - $node = $this->drupalCreateNode($edit); - $title = $edit['title']; - $this->drupalGet('tracker'); - $this->assertPattern('/' . $title . '.*new/', 'New nodes are flagged as such in the tracker listing.'); - $this->drupalGet('node/' . $node->id()); - $this->drupalGet('tracker'); - $this->assertNoPattern('/' . $title . '.*new/', 'Visited nodes are not flagged as new.'); - - $this->drupalLogin($this->other_user); + // Verify. $this->drupalGet('tracker'); - $this->assertPattern('/' . $title . '.*new/', 'For another user, new nodes are flagged as such in the tracker listing.'); - - $this->drupalGet('node/' . $node->id()); - $this->drupalGet('tracker'); - $this->assertNoPattern('/' . $title . '.*new/', 'For another user, visited nodes are not flagged as new.'); - } - - /** - * Tests for comment counters on the tracker listing. - */ - function testTrackerNewComments() { - $this->drupalLogin($this->user); - - $node = $this->drupalCreateNode(array( - 'comment' => 2, - 'title' => $this->randomName(8), - )); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->getChangedTime()); + $this->drupalGet('tracker/' . $this->user->id()); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->getChangedTime()); + $this->drupalGet('user/' . $this->user->id() . '/track'); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->getChangedTime()); - // Add a comment to the page. - $comment = array( + // Add a comment to the page, make sure it is created after the node. + $edit = array( 'subject' => $this->randomName(), 'comment_body[' . Language::LANGCODE_NOT_SPECIFIED . '][0][value]' => $this->randomName(20), ); - // The new comment is automatically viewed by the current user. - $this->drupalPost('comment/reply/' . $node->id(), $comment, t('Save')); + sleep(1); + $this->drupalPost('comment/reply/' . $node->id(), $edit, t('Save')); + // Reload the node so that comment.module's hook_node_load() + // implementation can set $node->last_comment_timestamp for the freshly + // posted comment. + $node = node_load($node->id()); - $this->drupalLogin($this->other_user); + // Verify. $this->drupalGet('tracker'); - $this->assertText('1 new', 'New comments are counted on the tracker listing pages.'); - $this->drupalGet('node/' . $node->id()); - - // Add another comment as other_user. - $comment = array( - 'subject' => $this->randomName(), - 'comment_body[' . Language::LANGCODE_NOT_SPECIFIED . '][0][value]' => $this->randomName(20), - ); - // If the comment is posted in the same second as the last one then Drupal - // can't tell the difference, so we wait one second here. - sleep(1); - $this->drupalPost('comment/reply/' . $node->id(), $comment, t('Save')); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->last_comment_timestamp); + $this->drupalGet('tracker/' . $this->user->id()); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->last_comment_timestamp); + $this->drupalGet('user/' . $this->user->id() . '/track'); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->last_comment_timestamp); - $this->drupalLogin($this->user); + // Log out, now verify that the metadata is still there, but the library is + // not. + $this->drupalLogout(); $this->drupalGet('tracker'); - $this->assertText('1 new', 'New comments are counted on the tracker listing pages.'); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->last_comment_timestamp, FALSE); + $this->drupalGet('user/' . $this->user->id() . '/track'); + $this->assertHistoryMetadata($node->id(), $node->getChangedTime(), $node->last_comment_timestamp, FALSE); } /** @@ -240,9 +226,6 @@ function testTrackerCronIndexing() { foreach ($nodes as $i => $node) { $this->assertText($node->label(), format_string('Node @i is displayed on the tracker listing pages.', array('@i' => $i))); } - $this->assertText('1 new', 'New comment is counted on the tracker listing pages.'); - $this->assertText('updated', 'Node is listed as updated'); - // Fetch the site-wide tracker. $this->drupalGet('tracker'); @@ -250,7 +233,6 @@ function testTrackerCronIndexing() { foreach ($nodes as $i => $node) { $this->assertText($node->label(), format_string('Node @i is displayed on the tracker listing pages.', array('@i' => $i))); } - $this->assertText('1 new', 'New comment is counted on the tracker listing pages.'); } /** @@ -280,4 +262,35 @@ function testTrackerAdminUnpublish() { $this->drupalGet('tracker'); $this->assertText(t('No content available.'), 'Node is displayed on the tracker listing pages.'); } + + /** + * Passes if the appropriate history metadata exists. + * + * Verify the data-history-node-id, data-history-node-timestamp and + * data-history-node-last-comment-timestamp attributes, which are used by the + * drupal.tracker-history library to add the appropriate "new" and "updated" + * indicators, as well as the "x new" replies link to the tracker. + * We do this in JavaScript to prevent breaking the render cache. + * + * @param int $node_id + * A node ID, that must exist as a data-history-node-id attribute + * @param int $node_timestamp + * A node timestamp, that must exist as a data-history-node-timestamp + * attribute. + * @param int $node_last_comment_timestamp + * A node's last comment timestamp, that must exist as a + * data-history-node-last-comment-timestamp attribute. + * @param bool $library_is_present + * Whether the drupal.tracker-history library should be present or not. + * + * @return + * TRUE if the assertion succeeded, FALSE otherwise. + */ + function assertHistoryMetadata($node_id, $node_timestamp, $node_last_comment_timestamp, $library_is_present = TRUE) { + $settings = $this->drupalGetSettings(); + $this->assertIdentical($library_is_present, isset($settings['ajaxPageState']['js']['core/modules/tracker/js/tracker-history.js']), 'drupal.tracker-history library is present.'); + $this->assertIdentical(1, count($this->xpath('//table/tbody/tr/td[@data-history-node-id="' . $node_id . '" and @data-history-node-timestamp="' . $node_timestamp . '"]')), 'Tracker table cell contains the data-history-node-id and data-history-node-timestamp attributes for the node.'); + $this->assertIdentical(1, count($this->xpath('//table/tbody/tr/td[@data-history-node-last-comment-timestamp="' . $node_last_comment_timestamp . '"]')), 'Tracker table cell contains the data-history-node-last-comment-timestamp attribute for the node.'); + } + } diff --git a/core/modules/tracker/tracker.module b/core/modules/tracker/tracker.module index 581a10d..1b9ee96 100644 --- a/core/modules/tracker/tracker.module +++ b/core/modules/tracker/tracker.module @@ -159,6 +159,27 @@ function tracker_cron() { } /** + * Implements hook_library_info(). + */ +function tracker_library_info() { + $libraries['drupal.tracker-history'] = array( + 'title' => 'Tracker history', + 'version' => VERSION, + 'js' => array( + drupal_get_path('module', 'tracker') . '/js/tracker-history.js' => array(), + ), + 'dependencies' => array( + array('system', 'jquery'), + array('system', 'drupal'), + array('history', 'drupal.history'), + ), + ); + + return $libraries; +} + + +/** * Access callback: Determines access permission for a user's own account. * * @param int $account diff --git a/core/modules/tracker/tracker.pages.inc b/core/modules/tracker/tracker.pages.inc index 8e187ef..5cd521a 100644 --- a/core/modules/tracker/tracker.pages.inc +++ b/core/modules/tracker/tracker.pages.inc @@ -18,6 +18,8 @@ * @see tracker_menu() */ function tracker_page($account = NULL, $set_title = FALSE) { + global $user; + if ($account) { $query = db_select('tracker_user', 't') ->extend('Drupal\Core\Database\Query\PagerSelectExtender') @@ -67,23 +69,23 @@ function tracker_page($account = NULL, $set_title = FALSE) { $comments = 0; if ($node->comment_count) { $comments = $node->comment_count; - - if ($new = comment_num_new($node->id())) { - $comments .= '
    '; - $comments .= l(format_plural($new, '1 new', '@count new'), 'node/' . $node->id(), array('fragment' => 'new')); - } } - $mark_build = array( - '#theme' => 'mark', - '#status' => node_mark($node->id(), $node->getChangedTime()), - ); + $mark_build = array('#theme' => 'mark', '#status' => ''); $row = array( 'type' => check_plain(node_get_type_label($node)), - 'title' => array('data' => l($node->getTitle(), 'node/' . $node->id()) . ' ' . drupal_render($mark_build)), + 'title' => array( + 'data-history-node-id' => $node->id(), + 'data-history-node-timestamp' => $node->getChangedTime(), + 'data' => l($node->getTitle(), 'node/' . $node->id()) . ' ' . drupal_render($mark_build) + ), 'author' => array('data' => array('#theme' => 'username', '#account' => $node->getAuthor())), - 'replies' => array('class' => array('replies'), 'data' => $comments), + 'replies' => array( + 'class' => array('replies'), + 'data-history-node-last-comment-timestamp' => $node->last_comment_timestamp, + 'data' => $comments + ), 'last updated' => array('data' => t('!time ago', array('!time' => format_interval(REQUEST_TIME - $node->last_activity)))), ); @@ -143,5 +145,9 @@ function tracker_page($account = NULL, $set_title = FALSE) { ); $page['#sorted'] = TRUE; + if (module_exists('history') && $user->isAuthenticated()) { + $page['#attached']['library'][] = array('tracker', 'drupal.tracker-history'); + } + return $page; } diff --git a/core/modules/user/user.module b/core/modules/user/user.module index 62ccfd6..e0a53e4 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -117,6 +117,17 @@ function user_theme() { function user_page_build(&$page) { $path = drupal_get_path('module', 'user'); $page['#attached']['css'][$path . '/css/user.module.css'] = array('every_page' => TRUE); + + // Provide the user ID in drupalSettings to allow JavaScript code to customize + // the experience for the end user, rather than the server side, which would + // break the render cache. + global $user; + $page['#attached']['js'][] = array( + 'type' => 'setting', + 'data' => array('user' => array( + 'uid' => $user->id(), + )), + ); } /** diff --git a/core/themes/bartik/templates/comment.html.twig b/core/themes/bartik/templates/comment.html.twig index 2671268..26168f9 100644 --- a/core/themes/bartik/templates/comment.html.twig +++ b/core/themes/bartik/templates/comment.html.twig @@ -15,7 +15,6 @@ * - changed: Formatted date and time for when the comment was last changed. * Preprocess functions can reformat it by calling format_date() with the * desired parameters on the 'comment.changed' variable. - * - new: New comment marker. * - permalink: Comment permalink. * - submitted: Submission information created from author and created * during template_preprocess_comment(). @@ -32,8 +31,6 @@ * - preview: When previewing a new or edited comment. * The following applies only to viewers who are registered users: * - unpublished: An unpublished comment visible only to administrators. - * - by-viewer: Comment by the user currently viewing the page. - * - new: New comment since the last visit. * - title_prefix: Additional output populated by modules, intended to be * displayed in front of the main title tag that appears in the template. * - title_suffix: Additional output populated by modules, intended to be @@ -96,9 +93,12 @@
    - {% if new %} - {{ new }} - {% endif %} + {# + Hide the "new" indicator by default, let a piece of JavaScript ask + the server which comments are new for the user. Rendering the final + "new" indicator here would break the render cache. + #} + {{ title_prefix }} {{ title }}