Leaflet / Leaflet

πŸƒ JavaScript library for mobile-friendly interactive maps πŸ‡ΊπŸ‡¦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.97k stars 5.81k forks source link

Problem with marker click and popup/pan #1041

Closed Mathbl closed 11 years ago

Mathbl commented 11 years ago

Hello!

Not sure if the problem is with my code, but here is the situation :

Problem is that on nexus7 (but not on HTC Glacier running 2.3.6), when I click on a marker, the map pans, the binded popup opens, but it closes right after.

I did a little video to show what I mean.

http://d.pr/v/Na1j

Here is the way I do it (i've changed some variables name to simplify the code)

marker = new L.Marker(aMarkerLocation); marker.bindPopup("Point " + (poiNumber+1) );

I set the click event listener :

marker.addEventListener('click', function() { enableContent(circuitInfo, poiNumber, index, numberOfPoi, aMarkerLocation); }

And finally the enableContent function does that

map.setView(aMarkerLocation, map.getZoom()); marker.openPopup();

Anyone have a cue on what the problem could be ?

P.S. While testing several times, it seems to me that when a marker is clicked, the map pans, the popup opens but there seems to be another click triggered at the position where the marker was before panning...weird.

Thanks !

danzel commented 11 years ago

Could you please test with leaflet 0.4.4 (http://leaflet.cloudmade.com/download.html) to see if it happens there. A jsfiddle would be good too so we can test on some other devices, we've had a few really weird issues with android 4.1 Thanks!

Mathbl commented 11 years ago

Hello!

I think the jsfiddle wouldn't be useful in that case because it happens in the webview. If I test the code with google chrome (preinstalled on the nexus7) everything works well. But I did an example (note that there may be some strange things in the code, like the window variable, but in my app, it needs to be this way for things to work ;) ) http://jsfiddle.net/TddAj/embedded/result/

I did some research on that issue, and it may be on android side, nothing to do with leaflet. See the posts here :

https://groups.google.com/forum/?fromgroups=#!topic/iscroll/XL2Ny9gpKc0 http://code.google.com/p/android/issues/detail?id=37550

Problem is my only test device on 4.1 is nexus7 and it comes with chrome as the default browser. But from the posts I stated before, there's a jsfiddle that is suppose to show the problem.

danzel commented 11 years ago

Dumb, yeah this looks like an android browser issue :-(

danzel commented 11 years ago

@ciberado posted this apparently (I recieved an email alert), but then it disappeared?

~ Yes, it is. I'm suffering the same issue.

WebView in 4.1 has a broken implementation of the touchend event: it throws a native click. So you have two of them for every gesture you make, once when you press the screen and another one when you leave it.

The behaviour you're experiencing is caused by the second click, which closes the just popped marker.

I'm trying to hack the event-related source code in order to workaround it (I have a demo of the product this week!) but I'm not sure if I'll be able to do it in time. So any help will be very very appreciated. Oh, and please, forgive my English. ~

I would look at hacking popup open/close to not trigger again within a certain amount of time since the last popup open/close event. Record the time on each open/close call, if it is less than 1 second since the last then bail out.

Mathbl commented 11 years ago

Hello Danzel,

Yes I received that too. Unfortunately on this point I cannot help, it's a little bit too much for my skill I guess.

It's just bad that new problems are coming as new android versions release. :/

danzel commented 11 years ago

Yeah, I don't suggest we put a work around for this in leaflet, it is just really broken browser behaviour, which as you say seems to get worse with each android release :-( If you really need to support popups on android 4.1 WebView, something like the work around I posted above should work

Mathbl commented 11 years ago

For my app, it's not THAT important, as I also got buttons to navigate through markers. But more and more people seems to be getting 4.1 update, something like 30% of my users...

Congrats on your work, really appreciated

Mathbl commented 11 years ago

The problem with your workaround in my case :

When the user clicks a marker, the map gets recentered on this marker. It also seems to trigger another click after panning to recenter. When there's markers close to each others, the click can trigger another marker, so that's the main problem.

ciberado commented 11 years ago

Guys... sorry. I deleted the comment because I still don't know the nettiquette conventions of GitHub and I realized later than the topic was marked as closed. I thought it wouldn't be nice to continue with it. But ey, this is what I've done this afternoon, based on the code found in the links you post:

function preventGhostClick(evt) {
    var lastEventTimestamp = window.lastEventTimestamp || 1;
    var currentEventTimestamp = new Date().getTime();
    var msDifference = currentEventTimestamp - lastEventTimestamp;
    if (msDifference < 20) {
        console.log('We decided not to fire the mouse (event): ' + msDifference + '.');
        evt.stopImmediatePropagation();
        return false;
    } 
    window.lastEventTimestamp = currentEventTimestamp;    
    return true;
}

L.Map.prototype._originalFireMouseEvent = L.Map.prototype._fireMouseEvent;
L.Map.prototype._fireMouseEvent = function(evt) {
    return preventGhostClick(evt) ? this._originalFireMouseEvent(evt) : null;
}

It works. At least I think it works. Of course I'll check the version of the browser before applying the patch, but I need to fix other things before the demo ;-)

HooliganQC, you saved my day! Thanks!

mourner commented 11 years ago

You're always welcome to comment closed tickets if you can add anything to the discussion. :) Thanks for the workaround, lets see if it can/should be used in Leaflet core!

ciberado commented 11 years ago

:+1: :)

Mathbl commented 11 years ago

I'm coming back to you ciberado. Maybe my case is specific, but I can't get your fix to work.

In my code for webview, I have a place where i'm registering click event for markers using marker.on("click", function(e) etc..

Should I put your code inside it? Or elsewhere?

Thanks for your help

xEtherealx commented 11 years ago

I see this same issue on Centos 6, Chrome 26... seems to not be android specific.

ilvalle commented 10 years ago

I've just tested in an android webview with the last stable version of leaflet. The click on a marker works on android 4.1.1, it doesn't work on android 2.3.6.

turbobuilt commented 8 years ago

I have this same issue in both ie and chrome for desktop.

0xae commented 8 years ago

Hello guys, i've found a rather dirty fix for this: I went to leaflet-src.js and comment the lines 6939 to 6943 and the touch worked great in android:

//              if (type === 'click' && L.Browser.android) {
//                  handler = function (e) {
//                      return L.DomEvent._filterClick(e, originalHandler);
//                  };
//              }
                obj.addEventListener(type, handler, false);
0xae commented 8 years ago

The fix (or another name you want to give it) also fixes the touches on the map. so all these issues are gone. I've made tests on my google chrome and it works just fine. @Mathbl @dtruel my app is a ionic / cordova app and it works like a charm now!

pelzerim commented 8 years ago

0xae's fix took care of an ionic issue for me, where leaflet would only recognize every other click event. It works perfectly now.

sturmenta commented 6 years ago

Thanks 0xae! :D It work for me too.