Page MenuHomePhabricator

Kartographer maplink presents senseless coordinates in Wikivoyage for geoline external data
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT

Description

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

What happens?:

  • An additional paragraph occurs showing "0°0′0″N 0°0′0″E" several times if a maplink tag is generated using external data (from OSM). The maplink definition was not defined as a part of a mapframe but separately outside the mapframe definition. This is a usual method in Wikivoyage.

senseless-coordinates.jpg (763×1 px, 392 KB)

What should have happened instead?:

  • Nothing should be shown.

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

  • Kartographer – (eab7e1c) 23:43, 30. Jan. 2023

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

  • The mapshape template generates a dummy maplink that should not shown in text but only on maps.

I recommend reverting the changes.

Demo for fix:
https://de.wikivoyage.org/wiki/Kairo/Ta%E1%B8%A5r%C4%ABr-Platz

  • The faulty paragraph with "0°0′0″N 0°0′0″E" is gone.

Event Timeline

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

[mediawiki/extensions/Kartographer@master] Fix and add mising parser test for maplink with suppressed text=""

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

thiemowmde moved this task from Proposed to Currently in sprint on the WMDE-TechWish board.
thiemowmde set the point value for this task to 2.
thiemowmde subscribed.

From the example article:

{{Mapshape | type = geoline | id = Q5017774 | stroke = {{Cairo Metro|Linie = 1 }} | stroke-opacity = 1 | stroke-width = 3 | group = Anreise }}

Expanded via https://de.wikivoyage.org/wiki/Spezial:Vorlagen_expandieren:

<maplink class="no-icon" group="Anreise" text="">{"properties":{"stroke-opacity":1,"title":"Metrolinie 1 in Kairo","stroke-width":3,"fill-opacity":0.5,"stroke":"#0000FF","description":"[[file:Egypt.Cairo.Metro.01.jpg|141px]]","fill":"#555555"},"type":"ExternalData","service":"geoline","ids":"Q5017774"}</maplink>[[Category:Seiten, die die Wikidata-Eigenschaft P18 benutzen]][[Category:Seiten, die die Wikidata-Eigenschaft P18 benutzen]]

This, but minimized:

<maplink>{"type":"ExternalData","service":"geoline","ids":"Q5017774"}</maplink>

This ExternalData node will be resolved like this: https://maps.wikimedia.org/geoline?getgeojson=1&ids=Q55098. This appears to work this time.

The resulting GeoJSON renders just fine when I put it in a <mapframe>. In a <maplink> without a label it will also work but show said "0°0′0″N 0°0′0″E":

<maplink>{"type":"FeatureCollection","features":[{"type":"Feature","id":"Q55098","properties":{},"geometry":{"type":"MultiLineString","coordinates":[[[7.412041582,43.728095448],[7.413299621,43.727421543],[7.415611099,43.726263751],[7.418523391,43.72475987]],[[7.418523391,43.72475987],[7.419216826,43.725267394],[7.41929109,43.725213079],[7.419376082,43.725201093],[7.419482449,43.725263957],[7.419483454,43.725344088],[7.419405838,43.725403265],[7.420469921,43.726174735],[7.420531276,43.726130227],[7.420641833,43.72612436],[7.420720875,43.72618253],[7.420730346,43.726206335],[7.420713331,43.726267942],[7.420652059,43.726310522],[7.4209984,43.726559464],[7.423979926,43.727963852],[7.423831483,43.728127886]],[[7.422822134,43.728464922],[7.423831483,43.728127886]],[[7.422822134,43.728464922],[7.422833869,43.728600458],[7.422764299,43.728607331],[7.422804532,43.729411323]],[[7.422589369,43.729487096],[7.422804532,43.729411323]],[[7.418489947,43.730922329],[7.418515596,43.730940099],[7.418552644,43.730911935],[7.418584831,43.730931884],[7.418626237,43.730904056],[7.41868424,43.730943535],[7.418824134,43.730895926],[7.418906947,43.730910259],[7.418994873,43.730887879],[7.419014068,43.730847478],[7.419153626,43.730810933],[7.419202493,43.730832391],[7.419282792,43.730811855],[7.419315649,43.730781681],[7.419388236,43.730749159],[7.419461745,43.730769275],[7.419680429,43.730559895],[7.419797273,43.730531062],[7.419886037,43.730427126],[7.419972958,43.730255632],[7.420031463,43.730261751],[7.420080497,43.730212549],[7.420096255,43.73009646],[7.420228354,43.730003337],[7.420345282,43.729884565],[7.42050085,43.729863862],[7.420680139,43.729907699],[7.420773429,43.729894875],[7.420848531,43.730002164],[7.42092204,43.729990261],[7.420967889,43.730072236],[7.421087667,43.73010677],[7.42114106,43.730092018],[7.421146759,43.73016494],[7.421243486,43.73021892],[7.421497374,43.730180698],[7.42164456,43.73009252],[7.421743718,43.730065866],[7.421805661,43.729853049],[7.42176065,43.729806698],[7.421756878,43.729700834],[7.421804571,43.72967133],[7.421923175,43.72965532],[7.421988135,43.729557923],[7.422218972,43.729439319],[7.422327853,43.729524898],[7.422443272,43.729536297],[7.422589369,43.729487096]],[[7.417518569,43.73157377],[7.417501889,43.731466985],[7.417959876,43.731163225],[7.418094238,43.731224077],[7.418489947,43.730922329]],[[7.415860377,43.731388447],[7.416194982,43.731156687],[7.417518569,43.73157377]],[[7.415860377,43.731388447],[7.414716163,43.730874971],[7.414305953,43.730554447],[7.412819674,43.730151361],[7.412955041,43.729998895],[7.412678774,43.72968692],[7.412860829,43.729579045],[7.412184577,43.728415385],[7.412041582,43.728095448]]]}}]}</maplink>

