Skip to content

Commit

Permalink
Refs #1845
Browse files Browse the repository at this point in the history
 * the tests/js is now passing following Benaka's commit, sweeet
 * Adding setsiteId to the "special case" so that setSiteId can be called after trackpageView and it still works
 * Adding pathname in the generated JS
 * HTML entity the & in the IMG src
  • Loading branch information
mattab committed Feb 26, 2013
1 parent b562ded commit c405a15
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 22 deletions.
4 changes: 2 additions & 2 deletions core/Tracker/javascriptCode.tpl
@@ -1,16 +1,16 @@
<!-- Piwik -->
<script type="text/javascript">
var _paq = _paq || [];
_paq.push(['setSiteId', {$idSite}]);
_paq.push(['trackPageView']);
_paq.push(['enableLinkTracking']);
(function() {
var u=(("https:" == document.location.protocol) ? "https" : "http") + "://{$piwikUrl}/";
_paq.push(['setTrackerUrl', u+'piwik.php']);
_paq.push(['setSiteId', {$idSite}]);
var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.type='text/javascript';
g.defer=true; g.async=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s);
})();
</script>
<noscript><p><img src="http://{$piwikUrl}piwik.php?idsite={$idSite}" style="border:0" alt="" /></p></noscript>
<!-- End Piwik Code -->
10 changes: 6 additions & 4 deletions js/piwik.js
Expand Up @@ -2523,8 +2523,9 @@ var

