Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
We are geolocating using Smart IP. This works. The banner shows in GDPR countries but not in non-GDPR countries. However, in non-GDPR countries the Privacy Settings tab does show. I think it should not, and that this is a bug.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | eu_cookie_compliance-when-geolocation-privacy-settings-tab-still-shows-for-non-gdpr-users-3269129-5.patch | 2.58 KB | svenryen |
Comments
Comment #2
john_b commentedHere is a patch. The approach is to add two new settings to drupalSettings, for popup_eu_only and for geoip_match. If popup_eu_only OR popup_eu_only_js are true, AND geoip_match is not true, return nothing from the JS execute() function.
On D9 I still have problems with caching on a non-Varnish site (reported for D7 at #3261155: We sometimes hit the cache before getting geographic location), and I am inclined to think the non-JS option should be removed. However, that is a separate issue. Provided drupalSettings are returning correct value for geoip_match, my patch is working for me.
Comment #3
svenryen commentedComment #4
svenryen commentedI had a look at the patch. It seems to make the new
geoip_matchsetting part of the page source code (the value will be output along drupalSettings). This means the patch won't work on cached pages, and might drop the banner for some people that should have seen the banner, it might even be inconsistent, depending on the country of the visitor that caused the cache to be created visited from.Also, the array returned from
eu_cookie_compliance_build_data()is cached once per theme/language/domain combination, which adds to the likelihood this patch will break on a site that gets frequent visitors from around the world.I'll see if I can come up with a different approach that does work with cached pages, but I'll wait till the big patch in #3243001: Banner at top is not hiding correctly is merged, otherwise we'll probably have to re-roll the patch for this issue.
Comment #6
svenryen commentedI modified the patch a bit and changed it so that the geoip_match variable isn't cached in the database. Seems to work well now.