Skip to content

Issue 1520: Propagate feature to child layers of multi-geometries - #2263

Open
nina-mir wants to merge 3 commits into
python-visualization:mainfrom
nina-mir:fix-1520-multipoint
Open

Issue 1520: Propagate feature to child layers of multi-geometries#2263
nina-mir wants to merge 3 commits into
python-visualization:mainfrom
nina-mir:fix-1520-multipoint

Conversation

@nina-mir

@nina-mir nina-mir commented Aug 8, 2026

Copy link
Copy Markdown

Leaflet's geometryToLayer returns a FeatureGroup for MultiPoint, and addData assigns feature to that group only. Tooltips resolve their source to the layer that fired the event, which is a child marker with no feature, so the content function throws before rendering. This copies the feature onto child layers in onEachFeature, descending recursively so that nested groups — a GeometryCollection containing a MultiPoint — are also covered.

Verified in Firefox/Chrome against the reporter's snippet and a matrix of all eight geometry types. MultiLineString and MultiPolygon are unaffected, since Leaflet flattens those into a single layer rather than a group.

Geometry Leaflet returns Before After
Point, LineString, Polygon single layer works works
MultiLineString, MultiPolygon single layer (flattened) works works
MultiPoint FeatureGroup broken works
GeometryCollection FeatureGroup broken works
GeometryCollection containing MultiPoint nested FeatureGroup broken works
Console error before the fix
Uncaught TypeError: can't access property "properties", layer.feature is undefined
    <anonymous> 
    _updateContent DivOverlay.js:277
    update DivOverlay.js:187
    onAdd DivOverlay.js:113
    onAdd Tooltip.js:84
    _layerAdd Layer.js:114
    whenReady Map.js:1477
    addLayer Layer.js:172
    openOn DivOverlay.js:63
    openTooltip Tooltip.js:337
    _openTooltip Tooltip.js:420
    fire Events.js:195
    _propagateEvent Events.js:311
    fire Events.js:204
    _propagateEvent Events.js:311
    fire Events.js:204
    _propagateEvent Events.js:311
    fire Events.js:204
    _fireDOMEvent Map.js:1452
    _handleDOMEvent Map.js:1401
    o DomEvent.js:108

If this approach looks right, the MultiPoint limitation in the GeoJSON docs and the GeometryCollection warning in GeoJsonDetail would need updating. Happy to do that here or in a follow-up.

Tested with

  • folium 0.20.0 (reproduction) and this branch off 03cb432
  • Leaflet 1.9.3, as pinned in folium/folium.py
  • Firefox 153.0.1 and Chrome 151 on Linux

Two tests fail on this branch, both of which also fail on clean main:

  • tests/test_map.py::test_icon_invalid_marker_colorsIcon.__init__ calls color.startswith("#") before validating the type, so color=42 raises AttributeError instead of emitting the expected UserWarning (folium/map.py:436). Unrelated to this change; happy to open a separate issue or PR.

  • tests/plugins/test_time_slider_choropleth.py::test_timedynamic_geo_jsongeodatasets not installed locally.

tests/test_features.py: 20 passed. tests/test_map.py and tests/plugins: 91 passed, 2 failed (both above).

Fixes #1520

Leaflet's geometryToLayer returns a FeatureGroup for MultiPoint, and addData assigns `feature` to that group only. Tooltips resolve their source to the layer that fired the event (Tooltip.js:418, v1.9.3), which is a child marker with no `feature`, so GeoJsonTooltip and GeoJsonPopup throw and never render.

Copy the feature onto child layers in onEachFeature, before any user-supplied callback, so both see the same data.

Fixes python-visualization#1520
A GeometryCollection containing a MultiPoint produces a FeatureGroup inside a FeatureGroup, placing the hovered marker two levels below the layer that owns `feature`. eachLayer only iterates one level, so the propagation now recurses.
@hansthen

hansthen commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

@nina-mir This looks like a promising and elegant approach. I must admit I do not grasp quite how this change solves the issue. Could you explain this a bit more (just for my curiosity)?

Interested to see the regression tests. I'd be happy to accept this here. Isn't this also an upstream problem? It looks like an issue in Leaflet itself as well.

@nina-mir

nina-mir commented Aug 8, 2026

Copy link
Copy Markdown
Author

Happy to explain!

The chain begins with the L.geoJson(...) call we emit, which is populated through addData.
That method runs once for each feature and performs three steps in sequence: build a
layer with geometryToLayer, assign feature to it, then call onEachFeature
(GeoJSON.js:118-129, v1.9.3).

The catch is what geometryToLayer returns. For most geometries it's a single
layer, but for MultiPoint and GeometryCollection it's a FeatureGroup built
from child layers. And, addData assigns feature to whatever it got back. So
for a MultiPoint the group gets feature and the markers inside it get nothing, which is a problem for tooltip binding!

GeoJsonTooltip binds one tooltip on the whole GeoJSON layer, with a function as its
content. When the tooltip opens, Leaflet picks the layer to hand that function via
this._tooltip._source = e.layer || e.target (Tooltip.js:418, v1.9.3) — the layer
that fired the event, i.e. the child marker under the cursor. That marker has no
feature, so layer.feature.properties[...] throws before anything renders.

So the two ends disagree: feature is assigned to the group, the content function is
handed a child. Instrumenting onEachFeature shows it directly — for a MultiPoint,
layer.getLayers().map(c => c.feature !== undefined) is [false] before the patch and
[true] after.

The patch copies the feature down onto the children inside onEachFeature, which is
the one point where both the feature data and the constructed layer are accessible.
The recursion is for a GeometryCollection containing a MultiPoint, where the marker
sits two levels below the layer that owns feature.

It also explains why MultiLineString and MultiPolygon were never affected — Leaflet
flattens those into a single Polyline/Polygon, so the layer you hover is the layer
that owns feature.

On upstream: I tested it outside folium via plain Leaflet from a CDN, the reporter's
coordinates, tooltip content as a function reading layer.feature. Same behavior in
1.9.3 (our pin), 1.9.4, and 2.0.0-alpha.1: Point works, MultiPoint throws. So it isn't
something folium introduces, but it also looks deliberate rather than accidental —
addData assigns feature to the layer geometryToLayer returned, and _openTooltip
resolves to the layer that fired the event. Both rules are reasonable alone; they
disagree when the geometry produces a group. 2.x renamed e.layer to
e.propagatedFrom and kept the same resolution, so the rewrite didn't change it.

I couldn't find an existing Leaflet issue for this. Happy to open one with the repro if
you think it's worth raising — though with 1.x in maintenance and 2.x behaving the
same, handling it here seems worthwhile either way. Glad to share the test pages too.

Regression tests are next: tests/selenium, covering MultiPoint and the nested case.

@nina-mir
nina-mir marked this pull request as ready for review August 9, 2026 02:44
@nina-mir

nina-mir commented Aug 9, 2026

Copy link
Copy Markdown
Author

Test added. It fails on main (tooltip never renders, wait_until times out) and passes here.

Two notes on how it's written:

  • It's the first hover-based test in tests/selenium/, and the hover is dispatched as a DOM MouseEvent rather than through ActionChains — Selenium's pointer actions don't reliably reach Leaflet's SVG paths in headless Chrome, while a bubbling event goes through Map._handleDOMEvent the same way a real hover does.

  • It deliberately omits verify_js_logs(): Leaflet 1.9.3 throws t.getElement is not a function from _addFocusListenersOnLayer for any GeoJSON layer containing a nested FeatureGroup, regardless of this fix. That's guarded upstream in 1.9.4, so a version bump would let the assertion come back. Happy to open that separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tooltip and Popup don't work in GeoJson with MultiPoint geometry

2 participants