By default, RecurlyJS forms use a user's location (if available) to set default form elements such as country. It is possible to disable this functionality via a variable. I think it would be beneficial to add the ability to disable this via the UI.

Patch to follow.

Comments

markdorison’s picture

Status: Active » Needs review
FileSize
1.91 KB
apotek’s picture

Patch looks good, and the implementation is clean, clear drupal standard.

I have a question though. If the location is being set by GeoIP lookup, then the location tracking is not something that is being set or handled by HTML 5 document.geolocation (which can be intrusive and requires user's to allow the location request), but rather by standard, old-school lookup of IP against a database of IP addresses and locations.

This old mechanism is non-intrusive to the user in that the user doesn't have to provide latitude and longitude location data, but has her location chosen by IP, which is much less specific.

Is the purpose of this patch to provide for extra-sensitive site admins whose users might be jarred by having their country or state pre-selected. Or is it a patch intended mainly to reduce the performance drag of the added IP lookup during the form building?

I ask only because, while it's great to provide the configuration option, it does add to more module code, and I am wondering about the cost/benefit here. If it were about disabling a document.geolocation lookup it would be strike me as far more necessary.

markdorison’s picture

@apotek The intention of this was functional, not performance based. The default functionality using GeoIP lookup results in a site based in the UK rendering a form appropriate for the US if that the user is located in the US. This is not always desired; you may always wish to display a UK specific form.

aburke626’s picture

@markdorison - Does this apply to the 7.x-2.x version? I was looking at the code and we can't apply the second part of the patch as we've taken out that function entirely, because we aren't sending a big list of config options to Recurly anymore. The only mention of 'enableGeoIP' that I can find in v3 of recurly.js is the issue you filed. I'm totally fine with pushing it to a new release of 7.x-1.x, I think there will be other patches that need to go that route, too.

markdorison’s picture

This only makes sense on the 7.x-1.x branch since RecurlyJS itself generates the form. In 7.x-2.x, we build the form so if we wanted to implement functionality similar to this, I believe we would need to do it ourselves.

aburke626’s picture

Status: Needs review » Closed (fixed)