Support several GeoJSON sources in one Leaflet map - #930
Merged
Conversation
Fixes #831 `geojson` becomes a list parameter, so `{{#display_map:geojson=A;B}}` and the Leaflet result format show several GeoJson pages or URLs on one map. Each source is fetched separately; sources that fail are dropped without affecting the rest. The decoded values go to `L.geoJSON()`, which takes a list natively. The delimiter is `;` rather than ParamProcessor's default `,`, since commas are common inside GeoJson page titles and URLs, and `;` is what the other Maps list parameters use. The visual editor saves the whole map layer back to `GeoJson:<GeoJsonSource>`, which with several sources would overwrite one page with the content of all of them. So `GeoJsonSource` and `GeoJsonRevisionId` are set only when the parameter got exactly one value that resolved to a GeoJson page, counted from the values given rather than from which fetches succeeded, so an unrelated page being created or deleted never makes the editor appear or disappear. Considered and omitted: each source as a separately toggleable overlay in the layer control (a distinct feature); a cap on the number of sources (a page can already trigger as many fetches with one map per source); Google Maps, which has no `geojson` parameter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mutation testing found two spots where production logic could be removed without any test noticing: the resource module gate that only loads the GeoJSON editor for an editable source, and the dropping of blank source names, which is what keeps `geojson=A;` editable. Also extracts the repeated two-page setup in LeafletTest and replaces the GeoJSON layer counting helper in FeatureBuilderTest with one that can assert the layer is absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Map data lives in the parser cache, which an extension upgrade does not invalidate, so the new code sees `geojson` as a single GeoJSON object for up to $wgParserCacheExpireTime. The length check then skipped the layer, and the editor was seeded with `geojson[0]` of an object, so it opened empty over a page that still had features. Saving from there replaced them with only what the user had just drawn. Both readers now go through one normalization at the entry point, which also makes the rendering path tolerate map data without a geojson key at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GeoJsonFetcher resolves any page whose content model is JSON, including `MediaWiki:Maps` and `User:*/*.json`. The editor builds its save target as `GeoJson:` plus the bare page name, so such a source offered an edit button pointing at an unrelated page. Also corrects the rationale in the docblock, which described the save as writing the combined content of all sources when it writes just one of them, and pins the leading delimiter case that the reindexing in the constructor exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A source containing a bare JSON number, string or boolean made normalizeJson() return a non-array from a method declared `: array`, so the TypeError took down the whole page render. With several sources per map, one such source would stop all the others from rendering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Say the geojson parameter takes a semicolon-separated list rather than "several" values, and frame the splitting caveat as what it costs an existing wiki. Drop "file" from the parameter description, where it invites reading the accepted URLs as file:// ones. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Fixes #831
geojsonbecomes a list parameter, so{{#display_map:geojson=A;B}}and the Leafletresult format show several GeoJson pages or URLs on one map. Each source is fetched
separately; sources that fail are dropped without affecting the rest.
The delimiter is
;rather than ParamProcessor's default,, since commas are commoninside GeoJson page titles and URLs, and
;is what the other Maps list parameters use.An existing value containing a semicolon now splits into several sources, which the
release notes call out.
The visual editor replaces the whole page it was opened on, so
GeoJsonSourceandGeoJsonRevisionIdare non-null only when exactly one non-empty value was given and itresolved to a page in the GeoJson namespace. The count comes from the values given rather
than from which fetches succeeded, so a source becoming reachable or unreachable can never
turn the editor on for a map that shows several. The namespace condition also fixes a
pre-existing bug: the fetcher resolves any page whose content is JSON, so
geojson=MediaWiki:Mapsused to show an edit button that saved toGeoJson:Maps.Two further failure paths are fixed here: map data still in the parser cache after an
upgrade holds a single GeoJSON object, which the editor would have opened empty over a page
that still had features; and a source whose content is a bare JSON scalar made the fetcher
throw, taking the whole page render down.
Considered and omitted: each source as a separately toggleable overlay in the layer control
(a distinct feature); de-duplicating identical sources; Google Maps, which has no
geojsonparameter. Left open: a cap on the number of sources. Wikitext could already trigger as many
fetches with one map per source, but
Special:Asktakes result-format parameters straightfrom the request and is not parser-cached, so one anonymous request can now trigger N
server-side fetches instead of one.