Page MenuHomePhabricator

Empty <maplink> generates unexpected color boxes
Closed, ResolvedPublicBUG REPORT

Description

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

What happens?:

Color boxes like this

image.png (114×413 px, 12 KB)

<a class="mw-kartographer-maplink mw-kartographer-autostyled no-icon mw-kartographer-link" data-mw-kartographer="maplink" data-style="osm-intl" style="background: rgb(34, 139, 34); cursor: pointer; pointer-events: auto; text-decoration: none;" data-overlays="[&quot;mask&quot;]" href="#"></a>

are generated into the text.

What should have happened instead?:

Nothing should be generated.

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

First observed around 11.1.2024 - https://en.wikivoyage.org/wiki/Wikivoyage:Travellers%27_pub#Colored_rectangles_caused_by_mapshapes

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

Event Timeline

I'm having trouble understanding what the structure in those articles is even supposed to achieve and how it is using Kartographer...

It seems as if the map link in the caption of the first Mapframe is confusing the parser ???

No idea what is happening, basically both {{mapshapes}} and {{mapshape}} templates just ultimately instantiate #tag:maplink, and whatever then happens inside mediawiki/kartographer I have no idea. Somehow it transforms those tags into leaflet markers and/or map masks.

But in this case, it also generates some garbage into the text...

No idea what is happening, basically both {{mapshapes}} and {{mapshape}} templates just ultimately instantiate #tag:maplink, and whatever then happens inside mediawiki/kartographer I have no idea. Somehow it transforms those tags into leaflet markers and/or map masks.

The article seems to be adding two maps, but only 1 map is created in practice. And I'm not sure which template is supposed to generate which.

What do you mean? The Fiordland has only one {{mapframe}} and one visible map, and the other one has 2x{{mapframe}} and two visible maps... In any case, the problematic code appears to be generated at the place {{mapshape}} is put, {{mapframe}} placement doesn't seem to have any effect on the issue.

So I'd conclude some transformator from #maplink -> html is the issue...

OK, I think I'm understanding this...

The {{mapshape}} template generates a <maplink> to a map with the OSM object. Because of Wikivoyage, where there is grouping functionality, this shape gets auto added to the mapframe generated with the template {{mapframe}}. This is what the authors are trying to achieve.

They intentionally leave the map link's text empty, so that an empty link is added, instead of a normal link to a map. We can see this in https://en.wikivoyage.org/w/index.php?title=Template:Mapshape/Inner&action=edit However, even though the link is empty, it is rendered, and because it is part of a default group (mask), it does have a background and padding and a position, thus generating the coloured box.

I'm not yet sure if this used to work differently before

For sure it did, we didn't have those color boxes. Probably there was special case handling for this that got removed unintentionally?

@thiemowmde I suspect this got broken in one of the rewrites for LegacyTagHandler vs ParsoidTagHandler ? Funnily enough, it works correctly when you use Parsoid ;)

I notice that the parsoid version doesn't have the mw-kartographer-autostyled class, while the legacy version of the page does have this class on the element (which changes the dimensions and makes it visible).

TheDJ renamed this task from maplink generates color boxes in text to Empty <maplink> generates unexpected color boxes.Jan 17 2024, 2:46 PM
TheDJ triaged this task as Medium priority.
TheDJ added a project: Regression.

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

[mediawiki/extensions/Kartographer@master] Fix state bleeding from one <maplink> into the next

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

thiemowmde added a subscriber: Fomafix.

11 months ago we learned that the Wikivoyage communities used an undocumented feature of Kartographer to create invisible maplinks with no text. See T328739 and the fix https://gerrit.wikimedia.org/r/886338. We properly documented this and covered it with tests. Nothing changed since then.

So I thought. I did a git bisect and found https://gerrit.wikimedia.org/r/985929 as the first bad commit. Before, a new LegacyMapLink instance was created for every <maplink>. Now there is only a single instance. The moment one <maplink> is auto-colored (which is something that's only possibly on Wikivoyages) but the next is not the color is not reset and bleeds into all following <maplink>.

I made a quick patch. An alternative is to revert https://gerrit.wikimedia.org/r/985929.

If this is a too big maintenance issue and you see some cleaner way to achieve the same result, I think modifying WV templates/calls is also an option, if you can suggest the API we should use...

It's really a bug in the code that slipped through because we had no test for it. I don't think there is a way to work around it, sorry. https://gerrit.wikimedia.org/r/991405 should probably be backported to fix it ASAP. @TheDJ or maybe @WMDE-Fisch, can you help? I won't be able to do this.

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

[mediawiki/extensions/Kartographer@wmf/1.42.0-wmf.14] Fix state bleeding from one <maplink> into the next

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

It's really a bug in the code that slipped through because we had no test for it. I don't think there is a way to work around it, sorry. https://gerrit.wikimedia.org/r/991405 should probably be backported to fix it ASAP. @TheDJ or maybe @WMDE-Fisch, can you help? I won't be able to do this.

FYI: Unfortunately we don't have capacity to backport this on short notice at the end of the week and take responsibility for possible outcomes of that.

It'll have to wait until next week. But could at least be tested intensively on the beta cluster now. Let's hope that the fix does not introduce some new inconvenience ... there was at least some weird test failing on CI now. Although I'm quite confident that that's mostly unrelated to a regression in the code. See T355300: Kartographer nearby qunit test fails randomly on gate and submit

Change 991343 abandoned by WMDE-Fisch:

[mediawiki/extensions/Kartographer@wmf/1.42.0-wmf.14] Fix state bleeding from one <maplink> into the next

Reason:

The patch on master evolved and we even might not backport it.

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

Change 991405 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Fix state bleeding from one <maplink> into the next

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

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

[mediawiki/extensions/Kartographer@master] [WIP] Remove misplaced $markerProperties from handler class(es)

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

The underlying issue should be fixed. If there are still colored boxes anywhere, it might help to save the affected article anew to clear the cache.

Andree.sk claimed this task.

Change 991626 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Remove misplaced $markerProperties from handler class(es)

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