• Resolved David Anderson

    (@davidanderson)


    Hi,

    This looks a useful plugin for programmers to build upon. Thank you! I was beginning to use it to build a country selector within another plugin.

    However, on the site I want to deploy upon, another plugin already has the same GeoIP library included. This causes a fatal error, as your library does not apply checks for whether the library is already loaded (the other plugin does – but unfortunately the other plugin can get loaded first).

    i.e. To make your plugin play nice with other plugins, you need to add various wrappers like:

    if(!class_exists(‘geoiprecord’)) {
    class geoiprecord {
    // (Previous contents of geoiprecord class)
    }
    }

    Alternatively, if you are happy to be PHP 5.3 only, then use name-spacing.

    Even better: update from the legacy PHP API to the current one: https://github.com/maxmind/GeoIP2-php

    Best wishes,
    David

    https://www.ads-software.com/plugins/geoip-detect/

Viewing 6 replies - 1 through 6 (of 6 total)
  • Thread Starter David Anderson

    (@davidanderson)

    Hi,

    I’ve created for you a patched version of this plugin with 3 improvements… get it here: https://david.dw-perspective.org.uk/tmp/geoip-detect.1.5.1.zip

    You are welcome to release this patched version as your next version.

    The 3 improvements are as follows:

    * FIX: Does not cause fatal errors if another plugin has already loaded the GeoIP library (checks function + class existence first)

    * FIX: Prevent PHP notice when looking up IPs with no region entry in GeoIP database

    * FIX: Remove duplicate copy of GeoIP library (your plugin had 2 copies in, and only one was used) and other unused testing scripts from the GeoIP library

    This is still using the old PHP API. I am in a hurry so just wanted to get something working quickly!

    Best wishes,
    David

    Plugin Author Benjamin Pick

    (@benjaminpick)

    Many thanks for taking the pain and even proposing a patch! I will have a look at it and adapt it in a way that I don’t need to modify the Maxmind files.

    Yes I agree the APIv2 would be great but AFAIK it is still in beta. Definitely something I consider for the long-term. However we will have to see how to keep backwards compatibility, I don’t want to include both libraries yet existing code should continue working.

    Thread Starter David Anderson

    (@davidanderson)

    Hi Benjamin,

    Thanks for getting back.

    It looked to me like APIv2 is no longer in beta – the github page says that v1 is “legacy”, and I can’t see any indications for v2 of not being current.

    Based on that, I wouldn’t worry about modifying the MaxMind libraries, as they’re not likely to be further developed. There shouldn’t be future changes to include. Actually the versions I put into yours were a fork maintained by Diego Zanella for one of his plugins (https://dev.pathtoenlightenment.net/shop/currency-switcher-woocommerce/), so you shouldn’t have any maintenance work. (And if you did, it should be minimal, presuming that you’re planning to switch from the legacy library anyway).

    David

    Plugin Author Benjamin Pick

    (@benjaminpick)

    class_exists()-Wrappers will be include in the about-to-be-released Version 1.6

    Thread Starter David Anderson

    (@davidanderson)

    Hi,

    Thanks – did you include any of the other 2 fixes (mentioned in my second post in this thread) in that release?

    David

    Plugin Author Benjamin Pick

    (@benjaminpick)

    Yes I did include other minor fixes from your version while not modifying the vendor-Library.

Viewing 6 replies - 1 through 6 (of 6 total)
  • The topic ‘Fatal PHP error on activation due to duplicate function names’ is closed to new replies.