Async node hit counter data collection is broken when path-based language negotiation is used

This issue depends on #2172859: Statistics JS and settings are not included on node pages, because the Statistics module is completely broken, regardless of the multilingual bug.

Here's a patch for D7, with tests. D8 is still dependent on #2172859: Statistics JS and settings are not included on node pages, but the relevant test modifications are below:

1. +++ b/modules/statistics/statistics.module
   @@ -123,7 +123,7 @@ function statistics_node_view($node, $view_mode) {
          );
   -      $settings = array('data' => array('nid' => $node->nid), 'url' => url(drupal_get_path('module', 'statistics') . '/statistics.php'));
   +      $settings = array('data' => array('nid' => $node->nid), 'url' => base_path() . drupal_get_path('module', 'statistics') . '/statistics.php');
          $node->content['#attached']['js'][] = array(
   

   The actual fix, very straightforward.

2. +++ b/modules/statistics/statistics.test
   @@ -61,7 +61,7 @@ class StatisticsLoggingTestCase extends DrupalWebTestCase {
      function setUp() {
   -    parent::setUp('statistics');
   +    parent::setUp('statistics', 'locale');
    
        $this->auth_user = $this->drupalCreateUser(array('access content', 'create page content', 'edit own page content'));
    
   @@ -75,9 +75,25 @@ class StatisticsLoggingTestCase extends DrupalWebTestCase {
   
   @@ -75,9 +75,25 @@ class StatisticsLoggingTestCase extends DrupalWebTestCase {
        variable_set('statistics_enable_access_log', 1);
        variable_set('statistics_count_content_views', 1);
    
   +    // User to add and remove language.
   +    $admin_user = $this->drupalCreateUser(array('administer languages', 'access administration pages'));
   +
   +    // Add custom language.
   +    $this->drupalLogin($admin_user);
   +    $this->language = array(
   +      'langcode' => 'xx',
   +      'name' => $this->randomName(16),
   +      'native' => $this->randomName(16),
   +      'prefix' => 'xx',
   +      'direction' => '0',
   +    );
   +    $this->drupalPost('admin/config/regional/language/add', $this->language, t('Add custom language'));
   +    $this->drupalPost('admin/config/regional/language/configure', array('language[enabled][locale-url]' => 1), t('Save settings'));
   +    $this->drupalLogout();
   +
        // Clear the logs.
   -    db_truncate('accesslog');
   -    db_truncate('node_counter');
   +    db_truncate('accesslog')->execute();
   +    db_truncate('node_counter')->execute();
      }
   

   Modifications necessary to enable path prefix based language negotiation.

3. +++ b/modules/statistics/statistics.test
   @@ -158,6 +174,34 @@ class StatisticsLoggingTestCase extends DrupalWebTestCase {
   +
   +    // Verify the same when loading the site in a non-default language.
   +    $this->drupalGet($this->language['prefix'] . '/node/' . $this->node->nid);
   +    $this->assertRaw($expected_script, 'Statistics JS found on valid node page in non-default language.');
   +    $this->assertRaw($expected_settings, 'Statistics settings found on valid node page in non-default language.');
   +    variable_set('statistics_count_content_views_ajax', 0);
   

   The only additional tests needed for D8. The rest are backports of D8 tests for D7.

Patch attached. As illustrated above, the issue is that url() takes path-based language negotiation into account when generating aliases, which we don't want (because /core/modules/statistics/statistics.js will always be at the Drupal root, not at a language path-prefixed location).

In addition to the fix and tests for that change, I'm being more explicit in the check for the script/settings on-page by:

• Encoding the expected settings JSON rather than faking it in plain-text,
• Searching for a regex that includes <script src="[/path/to/statistics.js]">

Also tagging D8MI to put it on the multilingual initiative's radar.

3: drupal8-ajax_node_counter_lang_path_prefix-2172871-3.tests-only.patch queued for re-testing.

3: drupal8-ajax_node_counter_lang_path_prefix-2172871-3.patch queued for re-testing.

3: drupal8-ajax_node_counter_lang_path_prefix-2172871-3.patch queued for re-testing.

Rebasing #3...

Ah. $base_url includes the path. Just need to use $base_root in the test.

15: drupal8-ajax_node_counter_lang_path_prefix-2172871-15.patch queued for re-testing.

15: drupal8-ajax_node_counter_lang_path_prefix-2172871-15.patch queued for re-testing.

15: drupal8-ajax_node_counter_lang_path_prefix-2172871-15.patch queued for re-testing.

^^ Head was broken. Looking good now.

Re-roll of #15.

Updating displayed patches to only those that are currently relevant.

Per #29, this needs work... Will be back shortly with two patches:

• Tests only (based on #25), which should pass
• Tests + #29, which should fail

#34 with regression from #29.

Thanks @effulgentsia.

I had to add a new #cache key of cache_context.language to the build array to get the drupalSettings to actually reflect the language. Without it, the drupalSettings attached to the node were being stored in render cache globally for all languages and thus, hook_node_view was never being triggered after initial load.

That behavior seems like a bug to me, but I don't really have time personally to investigate whether or not that's true, nor whether or not an issue already exists for that bug.

In any event, here's a patch with a working test that should pass. I'll follow-up with an updated regression.

And here's #39 + a tweaked regression based on #37. It should fail. #39 should be reviewed/committed.

Hiding regression from issue summary...

See #39.

Looks good! #45 is addressed.

Reverting name change so this makes more sense for D7.

Here's #2 verbatim, but renamed so that it will be tested.

Should probably be updated to use DrupalWebTestCase::drupalGetSettings(), per #45.