As far as I can see this is not new. While there is an auto-position feature it can't extrapolate a single coordinate for a label from a line or shape. How should it do this?

Turns out the key is the empty text="". This broke because it was never covered by a test and the old argument validation before https://gerrit.wikimedia.org/r/884958 was just terrible.

Change 886338 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Fix and add mising parser test for maplink with suppressed text=""

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

The authors of the English Wikivoyage are really angry. This is the second serious programming error within the last two weeks.

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

[mediawiki/extensions/Kartographer@wmf/1.40.0-wmf.21] Fix and add mising parser test for maplink with suppressed text=""

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

Do you have a link?

As said, the behavior with empty text="" was neither tested nor documented anywhere, as far as I can see. When reading the code it really looks like it was never meant to work like it did. In a <maplink> the text argument was validated with a regular expression that said \S+, which means it cannot be empty. An empty text="" was really meant to be rejected. Which makes sense. I mean, a link that can't be clicked is obviously broken, isn't it? Technically the validation happened and a failure state got generated, but to late, and the failure was never shown because of a programming error.

But this doesn't matter much any more. What matters is that we learned that this old bug turned out to be a useful feature. There was no way to know this ahead of time. Only now it's coded and tested properly.

It is correct that the Kartographer software was not yet well documented. It is unfurtunately the same like other software projects including mw core. Primarily, the Kartographer software was developed for the use in Wikivoyage wikis in a collaboration with Wikivoyage's authors, and that's why the prior behavior was intended even if not documented (or only in the shallow depths of the web). In case of point markers, usually the text is not empty (text="") but undefined (see for instance function mu.makeMarkerSymbol in https://de.wikivoyage.org/wiki/Modul:Marker_utilities). In case of lines and shapes defined by <maplink>, empty texts are combined with class = "no-icon" (see for instance function _mapframe in https://de.wikivoyage.org/wiki/Modul:Mapframe). This makes sense for geo objects like lines and shapes because they usually have no point coordinate which could serve as a map center.

There is another cause: In most cases, geo objects (points, lines, shapes) are not noted in Wikivoyage alltogether in a <mapframe> tag but separatedly outside this tag (Wikivoyage mode). They (sometimes several hundreds) will be collected for the use in usually one <mapframe> only. On the one hand, this is easier to handle by authors because GeoJSON is difficult to handle in an error-free manner. On the other hand, only this practice makes it possible to arrange geo objects into different groups (Wikivoyage mode). Point markers get a number. Lines and shapes get nothing because they are only used in <mapframe>s but not in <maplink>s.

I know that it is not easy to understand. But you could ask people from Wikivoyage communities like me. That's why I gave Johanna my contacts.

Geo objects (lines, shapes) defined in <maplink>s are never written to the articles source code/HTML code. That's why they could never used as a link.

Change 886105 merged by Urbanecm:

[mediawiki/extensions/Kartographer@wmf/1.40.0-wmf.21] Fix and add mising parser test for maplink with suppressed text=""

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

Mentioned in SAL (#wikimedia-operations) [2023-02-06T08:46:07Z] <urbanecm@deploy1002> Started scap: Backport for [[gerrit:886105|Fix and add mising parser test for maplink with suppressed text="" (T328739)]]

Mentioned in SAL (#wikimedia-operations) [2023-02-06T08:56:03Z] <urbanecm@deploy1002> wmde-fisch and urbanecm: Backport for [[gerrit:886105|Fix and add mising parser test for maplink with suppressed text="" (T328739)]] synced to the testservers: mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet

The "parse-time geoshape expansion feature" we will deploy in T326317 makes the external data available to Kartographer, so if this is enabled in the future it will become possible to render these links with actual coordinates.

Mentioned in SAL (#wikimedia-operations) [2023-02-06T09:05:03Z] <urbanecm@deploy1002> Finished scap: Backport for [[gerrit:886105|Fix and add mising parser test for maplink with suppressed text="" (T328739)]] (duration: 18m 56s)

FYI: The issue should be fixed. The patch for that could be merged to the master branch on Friday. Due to the no-deploy policy from Friday to Sunday we had to wait for the backport on production until now.

It might be that due to caching some links are still visible. They well vanish over time as the cache gets invalidated, or by updating the page with an edit.

I just want to be sure that the last "0.00" case is acceptable? See https://en.wikipedia.beta.wmflabs.org/wiki/Maptests/T328739

the last "0.00" case

The second last example on that page doesn't suppress the label with text="" and auto-generates one that says "0°0′0″N 0°0′0″E". That's obviously bogus and should never appear in an actual article. However:

  • The behavior always was like this. As far as I know nobody complained about it, so there should be no pressure to change it now.
  • Not sure what the alternative should be. An error message? I think the current "0°0′0″N 0°0′0″E" is so obvious that it effectively acts as an error message.

One simple idea I have is to add class="error" to such broken maplinks to make them more visible. What do you think?

  • Not sure what the alternative should be. An error message? I think the current "0°0′0″N 0°0′0″E" is so obvious that it effectively acts as an error message.

I would just show the marker and no coordinates, in other words implicitly assume text="".

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

[mediawiki/extensions/Kartographer@master] [POC] Mark bogus 0°0′0″N 0°0′0″E maplinks as red error

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

Change 887944 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Mark bogus 0°0′0″N 0°0′0″E maplinks as red error

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