Page MenuHomePhabricator

Geo coordinates are missing from MCS requess on Wikivoyage en Paris
Closed, ResolvedPublic


A unit test was broken by an underlying content change that switched Wikvoyage to use the page indicators. The functionality of the API was thus broken and coordinates are no longer surfaced by the API.

I skipped it for now so it's not blocking other work. We should fix or remove the test. (Why are we testing against Wikivoyage? [edit: because we are Wikimedia not Wikipedia and our APIs should be useful... and this bug will hit Wikipedia in the future if we are not careful?])


Event Timeline

There was a specific patch to make this work for Wikivoyage, too. See T145003: Service should surface coordinates in Wikivoyage.

We currently incorrectly assume that coordinates are always in the HTML e.g. inside span#coordinates .geo
This is not the case for all wikis and the mobileapps API is very Wikipedia centric in that regard so we should test on more wikis.

Really we should be using Wikidata or the GeoData api for this information.
If the test is failing we should investigate why rather than disabling as that means coordinates are no longer being surfaced there...

Change 368520 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/services/mobileapps@master] Fix broken tests (do not skip)

Jdlrobson renamed this task from Fix or remove unit test for geo coordinates on Wikivoyage en Paris to Geo coordinates are missing from MCS requess on Wikivoyage en Paris.Jul 28 2017, 10:49 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson triaged this task as High priority.EditedJul 28 2017, 10:57 PM

"Paris" wikivoyage test is broken because....
converted the non-standard #geoCoord id into an indicator.

It looks like Parsoid render this as the following:

<span typeof="mw:Extension/indicator mw:Transclusion" about="#mwt151" data-mw="{&quot;parts&quot;:[{&quot;template&quot;:{&quot;target&quot;:{&quot;wt&quot;:&quot;geo&quot;,&quot;href&quot;:&quot;./Template:Geo&quot;},&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;48.856&quot;},&quot;2&quot;:{&quot;wt&quot;:&quot;2.351&quot;}},&quot;i&quot;:0}}]}" id="mwBQc">

Thus it makes it impossible to pull out the coordinates.

Indicators are slowly being adopted by all our projects.... so this will bite us in other places too.

As far as I'm aware the MobileView API doesn't make the coordinates prop accessible, but to be future proof we should rely on this:
Maybe MobileFrontend should be updated to surface that or MCS should stop using the Mobileview Api...?


there isn't really a strong reason to use the mobile view api and we should aim to disentangle the mobileview api from it. MCS should not need to depend on MobileFrontend.

It's a bunch of work, but it looks like the main problems are going to be:

  • Language count
  • Article protection status and if it's editable.
  • the thumbwidth parameter
  • Understanding a bunch of code.

In the short term we could add an additional API request for coordinates.

Change 368954 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/services/mobileapps@master] Source coordinates from GeoData

Change 368954 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Source coordinates from GeoData

Stashbot added a subscriber: Stashbot.

Mentioned in SAL (#wikimedia-operations) [2017-08-15T17:55:00Z] <bsitzmann@tin> Started deploy [mobileapps/deploy@34a1304]: Update mobileapps to 33b80dd (T172829 T152441 T172021 T103362)

Mentioned in SAL (#wikimedia-operations) [2017-08-15T17:59:00Z] <bsitzmann@tin> Finished deploy [mobileapps/deploy@34a1304]: Update mobileapps to 33b80dd (T172829 T152441 T172021 T103362) (duration: 04m 00s)