There are 3 issues that prevent geoip check from correct work:

  • In eu_cookie_compliance_page_attachments() if the check is done, $geoip_match is always TRUE being a non-empty array.
  • CheckIfEuCountryJs::routes() doesn't provide the JS check path if the match is checked via geoip module or hook_eu_cookie_compliance_geoip_match_alter().
  • When the JS check is executed, hook_eu_cookie_compliance_geoip_match_alter() is not invoked at all.
CommentFileSizeAuthor
#3 eu_cookie_compliance-geo-ip-match-is-handled-wrong-2968700-3-8.x.patch2.38 KBsvenryen
#3 eu_cookie_compliance-geo-ip-match-is-handled-wrong-2968700-3-7.x.patch1.57 KBsvenryen
#2 eu_cookie_compliance-geoip-match-2968700-2-D8.patch1.76 KBalex.bukach

Comments

Alex Bukach created an issue. See original summary.

alex.bukach’s picture

alex.bukach commented
Assigned: alex.bukach » Unassigned
Status: Active » Needs review
StatusFileSize
new eu_cookie_compliance-geoip-match-2968700-2-D8.patch1.76 KB
svenryen’s picture

svenryen commented
StatusFileSize
new eu_cookie_compliance-geo-ip-match-is-handled-wrong-2968700-3-7.x.patch1.57 KB
new eu_cookie_compliance-geo-ip-match-is-handled-wrong-2968700-3-8.x.patch2.38 KB

@Alex - I'm a bit reluctant to change the parameter that's passed to the alter hook. The reason being people may have implemented the hook, and from what I understand, passing a boolean rather than the array to the hook will break their hooks. (Correct me if I'm wrong or missing something.)

Instead what I think will work even better is if we change the check for `$geoip_match` line 124 to check against `$geoip_match['in_eu']`.
Would that work for you? I realize if you used your patch to create an alter hook, you would have to update your hook or patch the module. Let me know what you think.

I'm attaching patches for 7.x and 8.x and committing these to -dev.

svenryen’s picture

svenryen commented
Status: Needs review » Fixed

  • svenryen committed aab5c4f on 8.x-1.x 
    Issue #2968700 by svenryen, Alex Bukach: Geo IP match is handled wrong

  • svenryen committed a08c46f on 7.x-1.x 
    Issue #2968700 by svenryen, Alex Bukach: Geo IP match is handled wrong

  • svenryen committed a08c46f on 7.x-2.x 
    Issue #2968700 by svenryen, Alex Bukach: Geo IP match is handled wrong
  • svenryen committed a44ca28 on 7.x-2.x
    Merge branch '7.x-1.x' into 7.x-2.x

* 7.x-1.x:
Issue #2965976 by...
alex.bukach’s picture

alex.bukach commented

@svenryen I'm fine with your change, thanks!

Actually your approach is what I wanted to implement initially, but then thought about other developers who already had that hook implemented and decided to keep back compatibility.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.