Adding country context for this module

Absolutely; I'm working on it at the moment and plan to have the patch ready soon.

By the way, While looking at the country list, I noticed that the country array contains a NULL entry, along with the country code strings. But, in the code for checking a geoip match, there is a check for a null country code:

if (isset($popup_settings['eu_only']) && $popup_settings['eu_only']) {
    //use the geoip function if exists
    $country_code = function_exists('geoip_country_code_by_name') ? geoip_country_code_by_name(ip_address()) : '';
    if(module_exists('geoip')) {
      $country_code = geoip_country_code();
    } elseif(module_exists('smart_ip')) {
       $smart_ip_session = smart_ip_session_get('smart_ip');
       $country_code = isset($smart_ip_session['location']['country_code']) ? $smart_ip_session['location']['country_code'] : NULL;
    }
    $geoip_match = false;
    if (in_array($country_code, array(NULL, 'BE', 'BG', 'CZ', 'DK', 'DE', 'EE', 'GB', 'IE', 'EL', 'ES', 'FR', 'HR', 'IT', 'CY', 'LV', 'LT', 'LU', 'HU', 'MT', 'NL', 'NO', 'AT', 'PL', 'PT', 'RO', 'SI', 'SK', 'FI', 'SE', 'UK'))) {
      $geoip_match = true;
    }
    /** This is the second NULL check **/
    if ($country_code == '')
    {
      $geoip_match = true;
    }
  } else {
    $geoip_match = true;
  }

Since the patch I'm working on will allow users to potentially delete the NULL value from the country list, I think the best solution would be to not include NULL in the country list array, add it in an array_push just before checking the list, or just using the extra check that you have already placed in the code. IIRC, using == instead of === , which is what the module is doing now, is sufficient to catch a null value.

OK, here's the patch. One thing to note is the new hook, hook_cookie_compliance_country_info() . I'm using this to allow other modules to provide a callback function for providing a two-letter country code to be evaluated by the geoip check. I went ahead and created three default cases for the hook: Smart IP, GeoIP, and if neither module is enabled, a fallback that always returns a match. I believe this matches the default behavior. If you wouldlike documentation for the hook, I would be glad to provide it.

To check it out, Check the " Limit the popup display to a defined list of countries " checkbox, and a radio list will appear, displaying the options for callbacks. The "always true" fallback only shows if nothing else is available.

New patch; adding some helper text re: installing Smart IP/GeoIP, and the default country list.

Sorry! One more: I found that there is some new code about cookie lifetimes, and my module didn't have that. I'm updating my module to include the missing code, so that the patch is created correctly.

Thanks, let me know if there is anything else I can do. I will be working on #2451131: Add option to add classes to popup/buttons in the meantime.

Sure thing. We're a little snowed under with work over at the office, but I'll aim to get this out before the week's end.

Here's the new patch, along with the country list include file. I'm not sure why, but when I create a patch, it isn't included. Maybe because it's a new/untracked file.

As always, right after checking it I manage to find one more typo. I didn't have a space in the str_replace statement (cleaning up stray spaces).

Just wanted to jump in and say its fine to close this - this ticket was prompted by work with a company I am no longer working for, and my development priorities have changed with my new job.

Yes, those look like reasonable tweaks.