Fastclick.js causes clicks to be lost in leaflet.js

[erjiang]

minimal reproduction example jsfiddle (requires device with touch input): https://jsfiddle.net/y723oet0/2/

Observations: on normal clicks on the map, this.targetElement = null in fastclick.js. If the <a> element is tapped quickly, then this.targetElement = <a>, which causes onMouse to reject the next map click.

I found that adding this.targetElement = null to the end of onTouchEnd() avoids this problem. Will that cause other problems? So far it seems to work for our application.

[ccremeans]

I've also seen the same issue with Fastclick and Leaflet, though only on iPads for some reason. I ended up disabling Fastclick for the button that interfered with the scenario I was running into, though that was far from ideal.

[arkonan]

I've also been seeing lost clicks with Leaflet 0.7.3 and Fastclick 1.0.6 on iOS 8.4 and 9.0 Mobile Safari and UIWebViews. I'm not sure how best to fix the issue but after some debugging I think this is what is happening:

When Fastclick sees a touchend event that it wants to generate a click event for, it does not always clear this.targetElement (https://github.com/ftlabs/fastclick/blob/v1.0.6/lib/fastclick.js#L605-L606), presumably because it expects the browser to generate a click event 300ms later and needs to remember to suppress it. However from my testing on iOS 8.4 and 9 Mobile Safari and UIWebViews, it appears that calling preventDefault() on a touchend event prevents the browser-generated click event. As a result the onClick() handler is not called and this.targetElement is not cleared.

Not clearing this.targetElement is normally not a problem because the onTouchEnd() call to preventDefault() suppresses the browser-generated click events, and forged click events have the forwardedTouchEvent property set on them (https://github.com/ftlabs/fastclick/blob/v1.0.6/lib/fastclick.js#L307). In addition, between touchstart and touchend events, this.trackingClick is true (https://github.com/ftlabs/fastclick/blob/v1.0.6/lib/fastclick.js#L437) which causes the onClick() handler to short circuit (https://github.com/ftlabs/fastclick/blob/v1.0.6/lib/fastclick.js#L684-L688).

Unfortunately Leaflet blocks propagation of touch events on callouts and pins. In some versions it also blocks click events. Because Fastclick listens for clicks in the event capturing phase and touch events in the event bubbling phase, if you have Fastclick attached to a parent of the map (e.g. document.body) it will only see the click events for Leaflet pins and callouts, not the corresponding touchstart and touchend events.

The lost clicks occur if there is a touchstart, touchend and click handled by FastClick on an element outside the map, followed by a click event on a map pin or callout. When the user taps on the map pin/callout, Fastclick's onClick() handler will be called with this.targetElement still set to the element outside the map. Fastclick doesn't realize the event is for a different target, sets this.targetElement to null (https://github.com/ftlabs/fastclick/blob/v1.0.6/lib/fastclick.js#L699) and returns false, canceling the Leaflet click event. Because this.targetElement is now null, subsequent clicks on the map work.

What I ended up doing was to add a second touchstart handler on the event capture phase. This handler behaves basically the same as onTouchCancel() and resets this.targetElement and this.trackingClick. Because this handler runs on the capture phase it cannot be blocked by Leaflet. The reason I'm not providing a pull request is that I'm not sure that there isn't a race condition where rapidly tapping on some browsers might result in extra clicks sneaking through. For my use case that isn't nearly as big a problem as the lost clicks.

Another possible approach would be to verify that the click event's event.target still corresponds to this.targetElement. However I'm guessing the reason the code doesn't do that already is that the target of the click event isn't reliably the same as the one from the touchstart event on all browsers.