From cb53100ec2fd398b6761bca258c9ca1815cceb18 Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Tue, 22 Nov 2016 15:52:59 +0000
Subject: [PATCH] refs #10797 improve default tracking code

---
 CHANGELOG.md                                  |  3 +-
 core/Tracker/TrackerCodeGenerator.php         |  8 +++-
 .../trackingcode/jstrackingcode.controller.js |  3 +-
 plugins/CoreAdminHome/lang/en.json            |  1 +
 .../templates/trackingCodeGenerator.twig      |  8 ++++
 .../Morpheus/templates/javascriptCode.twig    |  4 +-
 plugins/SitesManager/API.php                  |  5 ++-
 .../Tracker/TrackerCodeGeneratorTest.php      | 39 ++++++++++++++++---
 8 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4d7bfd1718..7b650ec250 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -101,7 +101,8 @@ Read more about migrating a plugin from Piwik 2.X to Piwik 3 on our [Migration g
  * `Updater.componentUninstalled` triggered after a component was uninstalled
 * New HTTP Tracking API parameter `pv_id` which accepts a six character unique ID that identifies which actions were performed on a specific page view. Read more about it in the [HTTP Tracking API](https://developer.piwik.org/api-reference/tracking-api);
 * New event `Segment.addSegments` that lets you add segments.
-* New Piwik JavaScript Tracker method `disableHeartBeatTimer()` to disable the heartbeat timer if it was previously enabled. 
+* New Piwik JavaScript Tracker method `disableHeartBeatTimer()` to disable the heartbeat timer if it was previously enabled.
+* The `SitesManager.getJavascriptTag` has a new option `getJavascriptTag` to enable the tracking of users that have JavaScript disabled
 
 ### New features
 * New "Sparklines" visualization that lets you create a widget showing multiple sparklines.
diff --git a/core/Tracker/TrackerCodeGenerator.php b/core/Tracker/TrackerCodeGenerator.php
index ffeea91b6a..ac958e6102 100644
--- a/core/Tracker/TrackerCodeGenerator.php
+++ b/core/Tracker/TrackerCodeGenerator.php
@@ -31,6 +31,7 @@ class TrackerCodeGenerator
      * @param string $customCampaignKeywordParam
      * @param bool $doNotTrack
      * @param bool $disableCookies
+     * @param bool $trackNoScript
      * @return string Javascript code.
      */
     public function generate(
@@ -44,7 +45,8 @@ class TrackerCodeGenerator
         $customCampaignNameQueryParam = null,
         $customCampaignKeywordParam = null,
         $doNotTrack = false,
-        $disableCookies = false
+        $disableCookies = false,
+        $trackNoScript = false
     ) {
         // changes made to this code should be mirrored in plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js var generateJsCode
 
@@ -120,7 +122,8 @@ class TrackerCodeGenerator
             'options'                 => $options,
             'optionsBeforeTrackerUrl' => $optionsBeforeTrackerUrl,
             'protocol'                => '//',
-            'loadAsync'               => true
+            'loadAsync'               => true,
+            'trackNoScript'           => $trackNoScript
         );
         $parameters = compact('mergeSubdomains', 'groupPageTitlesByDomain', 'mergeAliasUrls', 'visitorCustomVariables',
             'pageCustomVariables', 'customCampaignNameQueryParam', 'customCampaignKeywordParam',
@@ -161,6 +164,7 @@ class TrackerCodeGenerator
         $view = new View('@Morpheus/javascriptCode');
         $view->disableCacheBuster();
         $view->loadAsync = $codeImpl['loadAsync'];
+        $view->trackNoScript = $codeImpl['trackNoScript'];
         $jsCode = $view->render();
         $jsCode = htmlentities($jsCode);
 
diff --git a/plugins/CoreAdminHome/angularjs/trackingcode/jstrackingcode.controller.js b/plugins/CoreAdminHome/angularjs/trackingcode/jstrackingcode.controller.js
index a7c8dbaeae..df6befaec8 100644
--- a/plugins/CoreAdminHome/angularjs/trackingcode/jstrackingcode.controller.js
+++ b/plugins/CoreAdminHome/angularjs/trackingcode/jstrackingcode.controller.js
@@ -84,7 +84,8 @@
                 customCampaignNameQueryParam: null,
                 customCampaignKeywordParam: null,
                 doNotTrack: self.doNotTrack ? 1 : 0,
-                disableCookies: self.disableCookies ? 1 : 0
+                disableCookies: self.disableCookies ? 1 : 0,
+                trackNoScript: self.trackNoScript ? 1: 0
             };
 
             if (self.useCustomCampaignParams) {
diff --git a/plugins/CoreAdminHome/lang/en.json b/plugins/CoreAdminHome/lang/en.json
index 46003fe148..a5ee59a280 100644
--- a/plugins/CoreAdminHome/lang/en.json
+++ b/plugins/CoreAdminHome/lang/en.json
@@ -40,6 +40,7 @@
         "JSTracking_MergeSubdomainsDesc": "So if one visitor visits %1$s and %2$s, they will be counted as a unique visitor.",
         "JSTracking_PageCustomVars": "Track a custom variable for each page view",
         "JSTracking_PageCustomVarsDesc": "For example, with variable name \"Category\" and value \"White Papers\".",
+        "JSTracking_TrackNoScript": "Track users with JavaScript disabled",
         "JSTracking_VisitorCustomVars": "Track custom variables for this visitor",
         "JSTracking_VisitorCustomVarsDesc": "For example, with variable name \"Type\" and value \"Customer\".",
         "JSTrackingIntro1": "You can track visitors to your website many different ways. The recommended way to do it is through JavaScript. To use this method you must make sure every webpage of your website has some JavaScript code, which you can generate here.",
diff --git a/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig b/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig
index 5e0acb6169..4d096b3565 100644
--- a/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig
+++ b/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig
@@ -80,6 +80,14 @@
                  value="" inline-help="#jsTrackAllAliasesInlineHelp">
             </div>
 
+            <div piwik-field uicontrol="checkbox" name="javascript-tracking-noscript"
+                 ng-model="jsTrackingCode.trackNoScript"
+                 ng-change="jsTrackingCode.updateTrackingCode()"
+                 disabled="jsTrackingCode.isLoading"
+                 title="{{ 'CoreAdminHome_JSTracking_TrackNoScript'|translate|e('html_attr') }}"
+                 value="" inline-help="">
+            </div>
+
             <h3>{{ 'Mobile_Advanced'|translate }}</h3>
 
             <p>
diff --git a/plugins/Morpheus/templates/javascriptCode.twig b/plugins/Morpheus/templates/javascriptCode.twig
index da29f9b6b8..f54bbf1960 100644
--- a/plugins/Morpheus/templates/javascriptCode.twig
+++ b/plugins/Morpheus/templates/javascriptCode.twig
@@ -1,6 +1,7 @@
 <!-- Piwik -->
 <script type="text/javascript">
   var _paq = _paq || [];
+  // tracker methods like "setCustomDimension" should be called before "trackPageView"
 {$options}  _paq.push(['trackPageView']);
   _paq.push(['enableLinkTracking']);
   (function() {
@@ -14,5 +15,6 @@
 </script>
 {% if not loadAsync %}<script type='text/javascript' src="{$protocol}{$piwikUrl}/piwik.js"></script>
 {% endif %}
-<noscript><p><img src="{$protocol}{$piwikUrl}/piwik.php?idsite={$idSite}" style="border:0;" alt="" /></p></noscript>
+{% if trackNoScript %}<noscript><p><img src="{$protocol}{$piwikUrl}/piwik.php?idsite={$idSite}&rec=1" style="border:0;" alt="" /></p></noscript>
+{% endif %}
 <!-- End Piwik Code -->
diff --git a/plugins/SitesManager/API.php b/plugins/SitesManager/API.php
index a28093ed98..7c55be9fe1 100644
--- a/plugins/SitesManager/API.php
+++ b/plugins/SitesManager/API.php
@@ -95,12 +95,13 @@ class API extends \Piwik\Plugin\API
      * @param bool $customCampaignKeywordParam
      * @param bool $doNotTrack
      * @param bool $disableCookies
+     * @param bool $trackNoScript
      * @return string The Javascript tag ready to be included on the HTML pages
      */
     public function getJavascriptTag($idSite, $piwikUrl = '', $mergeSubdomains = false, $groupPageTitlesByDomain = false,
                                      $mergeAliasUrls = false, $visitorCustomVariables = false, $pageCustomVariables = false,
                                      $customCampaignNameQueryParam = false, $customCampaignKeywordParam = false,
-                                     $doNotTrack = false, $disableCookies = false)
+                                     $doNotTrack = false, $disableCookies = false, $`` = false)
     {
         Piwik::checkUserHasViewAccess($idSite);
 
@@ -120,7 +121,7 @@ class API extends \Piwik\Plugin\API
         $code = $generator->generate($idSite, $piwikUrl, $mergeSubdomains, $groupPageTitlesByDomain,
                                      $mergeAliasUrls, $visitorCustomVariables, $pageCustomVariables,
                                      $customCampaignNameQueryParam, $customCampaignKeywordParam,
-                                     $doNotTrack, $disableCookies);
+                                     $doNotTrack, $disableCookies, $trackNoScript);
         $code = str_replace(array('<br>', '<br />', '<br/>'), '', $code);
         return $code;
     }
diff --git a/tests/PHPUnit/Integration/Tracker/TrackerCodeGeneratorTest.php b/tests/PHPUnit/Integration/Tracker/TrackerCodeGeneratorTest.php
index a8787cc5ee..f596684d92 100644
--- a/tests/PHPUnit/Integration/Tracker/TrackerCodeGeneratorTest.php
+++ b/tests/PHPUnit/Integration/Tracker/TrackerCodeGeneratorTest.php
@@ -26,11 +26,12 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
             $visitorCustomVariables = array(array("name", "value"), array("name 2", "value 2")),
             $pageCustomVariables = array(array("page cvar", "page cvar value")),
             $customCampaignNameQueryParam = "campaignKey", $customCampaignKeywordParam = "keywordKey",
-            $doNotTrack = true);
+            $doNotTrack = true, $disableCookies = false, $trackNoScript = true);
 
         $expected = "&lt;!-- Piwik --&gt;
 &lt;script type=&quot;text/javascript&quot;&gt;
   var _paq = _paq || [];
+  // tracker methods like &quot;setCustomDimension&quot; should be called before &quot;trackPageView&quot;
   _paq.push([\"setDocumentTitle\", document.domain + \"/\" + document.title]);
   // you can set up to 5 custom variables for each visitor
   _paq.push([\"setCustomVariable\", 1, \"name\", \"value\", \"visit\"]);
@@ -50,7 +51,33 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
     g.type='text/javascript'; g.async=true; g.defer=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s);
   })();
 &lt;/script&gt;
-&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;//localhost/piwik/piwik.php?idsite=1&quot; style=&quot;border:0;&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
+&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;//localhost/piwik/piwik.php?idsite=1&amp;rec=1&quot; style=&quot;border:0;&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
+&lt;!-- End Piwik Code --&gt;
+";
+
+        $this->assertEquals($expected, $jsTag);
+    }
+
+    public function testJavascriptTrackingCode_noScriptTrackingDisabled_defaultTrackingCode()
+    {
+        $generator = new TrackerCodeGenerator();
+
+        $jsTag = $generator->generate($idSite = 1, $piwikUrl = 'http://localhost/piwik');
+
+        $expected = "&lt;!-- Piwik --&gt;
+&lt;script type=&quot;text/javascript&quot;&gt;
+  var _paq = _paq || [];
+  // tracker methods like &quot;setCustomDimension&quot; should be called before &quot;trackPageView&quot;
+  _paq.push(['trackPageView']);
+  _paq.push(['enableLinkTracking']);
+  (function() {
+    var u=&quot;//localhost/piwik/&quot;;
+    _paq.push(['setTrackerUrl', u+'piwik.php']);
+    _paq.push(['setSiteId', '1']);
+    var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0];
+    g.type='text/javascript'; g.async=true; g.defer=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s);
+  })();
+&lt;/script&gt;
 &lt;!-- End Piwik Code --&gt;
 ";
 
