New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JS Tracker: new setCrossDomainLinkingTimeout function useful to set a higher timeout for links #11828
Conversation
… higher timeout for links and increasing the default to 120 seconds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess we need to update the documentation as well and mention that new method?
js/piwik.js
Outdated
|
||
if (timeoutInSeconds <= 0) { | ||
return true; | ||
} | ||
if (currentTimestampInSeconds >= timestampInUrl | ||
&& currentTimestampInSeconds <= (timestampInUrl + timeoutInSeconds)) { | ||
// we only use visitorId if it was generated max 45 seconds ago |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment is outdated, as it's not 45 seconds, but configVisitorIdUrlParameterTimeoutInSeconds
instead
js/piwik.js
Outdated
@@ -3089,6 +3089,9 @@ if (typeof window.Piwik !== 'object') { | |||
// VDI = visitor device identifier | |||
configVisitorIdUrlParameter = 'pk_vid', | |||
|
|||
// Cross domain linking, the visitor ID is transmitted only in the 180 seconds following the click. | |||
configVisitorIdUrlParameterTimeoutInSeconds = 120, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment says it's 180 seconds, but value actually is 120?
# Conflicts: # js/piwik.min.js # piwik.js
Code looks ok now. Let's wait for the tests to pass... |
@mattab Seems some more changes are required here. See https://travis-ci.org/piwik/piwik/jobs/254631350#L498-L550 |
tests/javascript/index.php
Outdated
@@ -2872,12 +2873,13 @@ function makeReplaceHrefForCrossDomainLink(url) { | |||
strictEqual(tracker.getTrackerUrl(), makeReplaceHrefForCrossDomainLink(tracker.getTrackerUrl()), 'replaceHrefForCrossDomainLink, should not change URL if href is a tracker URL'); | |||
|
|||
tracker.setUserId('test'); | |||
debugger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattab Any reason for adding this?
and increasing the default to 120 seconds