Problem/Motivation

If your node template doesn't print {{ content }} or explicitly {{ content.statistics_content_counter }}, then statistics.js is not added.

Proposed resolution

Add the library to $build['#attached'], then it apparently works if at least one {{ content.something }} exists. I don't know why it works, other than @WimLeers said so ;)

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
1.04 KB

Patch was created on an older core version, let's see if this still applies.

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community
Berdir’s picture

Completely forgot about this one, thanks for the RTBC :)

Note: Adding a test for this would require a test theme with a node template that doesn't contain {{ content }}, that quite some effort.

timmillwood’s picture

Right, because there's no proper tests I'll look out for bug reports.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Re #3 this this like pretty important functionality for the statistics module and probably something that should be tested. #2385243: Make core user fields available for twig templates added a test theme for a similar reason - didn't seem like too much work.

thenchev’s picture

FileSize
6.04 KB
7.09 KB

Working on it.

timmillwood’s picture

Great start @Denchev thanks for the work on this!

thenchev’s picture

Status: Needs work » Needs review
FileSize
7.67 KB
2.53 KB
slashrsm’s picture

Status: Needs review » Needs work

Few cosmetic comments:

  1. +++ b/core/modules/statistics/src/Tests/StatisticsAttachedTest.php
    @@ -0,0 +1,63 @@
    +    // Install "block_test_theme" and set it as the default theme.
    

    s/block_test_theme/statistics_test_attached

    Copy-paste kind of thing I assume.

  2. +++ b/core/modules/statistics/src/Tests/StatisticsAttachedTest.php
    @@ -0,0 +1,63 @@
    +    global $base_root;
    +    $this->assertNotIdentical(strpos($page, '<script src="' . $base_root . base_path() . 'core/modules/statistics/statistics.js'), FALSE, 'Statistics library is available');
    

    Is there a way to avoid using the global here?

    Maybe asserting just statistics.js? Something like:

    $this->assertRaw('core/modules/statistics/statistics.js')
    
  3. +++ b/core/modules/statistics/tests/themes/statistics_test_attached/node.html.twig
    @@ -0,0 +1,93 @@
    + * Available variables:
    + * - node: Full node entity.
    + *   - id: The node ID.
    + *   - bundle: The type of the node, for example, "page" or "article".
    + *   - authorid: The user ID of the node author.
    + *   - createdtime: Time the node was published formatted in Unix timestamp.
    + *   - changedtime: Time the node was changed formatted in Unix timestamp.
    + * - label: The title of the node.
    + * - content: All node items. Use {{ content }} to print them all,
    + *   or print a subset such as {{ content.field_example }}. Use
    + *   {{ content|without('field_example') }} to temporarily suppress the printing
    + *   of a given child element.
    + * - author_picture: The node author user entity, rendered using the "compact"
    + *   view mode.
    + * - metadata: Metadata for this node.
    + * - date: Themed creation date field.
    + * - author_name: Themed author name field.
    + * - url: Direct URL of the current node.
    + * - display_submitted: Whether submission information should be displayed.
    + * - attributes: HTML attributes for the containing element.
    + *   The attributes.class element may contain one or more of the following
    + *   classes:
    + *   - node: The current template type (also known as a "theming hook").
    + *   - node--type-[type]: The current node type. For example, if the node is an
    + *     "Article" it would result in "node--type-article". Note that the machine
    + *     name will often be in a short form of the human readable label.
    + *   - node--view-mode-[view_mode]: The View Mode of the node; for example, a
    + *     teaser would result in: "node--view-mode-teaser", and
    + *     full: "node--view-mode-full".
    + *   The following are controlled through the node publishing options.
    + *   - node--promoted: Appears on nodes promoted to the front page.
    + *   - node--sticky: Appears on nodes ordered above other non-sticky nodes in
    + *     teaser listings.
    + *   - node--unpublished: Appears on unpublished nodes visible only to site
    + *     admins.
    + * - title_attributes: Same as attributes, except applied to the main title
    + *   tag that appears in the template.
    + * - content_attributes: Same as attributes, except applied to the main
    + *   content tag that appears in the template.
    + * - author_attributes: Same as attributes, except applied to the author of
    + *   the node tag that appears in the template.
    + * - 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
    + *   displayed after the main title tag that appears in the template.
    + * - view_mode: View mode; for example, "teaser" or "full".
    + * - teaser: Flag for the teaser state. Will be true if view_mode is 'teaser'.
    + * - page: Flag for the full page state. Will be true if view_mode is 'full'.
    + * - readmore: Flag for more state. Will be true if the teaser content of the
    + *   node cannot hold the main body content.
    + * - logged_in: Flag for authenticated user status. Will be true when the
    + *   current user is a logged-in member.
    + * - is_admin: Flag for admin user status. Will be true when the current user
    + *   is an administrator.
    + *
    + * @see template_preprocess_node()
    + *
    + * @todo Remove the id attribute (or make it a class), because if that gets
    + *   rendered twice on a page this is invalid CSS for example: two lists
    + *   in different view modes.
    + *
    + * @ingroup themeable
    + */
    +#}
    

    I think we don't need this entire comment block in a template that is used for testing purposes only.

thenchev’s picture

Status: Needs work » Needs review
FileSize
5.05 KB
4.13 KB

9.2) Yup, missed that somehow

Also this should fix everything else.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good now I think.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This is a normal bug fix that is not disruptive and therefore permitted during beta. Committed bfdc741 and pushed to 8.0.x. Thanks!

  • alexpott committed bfdc741 on 8.0.x
    Issue #2458601 by Denchev, Berdir: statistics library is not loaded if...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.