/**
* Strip hash tag (or anchor) from URL
* TODO: Remove as it is now part of core #3232
* Note: this can be done in the Piwik>Settings>Websites on a per-website basis
*
* @deprecated
* @param bool enableFilter
*/
discardHashTag: function (enableFilter) {
Expand Down Expand Up @@ -2882,10 +2883,11 @@ var
Date.prototype.getTimeAlias = Date.prototype.getTime;

asyncTracker = new Tracker();
// find the call to setTrackerUrl (if any) and call it first

// find the call to setTrackerUrl or setSiteid (if any) and call them first
for (i = 0; i < _paq.length; i++) {
if (_paq[i][0] == 'setTrackerUrl') {
if (_paq[i][0] === 'setTrackerUrl'
|| _paq[i][0] === 'setSiteId') {
apply(_paq[i]);
delete _paq[i];
}
Expand Down
4 changes: 2 additions & 2 deletions lang/en.php
Expand Up @@ -480,7 +480,7 @@
'CoreAdminHome_JSTracking_MergeSubdomains' => 'Track visitors across all subdomains of',
'CoreAdminHome_JSTracking_MergeSubdomainsDesc' => 'So visitors to %1$s will not be treated as new if they visit %2$s for the first time.',
'CoreAdminHome_JSTracking_MergeAliases' => 'Track visitors across all alias URLs of',
'CoreAdminHome_JSTracking_MergeAliasesDesc' => 'So visitors to the main URL will not be treated as new if they visit an alias URL for the first time.',
'CoreAdminHome_JSTracking_MergeAliasesDesc' => 'So clicks on links to Alias URLs (eg. %s) will not be counted as "Outlink".',
'CoreAdminHome_JSTracking_GroupPageTitlesByDomain' => 'Prepend the site domain to the page title when tracking',
'CoreAdminHome_JSTracking_GroupPageTitlesByDomainDesc' => 'So if someone visits the \'About\' page on blog.example.com it will be recorded as \'blog / About\'. This is the easiest way to get an overview of your traffic by sub-domain.',
'CoreAdminHome_JSTracking_VisitorCustomVars' => 'Track custom variables for this visitor',
Expand Down Expand Up @@ -617,7 +617,7 @@
'CoreHome_PeriodWeeks' => 'weeks',
'CoreHome_PeriodMonths' => 'months',
'CoreHome_PeriodYears' => 'years',
'CoreHome_YearShort_js' => 'yr',
'General_YearShort_js' => 'yr',

This comment has been minimized.

Copy link
@halfdan

halfdan Feb 26, 2013

Member

Was that change intentional? @sgiehl will have to update the translated files since tests are currently failing for some languages that still contain the CoreHome_YearShort_js entry.

This comment has been minimized.

Copy link
@sgiehl

sgiehl Feb 26, 2013

Member

I've renamed that key on OTrance. Running the script should update it correctly then. But it should be enough to simply change those files for now to have running tests...

This comment has been minimized.

Copy link
@halfdan

halfdan Feb 26, 2013

Member

Ok, I'll update.

This comment has been minimized.

Copy link
@halfdan

halfdan Feb 26, 2013

Member

Actually - no. I am not sure that this update was intentional. There are numerous other CoreHome_$date_js variables in there, it feels inconsistent of we change only one.

@mattab Can you confirm?

This comment has been minimized.

Copy link
@mattab

mattab Feb 26, 2013

Author Member

it's a bit hacky, but I wouldnt want to rename all other strings for sake of consistency as it would force translators to re-translate. So, IMHO it's OK

This comment has been minimized.

Copy link
@halfdan

halfdan Feb 26, 2013

Member

I think @sgiehl can rename keys for translations on OTrack - so that should be possible without having people re-translate anything.

'CoreHome_DaySu_js' => 'Su',
'CoreHome_DayMo_js' => 'Mo',
'CoreHome_DayTu_js' => 'Tu',
Expand Down
11 changes: 6 additions & 5 deletions plugins/CoreAdminHome/templates/jsTrackingGenerator.js
Expand Up @@ -174,8 +174,7 @@ $(document).ready(function() {
// generate JS
var result = '<!-- Piwik -->\n\
<script type="text/javascript">\n\
var _paq = _paq || [];\n\
_paq.push(["setSiteId", ' + JSON.stringify(idSite) + ']);\n';
var _paq = _paq || [];\n';

if (groupPageTitlesByDomain)
{
Expand Down Expand Up @@ -231,6 +230,7 @@ $(document).ready(function() {
(function() {\n\
var u=(("https:" == document.location.protocol) ? "https" : "http") + "://' + piwikHost + '/";\n\
_paq.push(["setTrackerUrl", u+"piwik.php"]);\n\

This comment has been minimized.

Copy link
@robocoder

robocoder Feb 26, 2013

Contributor

Are you sure about this? I thought we were going to serve js/ instead of piwik.php.

_paq.push(["setSiteId", ' + JSON.stringify(idSite) + ']);\n\
var d=document, g=d.createElement("script"), s=d.getElementsByTagName("script")[0]; g.type="text/javascript";\n\
g.defer=true; g.async=true; g.src=u+"piwik.js"; s.parentNode.insertBefore(g,s);\n\

This comment has been minimized.

Copy link
@robocoder

robocoder Feb 26, 2013

Contributor

Can we serve js/ instead of piwik.js?

This comment has been minimized.

Copy link
@mattab

mattab Feb 26, 2013

Author Member

We could but I didn't want to change this at the same time as all others, so we can do it in a next release

})();\n\
Expand All @@ -245,7 +245,7 @@ $(document).ready(function() {
{
// get data ( (("https:" == document.location.protocol)?"https://' + piwikHost + '":"http://' + piwikHost + '") )
var idSite = $('#image-tracker-website .custom_select_main_link').attr('siteid'),
piwikURL = "https:" == document.location.protocol ? "https://" + piwikHost : "http://" + piwikHost,
piwikURL = ("https:" == document.location.protocol ? "https://" + piwikHost : "http://" + piwikHost) + document.location.pathname,
actionName = $('#image-tracker-action-name').val(),
idGoal = null,
revenue = null;
Expand Down Expand Up @@ -280,9 +280,10 @@ $(document).ready(function() {
}

var result = '<!-- Piwik Image Tracker -->\n\
<img src="' + piwikURL + '/?' + $.param(params) + '" style="border:0" alt="" />\n\
<img src="' + piwikURL + '?' + $.param(params) + '" style="border:0" alt="" />\n\
<!-- End Piwik -->';


result = result.replace("&", "&amp;", "g");
$('#image-tracking-link textarea').val(result);
};

Expand Down
2 changes: 1 addition & 1 deletion plugins/CoreAdminHome/templates/jsTrackingGenerator.tpl
Expand Up @@ -52,7 +52,7 @@
<label for="javascript-tracking-all-aliases">{'CoreAdminHome_JSTracking_MergeAliases'|translate} <span class='current-site-name'>{$defaultReportSiteName}</span></label>

<div class="small-form-description">
{'CoreAdminHome_JSTracking_MergeAliasesDesc'|translate}
{'CoreAdminHome_JSTracking_MergeAliasesDesc'|translate:'x.domain.com'}
</div>
</div>

Expand Down
2 changes: 1 addition & 1 deletion plugins/CoreHome/templates/donate.js
Expand Up @@ -27,7 +27,7 @@ $(document).ready(function() {
var setSmileyFaceAndAmount = function(slider, pos)
{
// set text yearly amount
$('.slider-donate-amount', slider).text('$' + donateAmounts[pos] + '/' + _pk_translate('CoreHome_YearShort_js'));
$('.slider-donate-amount', slider).text('$' + donateAmounts[pos] + '/' + _pk_translate('General_YearShort_js'));

// set the right smiley face
$('.slider-smiley-face').attr('src', 'themes/default/images/smileyprog_' + pos + '.png');
Expand Down
2 changes: 1 addition & 1 deletion plugins/CoreHome/templates/donate.tpl
Expand Up @@ -23,7 +23,7 @@
<div class="slider-position"></div>
</div>
<div style="display:inline-block">
<div class="slider-donate-amount">$30/{'CoreHome_YearShort_js'|translate}</div>
<div class="slider-donate-amount">$30/{'General_YearShort_js'|translate}</div>

<img class="slider-smiley-face" width="40" height="40" src="themes/default/images/smileyprog_1.png"/>
</div>
Expand Down
2 changes: 1 addition & 1 deletion plugins/Feedback/templates/feedback.js
Expand Up @@ -23,7 +23,7 @@ $(function() {
title: feedback.html(),
modal: true,
width: 650,
position: ['center', 90],
position: ['center', 40],
resizable: false,
autoOpen: false
});
Expand Down
8 changes: 3 additions & 5 deletions tests/javascript/index.php
Expand Up @@ -31,9 +31,9 @@ function getToken() {
var _paq = _paq || [];
function testCallingTrackPageViewBeforeSetTrackerUrlWorks() {
_paq.push(["setSiteId", 1]);
_paq.push(["setCustomData", { "token" : getToken() }]);
_paq.push(["trackPageView", "Asynchronous Tracker ONE"]);
_paq.push(["setSiteId", 1]);
_paq.push(["setTrackerUrl", "piwik.php"]);
}
Expand Down Expand Up @@ -828,7 +828,7 @@ function PiwikTest() {
});
test("tracking", function() {
expect(79);
expect(80);
/*
* Prevent Opera and HtmlUnit from performing the default action (i.e., load the href URL)
Expand Down Expand Up @@ -1062,9 +1062,7 @@ function wait(msecs)
xhr.open("GET", "piwik.php?requests=" + getToken(), false);
xhr.send(null);
results = xhr.responseText;
equal( (/<span\>([0-9]+)\<\/span\>/.exec(results))[1], "26", "count tracking events" );
alert(results);
equal( (/<span\>([0-9]+)\<\/span\>/.exec(results))[1], "25", "count tracking events" );
// tracking requests
ok( /PiwikTest/.test( results ), "trackPageView(), setDocumentTitle()" );
Expand Down

2 comments on commit c405a15

@robocoder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this is complete, please commit/push the minified /piwik.js.

@mattab
Copy link
Member Author

@mattab mattab commented on c405a15 Feb 26, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, thanks for the reminder (done)

Please sign in to comment.