Move GeoJson page data from an inline script to a data attribute - #932
Merged
Conversation
The page JSON now travels in data-mw-maps-geojson on the map element, matching data-mw-maps-mapdata used by every other Maps map, instead of a var GeoJson inline script. Html::element applies Sanitizer::escapeCombiningChar to the script body, and a <script> is a raw-text element, so a U+0338 in a value reached the map as the literal text ̸. Attribute values are decoded by the HTML parser, so nothing is lost. Pages already in the parser cache carry the inline script and no attribute, so the reader falls back to window.GeoJson. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JeroenDeDauw
marked this pull request as ready for review
July 31, 2026 12:50
The wikipage.content handler body moves into maps.geoJsonPage.initializePage, leaving the hook wiring as one line. The early return for content without a GeoJson map had no coverage that counts toward a failed run: CI's karma QUnit environment never fires wikipage.content, and in the browser runner an uncaught async rethrow from the handler is reported but not counted. Calling initializePage directly makes guard removal a synchronous, attributed test failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The data attribute must win over window.GeoJson when both are present; with the sources swapped, every existing test stayed green. The ampersand test set the attribute via .attr(), which exercises no entity decoding and could not fail independently of the plain read test. Its replacement parses the markup shape GeoJsonMapPageUi emits (quotes and newlines entity-encoded, U+0338 as a character reference), which also pins the attribute name across the PHP/JS boundary: a consistent rename in reader and test helper previously passed the suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not a bug fix. The inline script does not corrupt values:
GeoJsonContent::formatJson()goes throughFormatJson::encode()withoutXMLMETA_OK, so<,>and&are already hex-escaped beforeHtml::element()sees them. What this buys is one less inline script, one less global, and the samedata-mw-maps-*attribute the other maps use.The
window.GeoJsonfallback is only for pages still served from the parser cache with the old markup, and can go once those turn over.Production notes
Spec and review by
Fable 5 (max), implementation by anOpus (max)subagent. It stopped short of opening this PR because the reported bug did not exist; the branch is offered as optional cleanup rather than as the fix that was asked for. The one real instance of the escaping mechanism is U+0338 COMBINING LONG SOLIDUS OVERLAY, whichSanitizer::escapeCombiningChar()rewrites to̸and a raw-text<script>then passes through verbatim.