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

Add the Osano Cookie Consent cookie to the cookie deny list #1763

Merged
merged 2 commits into from
Jun 12, 2020

Conversation

Mynyx
Copy link
Contributor

@Mynyx Mynyx commented May 21, 2020

@andreheeke
Copy link

Shouldn't we rather find a general solution how to manually add more cookies to the blacklist? Otherwise, everyone will soon have the idea to push their tool into the Contao repository.

@fritzmg
Copy link
Contributor

fritzmg commented May 21, 2020

@fritzmg
Copy link
Contributor

fritzmg commented May 21, 2020

Also the Osano Cookieconsent script should switch to localStorage, since it is purely client side - or at least only fall back to using Cookies for legacy browsers. There was a PR for that a while ago, but it got rejected: osano/cookieconsent#97

@Mynyx
Copy link
Contributor Author

Mynyx commented May 21, 2020

Otherwise, everyone will soon have the idea to push their tool into the Contao repository.

Osano Cookie Consent is not just any tool, it is one of the most popular, according to their own statement the most popular.

Toflar
Toflar previously approved these changes May 22, 2020
@Toflar Toflar added the bug label May 22, 2020
@Toflar Toflar added this to the 4.9 milestone May 22, 2020
@Toflar
Copy link
Member

Toflar commented May 22, 2020

Shouldn't we rather find a general solution how to manually add more cookies to the blacklist? Otherwise, everyone will soon have the idea to push their tool into the Contao repository.

The idea of the blacklist is to increase the possibility of cache hits without forcing every single Contao user to deep-dive into the topic of caching. We just want to generate good results by default.
If you know what you're doing and you understand the concept, you can specify the whitelist. This then increases the cache hits to the maximum :)

@leofeyer
Copy link
Member

I fully agree with @Toflar. We should try to add as many commonly used tools as possible to the blacklist, so Contao works nicely with them out of the box.

aschempp
aschempp previously approved these changes May 27, 2020
@leofeyer leofeyer changed the title Add the Osano Cookie Consent cookie to the cookie blacklist Add the Osano Cookie Consent cookie to the cookie deny list Jun 9, 2020
@leofeyer leofeyer dismissed stale reviews from aschempp and Toflar via bf26bce June 11, 2020 15:09
@leofeyer leofeyer requested review from Toflar and aschempp June 11, 2020 15:10
@leofeyer leofeyer merged commit 9817202 into contao:4.9 Jun 12, 2020
@leofeyer
Copy link
Member

Thank you @Mynyx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants