Don't set any cookies when no consent is given #16173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
build js |
tsteur
added
not-in-changelog
|
build js |
tsteur
commented
Jul 3, 2020
| * | ||
| * It will also automatically enable cookies if they were disabled previously. | ||
| * | ||
| * @param bool [enableCookies=true] Internal parameter. Defines whether cookies should be enabled or not. |
There was a problem hiding this comment.
we could generally expose this parameter as a support official parameter. However, then we'd also need to add this to rememberConsentGiven () method and the problem is that we'd then need to persist in yet another cookie whether rememberConsentGiven () should always enable cookies or not. Easier to consider it an internal parameter for now
jonasgrilleres
pushed a commit
to 1024pix/pix-analytics
that referenced
this pull request
Sep 22, 2020
* Better detection for cookies for browser plugins report * rebuilt piwik.js * improve comment * Add method to enable cookies * rebuilt piwik.js * fix test * no longer include cookie in fingerprint * only ignore cookies in fingerprint for IE * fix tests * fix test * tweak enablecookies * rebuilt piwik.js * send tracking request if needed when enable cookies * rebuilt piwik.js * tweak code * update docs * rebuilt piwik.js * Update Visit.php * fix tests * Don't set cookies unless consent given when consent is required * fix test * rebuilt piwik.js * add tests * add missing function * rebuilt piwik.js * fix jslint test
jbuget
pushed a commit
to 1024pix/pix-analytics
that referenced
this pull request
Sep 26, 2020
* Better detection for cookies for browser plugins report * rebuilt piwik.js * improve comment * Add method to enable cookies * rebuilt piwik.js * fix test * no longer include cookie in fingerprint * only ignore cookies in fingerprint for IE * fix tests * fix test * tweak enablecookies * rebuilt piwik.js * send tracking request if needed when enable cookies * rebuilt piwik.js * tweak code * update docs * rebuilt piwik.js * Update Visit.php * fix tests * Don't set cookies unless consent given when consent is required * fix test * rebuilt piwik.js * add tests * add missing function * rebuilt piwik.js * fix jslint test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #13246
this PR is based on https://github.com/matomo-org/matomo/pull/16113/files and should be reviewed and merged soon. The change is this PR is a lot smaller.
This applies when someone calls
requireConsent. In the past we would have still set a visitor cookie even though if no further call to egsetConsentGivenorrememberConsentGivenfollowed.In the past, the visitorId cookie was set when calling
setSiteId. I'm not sure why this was the case but it shouldn't be needed to set it so early in the configuration process of a new tracker. In fact we're setting the session and attribution cookie already only when callinggetRequest('...')(eggetRequest('action_name=foo')) so it should be fine to set the visitor cookie also only oncegetRequestis being called which this PR now does.It does change the
nowTswhich is stored in the visitor cookie but that shouldn't really change anything cause mostly a tracking request follows directly anyway. And if in past there were cookies disabled etc then we wouldn't have been able to set this anyway.It's adding a new tracker method
areCookiesEnabledbut not adding it to the changelog as it doesn't actually do anything and you can't really use it with _paq.push but will document it once merged.