Leaflet / Leaflet

🍃 JavaScript library for mobile-friendly interactive maps 🇺🇦
https://leafletjs.com
BSD 2-Clause "Simplified" License
41.24k stars 5.82k forks source link

Click event when two-finger zooming on iPhone #7465

Closed darrenklein closed 2 years ago

darrenklein commented 3 years ago

Hello, thanks so much for all of your hard work on this project. I am reporting a bug that I've encountered when using Leaflet maps on an iPhone (perhaps it affects iOS in general, but I don't have access to an iPad, so I can't say there) - essentially, when pinch/two-finger zooming in or out of a map that has a click event listener, a click event will be fired in between the zoomstart and zoomend events. I tested this on two Android devices as well (a Samsung phone and tablet) and was not able to reproduce the bug - they seem to handle pinch/two-finger zooming as expected, no additional click events are fired.

Fortunately, it is very easy for someone using this library to create a workaround for this issue - use the zoomstart and zoomend events to keep track of the zooming state and ignore any click events detected while the map is zooming - but I thought you'd appreciate having this documented.

I'll provide as much info as I can about this, but please don't hesitate to let me know if there's anything else that I can help you with diagnosing or understanding this behavior.

Steps to reproduce Steps to reproduce the behavior:

map.on('click', () => {
    // do something...
})

Expected behavior I expect zooming in and out to not register any events that are not directly related to zooming, especially click events.

Current behavior When zooming in and out via the two-finger pinch method on an iPhone, a click event is being fired, calling the click event callback. In my specific use case, I add a marker to the map on click, and so zooming in and out adds and moves a marker with each zoom. I was able to replicate this behavior on Safari, Chrome, and Firefox browsers on an iPhone.

This does not appear to affect Android devices.

Environment

Additional context

Minimal example reproducing the issue

https://plnkr.co/edit/K0zFAtZDzez9ebaP

(The click behavior is just to log a line to the console, so you'll need to plug an iPhone into a Mac and use Safari's Develop tool to get the phone browser's console)

johnd0e commented 3 years ago

Duplicate of #7255?

darrenklein commented 3 years ago

Hm, I don't quite think it's a duplicate of #7255. In the course of the project I'm working on, I encountered that issue as well, but that only seems to affect desktop Safari. The issue I'm reporting is on iOS, and occurs on at least the three browsers I mentioned, Safari, Chrome, and Firefox.

johnd0e commented 3 years ago

It's easy to check, just create map with option {tap: false}.

darrenklein commented 3 years ago

I tried it out creating the map like so

var map = new L.Map('leaflet', {
    layers: [
        new L.TileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
            'attribution': 'Map data © <a href="http://openstreetmap.org">OpenStreetMap</a> contributors'
        })
    ],
    center: [0, 0],
    zoom: 0,
    tap: false
});

still saw the behavior.

johnd0e commented 3 years ago

Well, I do not have iOS device to check this, so try to debug it yourself. You might start from inspecting call stack. E.g. attach click listener with console.warn(event);, and post log here.

darrenklein commented 3 years ago

Ok, here's what I tried - tested this on both Chrome and Safari on my iPhone, same results in both cases.

First off, I added both a console.warn(event) as well as a try/catch where I threw an error so I could get the stack trace -

map.on('click', (event) => {
    console.warn(event)

    try {
       throw new Error()
    } catch(e) {
      console.log(e.stack);
    }
});

When I did a two-finger zoom, here's what I saw -

[Warning] {originalEvent: MouseEvent, containerPoint: Object, layerPoint: Object, latlng: Object, type: "click", …} (index.js, line 14076)

[Log] http://000.000.00.00:3000/scripts/index.js:14079:20 (index.js, line 14081)
fire@http://000.000.00.00:3000/scripts/index.js:589:17
_fireDOMEvent@http://000.000.00.00:3000/scripts/index.js:4404:21
_handleDOMEvent@http://000.000.00.00:3000/scripts/index.js:4361:23

Interestingly, when I tried just a one-finger click on the map, I got a "double" event -

[Warning] {originalEvent: MouseEvent, containerPoint: Object, layerPoint: Object, latlng: Object, type: "click", …} (index.js, line 14076)

[Log] http://000.000.00.00:3000/scripts/index.js:14079:20 (index.js, line 14081)
fire@http://000.000.00.00:3000/scripts/index.js:589:17
_fireDOMEvent@http://000.000.00.00:3000/scripts/index.js:4404:21
_handleDOMEvent@http://000.000.00.00:3000/scripts/index.js:4361:23
dispatchEvent@[native code]
_simulateEvent@http://000.000.00.00:3000/scripts/index.js:13831:27
_onUp@http://000.000.00.00:30000/scripts/index.js:13804:26
_handlePointer@http://000.000.00.00:3000/scripts/index.js:2103:11
onUp@http://000.000.00.00:3000/scripts/index.js:2122:19

[Warning] {originalEvent: MouseEvent, containerPoint: Object, layerPoint: Object, latlng: Object, type: "click", …} (index.js, line 14076)

[Log] http://000.000.00.00:3000/scripts/index.js:14079:20 (index.js, line 14081)
fire@http://000.000.00.00:3000/scripts/index.js:589:17
_fireDOMEvent@http://000.000.00.00:3000/scripts/index.js:4404:21
_handleDOMEvent@http://000.000.00.00:3000/scripts/index.js:4361:23

(Note that the second stacktrace in the click case is the same as the stacktrace for the rogue click event in the zoom case)

This does make me think that this issue is in fact related to (or a duplicate of) #7255 - however, here we can see it occurring on three mobile browsers on iPhone, not just on desktop Safari. I'm going to make an assumption that events that produce the four line stacktrace are these extra clicks, and it seems that they are able to affect touch interactions other than clicks.

