Page MenuHomePhabricator

Defective Kartographer maps in Wikivoyage: empty maps and wrong group names
Closed, ResolvedPublicBUG REPORT

Description

The problems described occur only on Wikivoyage because these wikis are using the Wikivoyage mode allowing group and show parameters for Kartographer map definitions!

After the last rollout of the Kartographer extension, at least since last Monday (Jan 16, 2023) there are several failures with Kartographer maps:

  1. wrong group names in the leaflet control list
  2. maps created with ext.kartographer.box are empty: no markers, no geo objects like geolines

Steps to replicate the issue (include links if applicable):

  • Ad 1:
  • You can use every page with a map with markers, for instance https://en.wikivoyage.org/wiki/Edfu
  • Press on the leaflet layers control icon: Among others there are now entries like "Group: 0" instead of Go, See, Do, etc.

senseless-group-names.jpg (591×561 px, 55 KB)

  • Ad 2:
  • Please use a page with a map on German Wikivoyage like https://de.wikivoyage.org/wiki/Edfu
  • On the right side of the title there is a geo coordinate and an earth globe. If you press on this globe a map is inserted but it is empty: no markers and no geo objects. In the last years since the creation of the Kartographer extension it was working fine using the Kartographers guidelines for the creation of such maps.

empty-kartographer-map.jpg (734×1 px, 118 KB)

What happens?:

  • Wrong group names in the leaflet layers control list
  • Empty map.

What should have happened instead?:

  • It should work as before. Please rollback the last Kartographer version.

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Some information to Ad2:
As you can learn from https://de.wikivoyage.org/wiki/MediaWiki:Gadget-MapTools.js the map is created with ext.kartographer.box class. The data for the map objects are loaded from the JavaScript variable wgKartographerLiveData. This variable contains both the group names and the map features.

The new layer data are accepted: In the empty map, group names like "Gruppe: 0" are shown but no layer is added (with map.addGeoJSONLayer).

Event Timeline

The latter user script is trying to retrieve https://maps.wikimedia.org/v4/groupname/features.json for instance: https://maps.wikimedia.org/v4/Unterkunft/features.json

awight subscribed.

This does sound like something my recent changes to Kartographer might have broken. I'm taking a look now and may roll back the extension. Thank you @RolandUnger for the thorough bug report!

Change 880958 had a related patch set uploaded (by Awight; author: Awight):

[mapdata@master] Wire group name property into library output

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

Confirmed that this is a regression, deployed with wmf/1.40.0-wmf.18. The last working version is wmf/1.40.0-wmf.17.

We think that there's a small patch that fixes the glitch so will try to have that ready for a hotfix deployment tomorrow.

@awight would that deal with both problems described ?

Change 881263 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Kartographer@master] Pass group names through to layer selector

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

Change 880958 merged by jenkins-bot:

[mapdata@master] Wire group name property into library output

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

Change 881036 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Kartographer@master] Revert "Breaking upgrade: mapdata"

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

Change 881036 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Revert "Breaking upgrade: mapdata"

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

Change 881045 had a related patch set uploaded (by WMDE-Fisch; author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Kartographer@wmf/1.40.0-wmf.19] Revert "Breaking upgrade: mapdata"

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

I might be wrong but I think I know what's going on with the missing markers on "ext.kartographer.box-based" maps (the globe button in the top right corner) on pages like https://de.wikivoyage.org/wiki/Edfu. The script expects ExternalData. But we resolve these now. There are no ExternalData any more, but the actual Features.

This is the relevant line of code: https://de.wikivoyage.org/wiki/MediaWiki:Gadget-MapTools.js#L-298. As far as I understand this code it ignores Features.

Update: This analysis was wrong, see below.

More insights: The Gadget code reuses code from Kartographer and expects certain methods and properties to be there. For example, it calls map.addGeoJSONLayer( group.id, data[ group.id ], layerOptions ).

We changed the signature of this method in https://gerrit.wikimedia.org/r/859533.

Other methods might be affected as well.

Change 881045 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@wmf/1.40.0-wmf.19] Revert "Breaking upgrade: mapdata"

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

Mentioned in SAL (#wikimedia-operations) [2023-01-18T14:35:30Z] <lucaswerkmeister-wmde@deploy1002> Started scap: Backport for [[gerrit:881045|Revert "Breaking upgrade: mapdata" (T327151)]]

Mentioned in SAL (#wikimedia-operations) [2023-01-18T14:37:17Z] <lucaswerkmeister-wmde@deploy1002> lucaswerkmeister-wmde and wmde-fisch: Backport for [[gerrit:881045|Revert "Breaking upgrade: mapdata" (T327151)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-01-18T14:46:03Z] <lucaswerkmeister-wmde@deploy1002> Finished scap: Backport for [[gerrit:881045|Revert "Breaking upgrade: mapdata" (T327151)]] (duration: 10m 33s)

FYI: We just could revert the breaking change. Both things seem to be working again. An upcoming patch will redo the change including fixes for the cases described here.

Thanks again for the bug report!

Aklapper renamed this task from Defective Kartographer maps in Wikivoyge: empty maps and wrong group names to Defective Kartographer maps in Wikivoyage: empty maps and wrong group names.Jan 18 2023, 4:23 PM

Moving this to the backlog since we haven't fixed the bugs, only temporarily worked around them by reverting an important change that we want to reapply soon.

The script expects ExternalData. But we resolve these now. There are no ExternalData any more, but the actual Features.

This is the relevant line of code: https://de.wikivoyage.org/wiki/MediaWiki:Gadget-MapTools.js#L-298. As far as I understand this code it ignores Features.

The "expand external data" feature flag isn't enabled on production, so this explanation doesn't seem right. The linked code is expanding external data and copying attributions, but it looks like the resulting groups will be the same regardless of external data expansion.

More insights: The Gadget code reuses code from Kartographer and expects certain methods and properties to be there. For example, it calls map.addGeoJSONLayer( group.id, data[ group.id ], layerOptions ).

Thanks, I think this is the issue! The fix would be for the gadget to copy group.name (or the desired label) into layerOptions.name and omit the first parameter to addGeoJSONLayer. But to make migration simpler, I'll make the function backwards-compatible.

Change 881826 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Kartographer@master] Revert "Revert "Breaking upgrade: mapdata""

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

Change 881827 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Kartographer@master] [WIP] Back-compatibility for named layers

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

@awight: The Script https://de.wikivoyage.org/wiki/MediaWiki:Gadget-MapTools.js uses the data from wgKartographerLiveData Javascript variable. In most cases, the features are included in this variable. The only exceptions are external data from OpenStreetMap and Wikimedia Commons: they must be fetched from WM's map server because they are not available elsewhere. Unfortunately, ext.kartographer.box is not able to do this job by itself.

I've added backwards-compatibility so that MapTools will continue to work, we'll deploy permanent fixes to both of the issues with next week's train.

To resolve the deprecation warnings, make this change to the gadget:

-    map.addGeoJSONLayer( group.id, dataGroups[group.id], options );
+    map.addGeoJSONLayer( dataGroups[group.id], options );

The new interface finds group name as group.name, made available by the mapdata library.

Change 881826 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Revert "Revert "Breaking upgrade: mapdata""

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

Change 881263 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Pass group names through to layer selector

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

Change 881827 merged by Awight:

[mediawiki/extensions/Kartographer@master] Backwards-compatibility for named layers

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

Hey @RolandUnger, thanks again for the bug report. After the rollback we reapplied our changes but added a fallback with a deprecation warning so the gadget keeps working. The update should be rolled out to Wikivoyage already so you should change the code to use the new method signature. See T327151#8543268

Note, that we generally can not give guaranties about these methods. addGeoJSONLayer is not meant to be a public interface. Unfortunately there's no well established programmatically way in JS to enforce that. So it should be assumed that methods like that can change without warning. That's true for most client side JS in Wikimedia extensions. See also the scope in the stable interface policy. We try or best to check if we break essential tools before deployment and give warnings heads up but we can not catch everything obviously. :-)

Now, a new failure occured: External data are noted as "Group: 1" (Gruppe: 1) but are not included in their groups. As you can learn from the wgKartographerLiveData variable, these features belong to groups, too.

For instance, use https://de.wikivoyage.org/wiki/Kairo/Downtown .

