Page MenuHomePhabricator

Split GeoIP into a new component
Open, MediumPublic

Description

Background

Years ago, we would make a GeoIP lookup using a script tag added to the skin by CentralNotice. This was necessary to correctly geotarget campaigns to users, without splitting cache, etc.

This loading method has been deprecated thanks to some great work by @ori and others, and removed from the CentralNotice extension. At this point, the only thing remaining is some Javascript which parses a cookie value set by Varnish VCL into the window.Geo global. Please note that CentralNotice only uses country-level information, the rest of the data is being stored for use by other extensions.

Recommendation

Let's decouple GeoIP from CentralNotice. It could go into MediaWiki core or better yet, its own library and subsystem. Not sure what the best form would be.

Everything which depends on the Geo variable should have an explicit dependency to this library.

We should audit the privacy implications of all geodata usages.

Considerations

The last I checked, the following extensions make use of GeoIP:

  • CentralNotice
  • WikimediaEvents
  • NavigationTiming
  • UniversalLanguageSelector -~~ ImageMetrics~~
  • Petition
  • WikimediaShopLink

The following extensions seem to be performing their own geolocation:

  • DeviceMapLogCapture
  • DonationInterface
  • FundraiserLandingPage
  • LandingCheck
  • SecureSessions

Event Timeline

awight raised the priority of this task from to Needs Triage.
awight updated the task description. (Show Details)
awight added subscribers: awight, MBrar.WMF, ori.

This sounds like a good idea to me.

Note that https://en.wikipedia.org/wiki/Wikipedia:Geonotice is a popular gadget which also depends on the Geo variable to add local notices to watchlists. We should try not to break it. There may be other gadgets/userscripts as well.

Change 221759 had a related patch set uploaded (by AndyRussG):
WIP - Don't merge but please review - Refactor client-side API and RL modules for banner display

https://gerrit.wikimedia.org/r/221759

The patch for review isolates GeoIP from other CentralNotice logic. This should facilitate moving it to a new home. :)

Change 221759 merged by jenkins-bot:
Refactor client-side API and RL modules for banner display

https://gerrit.wikimedia.org/r/221759

chasemp triaged this task as Medium priority.May 5 2016, 8:43 PM
Krinkle awarded a token.
Krinkle subscribed.

Change 518449 had a related patch set uploaded (by Vedmaka Wakalaka; owner: Vedmaka Wakalaka):
[mediawiki/extensions/CentralNotice@master] Finer geo targeting

https://gerrit.wikimedia.org/r/518449

Change 588132 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[mediawiki/extensions/CentralNotice@master] Add method for creating unique region code

https://gerrit.wikimedia.org/r/588132

Change 588765 had a related patch set uploaded (by Ejegg; owner: Ejegg):
[mediawiki/extensions/CentralNotice@master] Banner Allocation: region dropdown depends on country

https://gerrit.wikimedia.org/r/588765

Change 518449 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Finer geo targeting

https://gerrit.wikimedia.org/r/518449

Change 588132 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Add method for creating unique region code

https://gerrit.wikimedia.org/r/588132

Change 588765 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Banner Allocation: region dropdown depends on country

https://gerrit.wikimedia.org/r/588765

Note that the LandingCheck extension only does its own geolocation lookup as a fallback (if Geo.country was not passed to it within the link).