Closed (fixed)
Project:
GeoIP API
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
16 Feb 2011 at 16:47 UTC
Updated:
15 Apr 2011 at 06:02 UTC
Jump to comment: Most recent file
Comments
Comment #1
bobooon commentedAttached is a patch and new JS file that adds country lookup support to GeoIP API.
Comment #2
drewish commentedI haven't opened up the JS since it's in a zip file but curious about the anti-caching code in hook_init(). For some reason I was thinking we could do that in the page callback. Does it have to be in hook_init?
Comment #3
bobooon commentedI'm actually not sure if it needs to be in hook_init() or in the page callback. It just seemed like hook_init() would be the safer choice, what do you think?
Comment #4
bobooon commentedAlso, should we use jquery.cookie to handle cookie read/writes?
Comment #5
bobooon commentedNew JS file corrects debug mode when using the JSON callback.
Comment #6
drewish commentedCool the text file is much easier to read. We should replace all the tabs with spaces in there and make sure we're using {} to wrap blocks of code in if else statements.
Looking at the lookupCountry() now I'm wondering if it makes more sense to have the caller pass in a callback function that we call from the AJAX call's success function. That way we don't have to block while we're waiting on the request. What do you think?
I kind of feel like the debug bit is a little weird. What do you think about trying to pull the geoip_debug from window.location.search ?
Comment #7
bobooon commentedAttached is a new patch file and javascript file. I updated the lookupCountry function to call a passed in function. I also added in the wondow.location.search regular expression for debug mode. Below is an example of how you would call lookupCountry and define a callback function.
Drupal.geoip.lookupCountry('Drupal.geoip_test');Comment #8
bobooon commentedSlightly updated javascript file that allows an extra parameter to be passed to the callback.
Comment #9
drewish commentedhere's a re-roll of another patch that rob had sent me.
Comment #10
drewish commentedmy re-roll dropped two functions, the json callback and the helper to add the js (which i've renamed geoip_add_js()). i also wrapped all the JS in a closure.
Comment #11
drewish commentedTightened up the regex a bit. Also removed the ability to pass the ip back as an argument. It seemed unused and made things a bit confusing.
Comment #12
drewish commentedNoticed a few issues with that last patch. Looks like if you're on a private IP e.g. 192.168.1.1 and the lookup fails the 'success' callback never fires. I feel like it should either fire or we should accept an error callback. For simplicity I just casted the result to a string so *something* comes back. Also fixed an undefined variable warning and cleaned up a couple of other small formatting issues.
Comment #13
drewish commentedFixed some bad comments.
Comment #14
drewish commentedCommitted to 6.x-1.x