Stop click on _openPopup

[yohanboniface]

Fix #3604.
I had a quick break.
Bubbling from marker is supposed to be the default, now, so I just stopped the event on _openPopup.
I could also just add it on bindPopup, if you prefer this pattern.

[mourner]

Bubbling from marker is supposed to be the default, now

Hmm, but why? I think I'd prefer to keep not bubbling up from markers the default.

[yohanboniface]

Hmm, but why?

The map is an event target, so it's considered while traversing the DOM, and given that it listens for click the event is fired.

I think I'd prefer to keep not bubbling up from markers the default.

Are we sure it's a problem (out of retrocompat) ? It's closer to DOM behaviour, no?

[ktoto]

@yohanboniface problem not in openPopup. problem happen when i have 'click' handler in marker and in map.
openPopup just is special case with this binding.

[yohanboniface]

@ktoto (take this with caution, as it's not fully decided): with the current code when you listen to a click event and you don't want the map to also receive it, you are responsible of stopping it.
In the case of bindPopup, given that the event listener has been added by Leaflet internally, it's its responsibility to then stop it. Makes sense?

[ktoto]

@yohanboniface but before #3307 all be fine. I just want to understand why logic with map is changed?
Why map want click if the first interaction element was marker? Why just not stop on marker at all?

[yohanboniface]

Why map want click if the first interaction element was marker? Why just not stop on marker at all?

Just like in normal DOM, when an event is fired by an element, all its ancestors get it, unless stopPropagation is called at some point.
You may have a use case where you want to catch all click events, both on markers (or polygons) and map.

[mourner]

I'd rather not follow DOM conventions and do an exception here, since I can't think of a use case where you'd want to trigger both map and marker on marker click, but people will have a very hard time adjusting to this breaking change.

[yohanboniface]

I'd rather not follow DOM conventions and do an exception here

You mean exception for marker or for click ? i.e. never bubble click or never bubble click on marker but still bubble it on polygons?
I can think of many use cases where I'd love to receive click events that where initially fired on a polygon. And by the way, being able to receive click on a map whatever the source was would be a killer simplification for all "drawing" application on the map.
But you decide :)

[mourner]

Exception for marker specifically, as opposed to vector layers and image overlays (which can be perceived as "click through" in some interfaces).

[yohanboniface]

OK, I'll do that asap :)

[yohanboniface]

@mourner only for click or we can think about other events?

[mourner]

@yohanboniface let's follow the 0.7.3 behavior here.

[mourner]

This should be in Layer.

[yohanboniface]

I hesitated ;)

[mourner]

Unfortunately IE only supports array indexOf starting from 9. We'll probably have to iterate over the array in a loop.

[yohanboniface]

Gasp :/
Should I add a polyfill L.Util.inArray for that? To make the code shorter here and more readable.

[mourner]

Yeah, maybe something like L.Util.indexOf.

[yohanboniface]

L.Util.indexOf seems meaning it also will work on strings, is that what we want?

[mourner]

It's fine to keep only for arrays since string indexOf doesn't need an util method. It's an internal utility after all, we don't have to make it generic.

[yohanboniface]

OK done :)

[yohanboniface]

Meeeeehhhh, how the console is playing with me!?

Array.prototype.indexOf?

[mourner]

Mine is like this :)

I think we're fine just not using native indexOf. It's not that much of a difference.

[yohanboniface]

Yeah, Chrome vs FF :/
OK, I make the loop for everybody :)

[mourner]

OK, let's rebase now.

[yohanboniface]

Actually I think it's fine at this moment :)

Humm, not sure which moment :p Seems I've commit one second before to remove the use of Array.indexOf

[mourner]

Nevermind :)

[yohanboniface]

Rebased and green :)