Page MenuHomePhabricator

Edge case / bug in id deduplication: running deduping across fragments embedded in data-mw can lead to false positives when that fragment is also part of the main doc (ex: indicators)
Closed, ResolvedPublic

Description

Compare legacy vs Parsoid.

The coordinates in the indicator area has an extra right margin in legacy output which is missing for Parsoid. Inspecting the HTML reveals that in the case of Parsoid, the coordinates looks like this: <span id="mw..", data-x-id="coordinates">...</span>. So, Parsoid thinks that the coordinates id is duplicated and dedupes it. So, this means that the CSS rules #coordinates doesn't apply to Parsoid output which is the cause of the diff.

However, if you look at the full page source, you see this: <meta typeof="mw:Extension/indicator mw:Transclusion" about="#mwt3" id="mwCw" data-mw='{"name":"indicator","attrs":{"name":"coordinates"},"body":{..},"html":"&lt;span id=\"coordinates\">...&lt;/span>","parts":[{"template":{"target":{"wt":"Coord","href":"./Modèle:Coord"},"params":{"1":{"wt":"40"},"2":{"wt":"-105"},"3":{"wt":"scale:20000000"},"display":{"wt":"title"}},"i":0}}]}'/><link rel="mw:PageProp/Category" href="./Catégorie:Article_géolocalisé_aux_États-Unis" about="#mwt3"/> (a bunch of content elided from that).

So, you see that the meta tag's data-mw has embedded HTML that is a copy of the indicator HTML that is now part of the main body. And, running deduping across the indicator and the main doc causes it to conflict with itself.

This is a false positive id conflict and should be handled. This is likely an issue in the OTP in core.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

One option would be to *move* the fragment from the meta tag to the indicator, instead of *copying* it. Or at least removing the id from the meta tag once it is copied? (There's probably some issue here about DOM->HTML->DOM conversions which is why this is being cloned even without an explicit fragment clone operation.)

I don't think Kartographer/1300827 is involved, I think it is more likely the general movement of indicators into ContentHolder fragments.

This appears to be something recent. Several wikis use a script that adds a link for collapsible map based on #coordinates in <indicator> tag. E.g. on this page the "(kaart)" link for collapsible map is currently still there next to coordinates at the top, while on this page the link disappeared now after I purged the page.

As for Kartographer, I tried this locally and can reproduce without having Kartographer installed. A simple example is also reported on the feedback page.

Change #1305673 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/services/parsoid@master] Remove html from data-mw when it's added to the indicators

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

Change #1305673 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Remove html from data-mw when it's added to the indicators

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

Change #1306186 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.24.0-a13

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

Change #1306225 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/extensions/Cite@master] Temporarily disable indicator tests

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

Change #1306226 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/extensions/Cite@master] Re-enable and fix indicator tests

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

Change #1306225 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Temporarily disable indicator tests

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

Change #1306186 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.24.0-a13

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

Change #1306226 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Re-enable and fix indicator tests

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