In the HTML header is written:
"wgKartographerLiveData":{"Anreise":[{"type":"ExternalData","service":"geoline","url":"https://maps.wikimedia.org/geoline?getgeojson=1\u0026ids=Q5017774",...

Q5017774 is the "blue" Cairo Metro Line 1 which belongs to the group "Anreise". This group consists of three external datasets (metro lines) and three markers.

If you click on the full-screen control a map occurs with a wrong layers list:

map-wrong-layers-list.jpg (742×1 px, 208 KB)

Contrary, if you click on the magnify button right of the map title the the expected bahiour is seen:

map-expected-layers-list.jpg (741×1 px, 218 KB)

Note, that we generally can not give guaranties about these methods. addGeoJSONLayer is not meant to be a public interface.

Is there any other method that can be safely used as replacement for addGeoJSONLayer?

Now, a new failure occured: External data are noted as "Group: 1" (Gruppe: 1) but are not included in their groups. As you can learn from the wgKartographerLiveData variable, these features belong to groups, too.

Thanks! We'll look into it. More edgecases \o/ :-)

As shown in the documentation at https://doc.wikimedia.org/Kartographer/master/js/#!/api/Kartographer.Box.MapClass-property-lang , addGeoJSONLayer is a public function.

Yeah it's pretty unfortunate. If we do not explicitly put doc blocks on every JavaScript function stating, that this is a private method, the autogenerated docs will obviously include these methods. Most of the - especially older - JS code is very much underdocumented, and we're also not always in the situation to fix that with every part we touch. There's also no established process for that. - Technically you can access almost any of these JS method anyways.

Because of that it's quite hard to foresee if anyone uses any of the functions anywhere outside of the MediaWiki extension ecosystem. And that's also the main reason why the stable interface policy excludes client side JavaScript and gives no guarantees that methods might change there.

I think what I was mainly aiming for with my "is not meant to be a public interface" is, that all kinds of extensions, that implement client side JS do that by using JS objects and methods all over the place. To explore whats happening there is fine of cause and also to write some cool tools and gadgets around the stuff the was done there. But especially since the code is very much not constantly documented, it cannot be assumed that these methods are there to stay and intended as stable interface to rely on "outside" of the ecosystem.

No hard feelings though and we're here to help and make sure users are happy. Just have to manage different expectations and requirements. :-)

Is there any other method that can be safely used as replacement for addGeoJSONLayer?

I'll sync with my team for a quick talk. For the moment it can be assumed that we're not touching that code again in the near future. See also my lengthy answer around that topic in the post above. Didn't want to scare anybody of, it was just a disclaimer. I'm not sure if there's anything that could be considered a "public interface" atm to do what you're doing.

Now, a new failure occured: External data are noted as "Group: 1" (Gruppe: 1) but are not included in their groups. As you can learn from the wgKartographerLiveData variable, these features belong to groups, too.

Thanks, I was able to reproduce this locally. A minimal test case is https://de.wikivoyage.org/wiki/Benutzer:Adamw/sandbox/T327151 . I'm still trying to understand how this ever worked--when I run this example against older Kartographer code, the layer has a very long title which is just a copy of the included GeoJSON.

Change 884281 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mapdata@master] [DNM] Remember Wikivoyage group name for external groups as well

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

The groups like "Gruppe: 0" (group: 0) are still shown today. But there is now an additional bug: T328739.

I'm surprised that this ever worked correctly, because of a glitch in the "mapdata" library which always pushes ExternalData groups up to the top-level. Running older releases of MediaWiki locally, I see other related bugs like these groups appearing with very long layer titles showing the literal GeoJSON fragment: {"features": [ {"geometry": .... Regardless of when the bug appeared, I agree that mapdata should be improved so that the external data is expanded in-place rather than getting pulled up to the top level.

But now I'm confused, because I can no longer reproduce the error after clicking the globe on Edfu. @RolandUnger Did you fix the layers popup with this change to MapTools.js? Is it sufficient, or are you still waiting on a follow-up change to mapdata?

Since examples for Ad1 and Ad2 are working now I'll close this task, but please comment or reopen if I misunderstand. I've split out the low-level fix out into an optional maintenance task T328760, which can be prioritized if the mapdata return structure causes any more issues.

awight moved this task from Doing to Demo on the WMDE-TechWish-Sprint-2023-01-18 board.

@awight: You are correct that the display of external data never worked correctly. It was changed several times on someone's (Wikipedian's) instance and became worse and worse. I think at the beginning there were hints like Wikidata ids linked to Wikidata. And as you correctly noted, there is another problem: normally these objects should not be displayed because they belong to groups. There do not present separate groups.

In a correct map these external objects are not listed because they belong to the group go (Anreise):

correct-map-without-geo-objects.jpg (664×1 px, 122 KB)

In an incorrect map they are listed as separate/first-level groups which should not happen:

incorrect-map-showing-geo-objects.jpg (846×1 px, 270 KB)

To reproduce the failure you have to understand that we use two completely different map tools at the German Wikivoyage. The Kartographer's build-in scripts are available only on the map itself and if you click on the map's full-screen control. All other actions are replaced by my own scripts (and that's why they work as I think): clicking on the globe icon, clicking on the magnify button below the map, and clicking on the marker numbers. Of course I changed the MediaWiki:Gadget-MapTools.js script for two reasones: to simplify it in the course of general maintenance and to prepare the transfer of the script to the Spanish Wikivoyage (i.e. adding i18n strings).

By the way, on Sunday I detected the bug T328866. It seems that the non-existing function map.addDataLayer is called in Map.js.

Change 887297 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Kartographer@master] Add hints for developers on JS code depenants

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

Change 887297 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Add hints for developers on JS code depenants

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

Since examples for Ad1 and Ad2 are working now I'll close this task, but please comment or reopen if I misunderstand. I've split out the low-level fix out into an optional maintenance task T328760, which can be prioritized if the mapdata return structure causes any more issues.

As stated above, the original issues from this ticket were addressed. There's more general remaining work covered by T328760: Mapdata library should not change structure of GeoJSON. Reopen if you think otherwise, but for new issues best create new tasks.

Change 884281 abandoned by Thiemo Kreuz (WMDE):

[mapdata@master] [DNM] Remember Wikivoyage group name for external groups as well

Reason:

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