#3294720: The attachBehaviors() for document is only called after Big Pipe chunks are processed (9.5) fixed a bug with Big Pipe that meant that Drupal.attachBehaviours() only ran after Big Pipe finished, which delayed attachments. Now it runs before and after.
This means that Drupal.behaviors.leaflet.attach now runs twice. This means the following:
add_features() gets called twice with the same data$(document).trigger('leafletMapInit', [data.map, data.lMap, mapid]); gets called twice$(document).trigger('leaflet.map', [data.map, data.lMap, mapid]); gets called twiceAdd some code that listens to leafletMapInit, it will be called twice, which isn't what you would expect for an init.
Use once() to prevent double runs - see https://www.drupal.org/docs/drupal-apis/javascript-api/javascript-api-ov....
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | leaflet_big_pipe_double_behaviours_3337537-4.patch | 13.75 KB | itamair |
Comments
Comment #2
itamair commentedthanks a lot @andrewbelcher.
Could you test/QA and review the attached patch that I built up following your suggestion, with the once() construct/pattern.
From my testing it is indeed limiting each callback to just once in the page ...
I also extended the same fix to the Leaflet Widget implementation.
Please let me know if it works as you would expect now, and the code looks appropriately solid. Feel free to add your corrections/amends ... if any.
Comment #3
jeremyskinner commentedhi @itamair I was going to review this on @andrewbelcher's behalf but we're unable to apply this patch to the 2.2.12 version of leaflet which we're using - it looks like the patch only supports 10.x.
I've attached a 2.2.x compatible version of the same patch as it's not possible for us to move to 10.x at present. I've tested this against our application and it appears to be working correctly.
Comment #4
itamair commentedWell ... actually better inspecting and testing the above approach and solution I realised that is not a working general fix.
With the above solution the once() condition will be triggered only for the all document load ("html"), and that is not good.
It won't trigger the Leaflet Map initialisation in case of Leaflet Map inside Ajax requests and contexts
Typical case the Paragraphs Widgets, in case the Geofield is located as part of a Paragraph ...
In the end, if the Big Pipe triggers twice the Drupal.behaviour ... well it could be right not to stop its second call.
What we need to stop is the re-initialisation of a Leaflet Map inside a container that already have an initialised one.
This assumption made me discovering a weakness in the existing Leaflet module code, that
checks if the
map_container.data('leaflet') === undefinedhere:https://git.drupalcode.org/project/leaflet/-/blob/10.0.x/js/leaflet.drup...
that is right ...
and if not it (else) tries to re-add-features here:
https://git.drupalcode.org/project/leaflet/-/blob/10.0.x/js/leaflet.drup...
that is totally wrong ... and not needed (no real cases to cope with that).
The new attached patch is thus resolving all this, with the following:
- applying itself to the 10.0.x branch (please note that the 2.2.x is not supported anymore, and everybody should update, relying in backward compatibility).
- removing the "else" statement (jon needed);
- wrapping all the subsequent code of the Drupal.behaviors.leaflet in the
if (map_container.data('leaflet') === undefined) {statement ...I have tested all this and works solidly, fixing this issue, hence I am going to commit this into 10.0.x branch and deploy a new Leaflet 10.0.8 release ...
@andrewbelcher, @JeremySkinner confident you will be easily able to translate that into 2.2.x, but keep in mind the 10.0.x branch is the way to go now ...
Comment #6
itamair commentedComment #7
jeremyskinner commentedThanks @itamair, we'll work towards upgrading to 10.x so we can make use of this.