@@ -78,6 +105,7 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
         $expected = "&lt;!-- Piwik --&gt;
 &lt;script type=&quot;text/javascript&quot;&gt;
   var _paq = _paq || [];
+  // tracker methods like &quot;setCustomDimension&quot; should be called before &quot;trackPageView&quot;
   _paq.push([\"setDocumentTitle\", document.domain + \"/\" + document.title]);
   // you can set up to 5 custom variables for each visitor
   _paq.push([\"setCustomVariable\", 1, \"name\", \"value\", \"visit\"]);
@@ -97,7 +125,6 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
     g.type='text/javascript'; g.async=true; g.defer=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s);
   })();
 &lt;/script&gt;
-&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;https://localhost/piwik/piwik.php?idsite=1&quot; style=&quot;border:0;&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
 &lt;!-- End Piwik Code --&gt;
 ";
 
@@ -125,6 +152,7 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
         $expected = "&lt;!-- Piwik --&gt;
 &lt;script type=&quot;text/javascript&quot;&gt;
   var _paq = _paq || [];
+  // tracker methods like &quot;setCustomDimension&quot; should be called before &quot;trackPageView&quot;
   _paq.push([\"setDocumentTitle\", document.domain + \"/\" + document.title]);
   // you can set up to 5 custom variables for each visitor
   _paq.push([\"setCustomVariable\", 1, \"name\", \"value\", \"visit\"]);
@@ -145,7 +173,6 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
     g.type='text/javascript'; g.async=true; g.defer=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s);
   })();
 &lt;/script&gt;
-&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;//localhost/piwik/piwik.php?idsite=1&quot; style=&quot;border:0;&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
 &lt;!-- End Piwik Code --&gt;
 ";
 
@@ -169,6 +196,7 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
         $expected = "&lt;!-- Piwik --&gt;
 &lt;script type=&quot;text/javascript&quot;&gt;
   var _paq = _paq || [];
+  // tracker methods like &quot;setCustomDimension&quot; should be called before &quot;trackPageView&quot;
   _paq.push([\"setDocumentTitle\", document.domain + \"/\" + document.title]);
   _paq.push(['trackPageView']);
   _paq.push(['enableLinkTracking']);
@@ -180,7 +208,6 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
   })();
 &lt;/script&gt;
 &lt;script type='text/javascript' src=&quot;//localhost/piwik/piwik.js&quot;&gt;&lt;/script&gt;
-&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;//localhost/piwik/piwik.php?idsite=1&quot; style=&quot;border:0;&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
 &lt;!-- End Piwik Code --&gt;
 ";
 
@@ -206,6 +233,7 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
         $expected = '&lt;!-- Piwik --&gt;
 &lt;script type=&quot;text/javascript&quot;&gt;
   var _paq = _paq || [];
+  // tracker methods like &quot;setCustomDimension&quot; should be called before &quot;trackPageView&quot;
   _paq.push(["setDocumentTitle", document.domain + "/" + document.title]);
   // you can set up to 5 custom variables for each visitor
   _paq.push(["setCustomVariable", 1, "abc\"def", "abc\"def", "visit"]);
@@ -223,7 +251,6 @@ class TrackerCodeGeneratorTest extends IntegrationTestCase
     g.type=\'text/javascript\'; g.async=true; g.defer=true; g.src=u+\'piwik.js\'; s.parentNode.insertBefore(g,s);
   })();
 &lt;/script&gt;
-&lt;noscript&gt;&lt;p&gt;&lt;img src=&quot;//abc&quot;def/piwik.php?idsite=1&quot; style=&quot;border:0;&quot; alt=&quot;&quot; /&gt;&lt;/p&gt;&lt;/noscript&gt;
 &lt;!-- End Piwik Code --&gt;
 ';
 
-- 
GitLab