Skip to content
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

Fix "disableCookies" does not work in some browers / in some cases #8056

Merged
merged 1 commit into from Jun 8, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jun 8, 2015

refs #7494
I solved three different things here:

  • Make sure disableCookies is executed before any other method that potentially causes a cookie to be set. To be concrete if setSiteId was executed before disableCookies, it might leave a cookie.
  • We make sure to deleteCookies() directly when disableCookies() is called, if possible. Otherwise it is possible that we create a cookie and we would never delete it if getRequest() is not executed
  • We delete a cookie only if one is actually set. A user reported to have a cookie with value=''. We set value='' when deleting a cookie. This way we make sure to not have potential cookies with value=''

@tsteur tsteur added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jun 8, 2015
@tsteur tsteur added this to the 2.14.0 milestone Jun 8, 2015
mattab pushed a commit that referenced this pull request Jun 8, 2015
Fix "disableCookies" does not work in some browers / in some cases
@mattab mattab merged commit 97d275e into master Jun 8, 2015
@mattab
Copy link
Member

mattab commented Jun 8, 2015

Looks good!

@mnapoli mnapoli deleted the 7494 branch June 8, 2015 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants