backdrop-contrib / leaflet

Backdrop CMS Integration with the Leaflet map scripting library.
https://backdropcms.org/project/leaflet
GNU General Public License v2.0
2 stars 2 forks source link

Uncaught Error: Map container is already initialized. #56

Closed AlexHoebart-ICPDR closed 6 months ago

AlexHoebart-ICPDR commented 6 months ago

In some cases, when I set multiple exposed filters in a View, I get the following Javascript Error:

Uncaught Error: Map container is already initialized.
    at e._initContainer (Map.js:1094:10)
    at e.initialize (Map.js:136:8)
    at new e (Class.js:24:20)
    at Object.<anonymous> (leaflet.backdrop.js?sa18ll:110:20)
    at Function.each (jquery.1.12.js?v=1.12.4:2:2881)
    at n.fn.init.each (jquery.1.12.js?v=1.12.4:2:846)
    at Object.attach (leaflet.backdrop.js?sa18ll:83:35)
    at Object.<anonymous> (backdrop.js?v=1.27.0:73:12)
    at Function.each (jquery.1.12.js?v=1.12.4:2:2931)
    at Backdrop.attachBehaviors (backdrop.js?v=1.27.0:71:5)

I found out that mapId is not unique in these cases. I add a PR which fixes the issue for me. But it does not fix the root issue, because I don't know why this happens. I even don't understand why leaflet_build_map() is called multiple times... It seems to happen in backdrop_html_id()? Could it have something to do with long query string in the URL? I'm sorry that I cannot easily provide a simple test case for this. I would appreciate any hints for further debugging the root of the problem.

indigoxela commented 6 months ago

Many thanks for reporting.

When I did a bigger rewrite recently, I already head-scratched over the ID problem (I saw, they don't seem to be unique at all). I belief, that should get sorted differently. Edit: they are unique via backdrop_html_id(). Edit 2: sometimes they aren't unique. (Different issue)

However, the error seems to be triggered by another problem. Have to play a bit with exposed filters, to figure out, if I can reproduce the problem, and if there's a clean fix for that.

Can you provide a bit more details, how to reproduce? Are you using ajax in your view?

AlexHoebart-ICPDR commented 6 months ago

Yes, it is not related to recent changes. I experienced the issue already a while ago, did that quick workaround - but now I wanted to "clean up" things. I'm not using Ajax in the View, but a combination of contextual and exposed filters (using the views_selective_filters module). The View has a table display and a map attachment (ip_geoloc). I don't think I ever saw this on Drupal7 where I used a similar setup. In my case, the View generally works, but when selecting more filters, the error appears - I just found out that it seems to happen always when there is only one single record in the result set left. So it does not have to do with the filters selected, but the result being 1 record. This one record appears on the attachment map, but not in the table. I could further test, if you could give me a hint which direction too look into...

indigoxela commented 6 months ago

In the meantime I did play with AJAX in a view and also saw that uncaught error then. The fix for the AJAX problem's fairly simple (#58). I wonder if it also fixes the problem with your setup.

I just found out that it seems to happen always when there is only one single record in the result set left

That sounds weird.

This one record appears on the attachment map, but not in the table

Hm. So you have different map layers? I never used ip_geoloc, so probably can't give any smart instructions for testing/debugging. :wink:

AlexHoebart-ICPDR commented 6 months ago

Yes, indeed, that fixes also my problem! You are great! Thank you very much!

Side question: do you know why this is necessary, why this is called multiple times? Isn't this impacting performance?

indigoxela commented 6 months ago

Yes, indeed, that fixes also my problem!

Awesome!

... do you know why this is necessary, why this is called multiple times?

I can't tell why this happens in your setup, but when using AJAX in Backdrop, multiple calls aren't uncommon. Usually these additional calls are caught with jQuery.once, but in this case we're using a property attached by Leaflet, to check if we have to initialize or not. Seems like the property _leaflet isn't attached at all, though (maybe it used to exist...), so the check failed and another initialize was attempted (that's why Leaflet nagged). Using the right property fixes that.

In the meantime I fiddled a more reliable way to construct the container id. That didn't work for blocks before. Testing highly appreciated.

AlexHoebart-ICPDR commented 6 months ago

I can confirm that creating the map id with rand also solves my issue - independently from the other change in the Javascript code.

In my case, the map id is generated by the ip_geoloc module. It used another approach to generate the id and there is an old issue about it here: https://www.drupal.org/project/ip_geoloc/issues/1802732

I replaced in that code the serialize with the rand function and it also fixed the error - so I could test that fix only within that module.

As the change in the Javascript code fixes the issue for both modules, I would say it is preferable, at least from my point of view. Unfortunately, it seems that the ip_geoloc module development does not get a lot of attention recently, neither in Backdrop nor in Drupal (despite D7 version has >4000 users). It is obviously not easy to port as it has a lot of features.

indigoxela commented 6 months ago

As the change in the Javascript code fixes the issue for both modules, I would say it is preferable

They fix different issues, actually. The JS change fixes the attempt to attach multiple times to the same container, the id fix in PHP fixes leaflet map blocks (or a page and a block) - when there are multiple different (independent) maps on the same page. Not something you seem to struggle with, but something that popped up (again) when testing for this issue here.

I'd ask the other way round: does the id fix in PHP break anything in your setup, or is it irrelevant for your use-case?

Unfortunately, it seems that the ip_geoloc module development does not get a lot of attention recently

The Backdrop module maintainer's pretty active though. I'm sure he just missed the recent activity in one of the modules, he maintains. Pro tip: join our Zulip chat and ping him there, he's usually around. Or maybe he reads this: @laryn :wink: have a look at ip_geoloc

AlexHoebart-ICPDR commented 6 months ago

I'd ask the other way round: does the id fix in PHP break anything in your setup, or is it irrelevant for your use-case?

No, it's irrelevant in my use-case. But as I could use the same approach to fix it with ip_geoloc, it seems to be good.

laryn commented 6 months ago

I'm sure he just missed the recent activity in one of the modules, he maintains...

Yes, I miss things periodically. 😅 I am no longer on the project I was testing that module for and have less time -- @AlexHoebart-ICPDR are you interested in joining that module as a co-maintainer, since you are actively using it? There is no formal release yet but I suspect some of your changes may be helpful in getting there.

AlexHoebart-ICPDR commented 6 months ago

@AlexHoebart-ICPDR are you interested in joining that module as a co-maintainer, since you are actively using it? There is no formal release yet but I suspect some of your changes may be helpful in getting there.

@laryn What exactly does it mean to be a co-maintainer? Would you or someone else review my PRs? Developing is just a part of my daily work and I mostly learned it by doing. And I don't use the full feature set of the ip_geoloc module. On the other hand, I used and customized Drupal since version 5 and still have several Drupal7 sites with considerable amount of custom modules and not-yet-ported modules to be migrated to Backdrop - so I already started to port some modules and consider contributing them as I see the benefits of that.

laryn commented 6 months ago

@AlexHoebart-ICPDR I moved this conversation over here: https://github.com/backdrop-contrib/ip_geoloc/issues/12