johnd0e commented 3 years ago
  1. 7255 can be neutralized by tap:false (do it).

  2. Show all fields of event.originalEvent.
  3. Describe in detail how that clicks occured: on touch start, or during pinch zoom, of after touch end.
darrenklein commented 3 years ago

Ok, I set tap: false like so:

const map = new L.map('map', {
    tap: false
}).setView([51.505, -0.09], 13);

When zooming in, I do still get the click event, here is the content of event.originalEvent:

MouseEvent {
  altKey: false,
  bubbles: true,
  button: 0,
  buttons: 0,
  cancelBubble: false,
  cancelable: true,
  clientX: 521,
  clientY: 204,
  composed: true,
  ctrlKey: false,
  currentTarget: null,
  defaultPrevented: false,
  detail: 1,
  eventPhase: 0,
  fromElement: null,
  isTrusted: true,
  layerX: 513,
  layerY: 196,
  metaKey: true,
  offsetX: 513,
  offsetY: 196,
  pageX: 521,
  pageY: 204,
  relatedTarget: null,
  returnValue: true,
  screenX: 521,
  screenY: 204,
  shiftKey: false,
  srcElement: <div id="map">,
  target: <div id="map">,
  timeStamp: 10173,
  toElement: <div id="map">,
  type: "click",
  view: Window {listeners: Object, NaN: NaN, Infinity: Infinity, window: Window, undefined: undefined, …},
  webkitForce: 1,
  which: 1,
  x: 521,
  y: 204
}

In order to test the sequence of events, I added a few console logs like so:

map.on('zoomstart', () => {
    console.log('zoomstart');
});

map.on('zoomend', () => {
    console.log('zoomend');
});

map.on('click', () => {
    console.log('click');
});

I observed that, when initially touching the map with two fingers, I'd see zoomstart log to the console. I'd then spread them apart to zoom the map - as soon as my fingers came off the map, I'd see click logged, followed immediately by zoomend. However, this only seems to happen if the entire process of initial contact, spread, and release takes less than approximately 3 seconds - if I made the process take longer, the click event didn't fire. I tested this by extending each of the three phases separately - holding my fingers on longer after initial contact but before moving; starting to move immediately but moving them slowly; starting the process immediately and zooming quickly, but pausing before releasing - so it does seem to be time-sensitive.

johnd0e commented 3 years ago

https://rbyers.github.io/eventTest.html

Try click here. 1) without moving finger 2) with some moving

Dou you see click events in both cases?

darrenklein commented 3 years ago

I believe so? It's a little hard to tell from the output that's written to the UI, but there seems to be a click event in both cases - here are two screenshots, the first is click only, the second was click and move -

click click_and_move

johnd0e commented 3 years ago

There is Config where you can disable all event types except mouse. Anyway, I do not see click on second screenshot.

So I have no idea why it may occur on leaflet map container in similar conditions..

Try also such setup: Screenshot_20210214125442.png (Confirm it with OK button)

darrenklein commented 3 years ago

Sorry, that makes sense. Anyway yes, I disabled everything except for mouse events (and then a second test with touchstart and touchmove enabled), and in both cases the click behavior was registered but the click and drag behavior was not. I also tested enabling gestures - those seemed to correctly log the two-finger zoom behavior, but no additional click event was added in between.

johnd0e commented 3 years ago

Have you noted preventDefault title of config section?

darrenklein commented 3 years ago

Yes - not sure what you mean by that. If there are specific configurations that you'd like me to test, please let me know.

Falke-Design commented 2 years ago

@darrenklein Can you please check out the latest beta https://github.com/Leaflet/Leaflet/releases/tag/v1.8.0-beta.3 and see if it is resolved?

darrenklein commented 2 years ago

@Falke-Design I'd love to be able to test this in my project, but I'm having a bit of a difficult time getting the dependency installed in my existing project - my apologies, I'm actually not much of a front-end dev, and the intricacies of yarn and the nested dependencies of NPM modules are all a little mysterious to me. My project has several dependencies that themselves all depend on Leaflet, so trying to install a particular tag of Leaflet is proving difficult, giving me a lot of this:

 ERROR  Failed to compile with 26 errors                                                                                                                           11:07:07 PM

This dependency was not found:

* leaflet in ./node_modules/vue2-leaflet/dist/components/LTileLayer.js, ./node_modules/vue2-leaflet/dist/components/LControlScale.js and 24 others

To install it, you can run: npm install --save leaflet

Maybe there's an easier way that I could test that out, since it seems like the dependency tree in my existing project is a little cluttered? Is there any way I could install that particular tag in a sandbox like Plunkr or something, or a really bare-bones Node project you could direct me to that I could use to quickly install it?

Thanks and my apologies - I'd love to help confirm the state of this issue, but I don't work much in the Node/NPM/Yarn world, so I'm a little out of my depth.

Falke-Design commented 2 years ago

No problem, let us try it with plunkr. First your sample, does it still fire the click event?: https://plnkr.co/edit/K0zFAtZDzez9ebaP?preview Then the 1.8.0-beta.3 Version:https://plnkr.co/edit/v3bfqlGQ20OQUXyH

darrenklein commented 2 years ago

@Falke-Design thank you! Yes, I can confirm that this issue has been resolved in the 1.8.0-beta.3 release. I tested Safari, Chrome, and Firefox on iOS, but could not reproduce the issue (it did still occur on 1.7). Many thanks to you and the Leaflet team!

Falke-Design commented 2 years ago

Nice thank you for testing!