adrian Posted August 23, 2018 Author Share Posted August 23, 2018 4 hours ago, ceberlin said: on localhost, I get a warning: Notice: Undefined index: countryCode in /Users/.../modules/CookieManagementBanner/CookieManagementBanner.module on line 42 Hi @ceberlin - that's strange - for me on localhost the countryCode index is defined, but it is set to a blank string. Assuming you have Tracy installed, can you please replace the lines starting at 37 with this block: if(!$this->wire('session')->userFromEu) { $http = new WireHttp(); bd($this->get_user_ip_addr()); $userLocation = $http->getJSON('https://extreme-ip-lookup.com/json/'.$this->get_user_ip_addr()); bd($userLocation); // test EU IP address Then in the PW Info panel, click the "Clear Session & Cookies" link and then post the output of those two bd() calls. This is what I get: Link to comment Share on other sites More sharing options...
ceberlin Posted August 24, 2018 Share Posted August 24, 2018 Maybe there should be a note in the plugin settings that using Checking EU is sending personal IP data to extreme-ip-lookup.com (USA?) and restcountries.eu which might be breaking GDPR law (Server location?), when checking that option. [OFFTOPIC] Tracy: I only use parts of it, especially using bd() causes an Uncaught Error: Call to undefined function bd - had no time to figure that out yet [/OFFTOPIC] ... your question replied the quick 'n dirty way: echo "|" . ($this->get_user_ip_addr()) . "|"; returns |::1| The notice only appears when first visiting the page, it is gone after a refresh and visiting other pages of that site. (PHP 7.1) My solution for now: Unchecking the EU feature. 3 Link to comment Share on other sites More sharing options...
matjazp Posted August 24, 2018 Share Posted August 24, 2018 Looks like extreme-ip-lookup.com can't deal with IPv6 address. And PW is on the same boat. 2 Link to comment Share on other sites More sharing options...
adrian Posted August 24, 2018 Author Share Posted August 24, 2018 2 hours ago, matjazp said: Looks like extreme-ip-lookup.com can't deal with IPv6 address. And PW is on the same boat. Yeah, looks like you are correct. I have found another free service that support IPv6 and it seems to be working fine now. I have committed a new version with this - I'd appreciate if some others could test and make sure there are no issues at your end. Note these lines: https://github.com/adrianbj/CookieManagementBanner/blob/609617167e476fb5a2d964aa431e2a8e7a749a93/CookieManagementBanner.module#L48-L50 if you want to force an EU IP address for testing - both the IPv4 and IPv6 addresses are German. 4 hours ago, ceberlin said: Maybe there should be a note in the plugin settings that using Checking EU is sending personal IP data to extreme-ip-lookup.com (USA?) and restcountries.eu which might be breaking GDPR law (Server location?), when checking that option. Interesting point - I don't honestly know the ins and outs of the law, but I have added a note mentioning this to the config setting. A way to partially avoid this is relying on a local IP to Country database, but they are a pain to maintain, so I think for now the warning will have to do and devs can decide what they prefer. OT, but has anyone out there tested the vanilla JS fork by tpr yet? @GhostRider ? 1 Link to comment Share on other sites More sharing options...
adrian Posted August 24, 2018 Author Share Posted August 24, 2018 Just committed a significant upgrade. The IP to country lookup now has three services it can use - all of which support IPV6. You can specify the order and remove some if you like. They will be used in order and in case one fails, it will move onto the next one. You can now specify an IP address in the config settings to test. Note that saving the module config settings will remove the "FromEU" session variable so the result will be fresh each time. Also, because all of these three services can automatically detect the user's IP address, I am no longer supplying it via the API call (unless you are using the test option), which means that tests from your localhost dev environment will actually work properly. I have also included Tracy bd() calls that return more information. Obviously this will only show if you have Tracy installed, but it tells you which service was used, the results from it, and then the call to the other service to check if the country is in the EU, as well as the result of this test: true/false. 3 Link to comment Share on other sites More sharing options...
adrian Posted August 24, 2018 Author Share Posted August 24, 2018 Just had a small world experience. I have been emailing with Ueli at https://www.whatwedo.ch/ who are the folks who operate the https://ip.nf/ IP to country API service. He just mentioned that they are partners with https://novu.ch/ (built with PW) and that they use ProcessWire a lot. So anyway, he is keen to support this module and to make sure their service will be able to keep up with any requests made by it to their service ? 10 Link to comment Share on other sites More sharing options...
adrian Posted August 24, 2018 Author Share Posted August 24, 2018 Ok, to help make it easy for you guys to test the vanilla JS version that @tpr put together, I have included a new toggle. It defaults to the jQuery one for now so you can upgrade to this new version without breaking anything. Everything appears to work fine, but I would like the folks that use the module together with Google Tag Manager to test that everything is firing as expected. Once I have some confirmation I will remove the jQuery version and this setting and also ask @tpr for a PR so he gets the Github credit for this. Thanks for helping to test. Link to comment Share on other sites More sharing options...
adrian Posted August 25, 2018 Author Share Posted August 25, 2018 I have refactored a few things this morning. 1) On the module config settings page, if you have the Only display for EU visitors setting checked, the check for their status happens on every page load (session is ignored). This does not affect the frontend - users here will still only be checked once per session. 2) There is a new "Clear Local Storage" option to reset stored banner settings so that it's easier to test changes to the banner. 2 Link to comment Share on other sites More sharing options...
wbmnfktr Posted September 4, 2018 Share Posted September 4, 2018 (edited) Bug or feature: Denying cookies works different to what I expected. I allow page visitors to manage cookie settings. Those who deny cookies get a success message but afterwards the cookie banner shows up again. I would expect that those who deny cookies get the same experience as those visitors that allow cookies and therefore that the banner doesn't show up again. Tested with version 0.4.0 in jQuery and vanilla flavour. So... is it my expectation or the module/banner behaviour that needs a fix? +++ UPDATE +++ The option auto-accept mode interferes here. Enabling this results in the slightly unexpected behaviour. Disabling this option ends in the expected behaviour. I think the bug/feature question is answered now. Edited September 4, 2018 by wbmnfktr Update: Reason for behaviour change found Link to comment Share on other sites More sharing options...
adrian Posted September 4, 2018 Author Share Posted September 4, 2018 2 hours ago, wbmnfktr said: +++ UPDATE +++ The option auto-accept mode interferes here. Enabling this results in the slightly unexpected behaviour. Disabling this option ends in the expected behaviour. Thanks for narrowing it down to that option. It looks like it doesn't work in combination with the Allow Users to Manage option. If you uncheck that one, then the AutoAccept seems to work fine. It would be great if you could confirm that at your end, but I think I will make it so it's not possible to select both of these - it will be one or the other. Link to comment Share on other sites More sharing options...
wbmnfktr Posted September 4, 2018 Share Posted September 4, 2018 I can confirm that the latest version only supports either auto-accept mode or allow users to manage. But by now I use both options auto-accept mode and allow users to manage without any problems. I added one line to the cookie.monster.block function (jQuery version) to make it work. //set cookieMonster variables when user blocks cookieMonster.block = function() { // added to disable banner while blocking cookies in auto-accept mode cookieMonster.cfg.viewCount = -1; // cookieMonster.cfg.allowCookies = "n"; cookieMonster.cfg.selectionMade = "y"; cookieMonster.cfg.storedVersion = cookieMonster.cfg.version; cookieMonster.sendActionBeacon(); cookieMonster.updateStatus(); } I'm not sure anymore that auto-accept mode was the real deal-breaker here but that missing line. Maybe the (original) author had something in mind when he/she decided not to add that negative viewCount in the block function. In my use case I need it to get my expected behaviour while having all options (auto-accept and blocking) I want and need. 1 Link to comment Share on other sites More sharing options...
adrian Posted September 4, 2018 Author Share Posted September 4, 2018 @wbmnfktr - I am not sure about that fix. When I add that and I click the Do not consent, the banner keeps appearing on page reload. Does that not happen for you? Link to comment Share on other sites More sharing options...
wbmnfktr Posted September 4, 2018 Share Posted September 4, 2018 In my case the banner doesn't show up again. It works as expected. Banner is gone. Cookies are set. Values for cookies are fine. Link to comment Share on other sites More sharing options...
adrian Posted September 4, 2018 Author Share Posted September 4, 2018 Maybe I don't fully understand what the auto accept mode is intended to do, but there certainly seems to be a difference in behavior when Allows users to manage is checked along with auto accept. Anyway, regarding the new cookieMonster.cfg.viewCount = -1; you have added, I don't really get it - why do we want to set the viewCount when the user blocks marketing cookies? Sorry, I am distracted on other things at the moment and I haven't ever really got my head around this part of the module. Link to comment Share on other sites More sharing options...
wbmnfktr Posted September 4, 2018 Share Posted September 4, 2018 The negative viewCount becomes relevant (at least in my case) at this point around line ~257: // if they haven't explicitly accepted it (ie: auto-accept) then display the banner if (pwcmb_settings.auto_accept && cookieMonster.cfg.viewCount != -1) { cookieMonster.ui.show(); } I don't know if it's really necessary for you to get your head around it - at this point. Maybe in the future. To be honest... I think my actual case seems to be kind of special. Link to comment Share on other sites More sharing options...
adrian Posted September 4, 2018 Author Share Posted September 4, 2018 Yeah, but what I don't get it why you want to display the banner at this point. Auto accept is true and the page has been viewed - why now show it? Link to comment Share on other sites More sharing options...
wbmnfktr Posted September 4, 2018 Share Posted September 4, 2018 I don't want to show the banner. That's the point. auto-accept mode + allow user to manage + user blocks cookies = banner does show up auto-accept mode + allow user to manage + added line to .js + user blocks cookies = banner does not show up Link to comment Share on other sites More sharing options...
adrian Posted September 4, 2018 Author Share Posted September 4, 2018 2 hours ago, wbmnfktr said: I don't want to show the banner. That's the point. auto-accept mode + allow user to manage + user blocks cookies = banner does show up auto-accept mode + allow user to manage + added line to .js + user blocks cookies = banner does not show up I agree but I wonder if you're solution actually makes sense. Surely this bit of code is not logical in the first place - why would we want to show the banner if auto_accept has already been triggered by a view. I feel like they were headed somewhere with this but didn't quite get the logic correct. // if they haven't explicitly accepted it (ie: auto-accept) then display the banner if (pwcmb_settings.auto_accept && cookieMonster.cfg.viewCount != -1) { cookieMonster.ui.show(); } I also find this weird: // if this is NOT the first page view (ie: 2nd, 3rd, 4th, 5th, ...) pageview, then implicitly allow cookies if (cookieMonster.cfg.viewCount == 1) { cookieMonster.allow(false); } // but if they haven't explicitly allowed cookies, then display the banner if (cookieMonster.cfg.viewCount >= 0) { cookieMonster.ui.show(); cookieMonster.cfg.viewCount++; } It says if it's not the first, but then it allows when viewCount == 1 and then another case of displaying the banner if they haven't explicitly allowed cookies. To me the auto accept should work the same as explicitly allowing, otherwise there will be this weird stuff when the user manage option is checked. I don't know - maybe I'll try to get some clarification from the original devs. Link to comment Share on other sites More sharing options...
Noel Boss Posted September 5, 2018 Share Posted September 5, 2018 Hey @adrian, amazing Module, works like a charm! May I ask why you use two checkboxes for the consent instead of radios which toggle automatically in relation to each other? Imho radios would be a better fit for they transport the notion of having an OR relation better than checkboxes where the user probably assumes its a AND relation… For anyone not wanting to use a module, check out http://cookieconsent.insites.com – pretty easy to integrate, I've included it before as a simple first step, with using page-fields to feed the labels. 1 Link to comment Share on other sites More sharing options...
wbmnfktr Posted September 5, 2018 Share Posted September 5, 2018 12 hours ago, adrian said: why would we want to show the banner if auto_accept... auto-accept is like a silent opt-in until someone decides to block cookies. auto-accept doesn't hide the cookie banner at all. Without auto-accept and without user consent (allow cookies) you couldn't trigger analytics or anything else. 12 hours ago, adrian said: It says if it's not the first, but then it allows when viewCount == 1 and then another case of displaying the banner if they haven't explicitly allowed cookies. That's fine, too. Visitors have a chance to opt-out. Their very first visit will not trigger any analytics or third-party cookies as they aren't at that point. When you deal with Google Analytics, Adsense or affiliate/marketing cookies this possibility and settings are nice little helpers. Link to comment Share on other sites More sharing options...
adrian Posted September 5, 2018 Author Share Posted September 5, 2018 7 hours ago, Noel Boss said: May I ask why you use two checkboxes for the consent instead of radios which toggle automatically in relation to each other? Imho radios would be a better fit for they transport the notion of having an OR relation better than checkboxes where the user probably assumes its a AND relation… I didn't write the initial module - this is a port of a Drupal module. I did this for an agency who wrote the Drupal module and because they are now using PW for several sites, they wanted a PW version of this. It was their choice to use checkboxes. As you can tell they are set up to toggle automatically, but I agree it doesn't really make sense - users are used to radios toggling and checkboxes being for multiple selections. Maybe at some point I'll move this module to use radios, but we'll see ? 1 Link to comment Share on other sites More sharing options...
netcarver Posted September 5, 2018 Share Posted September 5, 2018 Hi @adrian, just noticed I'm getting this warning with debug mode on for a site I updated to the latest version... ( ! ) Warning: Illegal string offset 'value' in <snip>/site/assets/cache/FileCompiler/site/modules/CookieManagementBanner/CookieManagementBanner.config.php on line 109 1 Link to comment Share on other sites More sharing options...
adrian Posted September 5, 2018 Author Share Posted September 5, 2018 1 minute ago, netcarver said: Hi @adrian, just noticed I'm getting this warning with debug mode on for a site I updated to the latest version... ( ! ) Warning: Illegal string offset 'value' in <snip>/site/assets/cache/FileCompiler/site/modules/CookieManagementBanner/CookieManagementBanner.config.php on line 109 Thanks Steve, it should be fixed in the latest version just committed. 1 Link to comment Share on other sites More sharing options...
Marco Ro Posted October 22, 2018 Share Posted October 22, 2018 Hi @adrian, I was working with geoip.nekudo (it was among the services you suggested in another post) and so, geoip.nekudo it's not more free service it was bought by ipapi.com ? 1 Link to comment Share on other sites More sharing options...
adrian Posted October 22, 2018 Author Share Posted October 22, 2018 2 hours ago, MarcoPLY said: Hi @adrian, I was working with geoip.nekudo (it was among the services you suggested in another post) and so, geoip.nekudo it's not more free service it was bought by ipapi.com ? Thanks for the heads up - I have removed this one from the module. 1 Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now