Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't disable map drag (Windows IE11 only) #3666

Closed
skalb opened this issue Jul 23, 2015 · 15 comments · Fixed by #3686
Closed

Can't disable map drag (Windows IE11 only) #3666

skalb opened this issue Jul 23, 2015 · 15 comments · Fixed by #3686
Assignees
Milestone

Comments

@skalb
Copy link

skalb commented Jul 23, 2015

I'm trying to disable the map drag when a user clicks a layer. It works fine on Mac/Chrome, but on Windows/IE the map drag is not disabled.

It seems like the mousedown event handler on the map object is never actually removed.

See: http://jsfiddle.net/skalb/otwgyutn/3/

Any ideas?

Edit: using leaflet master

@mourner
Copy link
Member

mourner commented Jul 27, 2015

also cc @danzel

@mourner mourner added the bug label Jul 27, 2015
@IvanSanchez IvanSanchez self-assigned this Jul 29, 2015
@IvanSanchez
Copy link
Member

Hm. Apparently disabling the map dragging works, but the order IE11 fires the events means it's disabled after starting the drag (in that fiddle's use case).

@yohanboniface
Copy link
Member

but the order IE11 fires the events

I'm curious about the details

@skalb
Copy link
Author

skalb commented Sep 9, 2015

Upon further testing, I'm seeing this issue on both iOS Chrome and Safari as well (using iPhone 6 and simulator).

Same fiddle: https://jsfiddle.net/skalb/otwgyutn/5/embedded/result/

@skalb
Copy link
Author

skalb commented Sep 9, 2015

This looks like a different issue so I opened #3825 instead.

@IvanSanchez
Copy link
Member

Apparently this is still reproducible in beta2 (as reported in #4157) - anyone up to git bisect the code?

@IvanSanchez IvanSanchez reopened this Jan 18, 2016
@IvanSanchez IvanSanchez added this to the 1.0 milestone Jan 18, 2016
@amir-infsoft
Copy link

I did some further testing and found that is works correctly in one (sub)version of 1.0beta2 and not correct in another.

There are also fiddles where you can check.

In current version from www.leaflet.js it does not work:
Leaflet: Leaflet 1.0.0-beta.2 ("954e83d")
http://cdn.leafletjs.com/leaflet/v1.0.0-beta.2/leaflet.js
http://jsfiddle.net/xu72htcy/7/embedded/result/

In version I found in one example fiddle it works correctly in IE:
Leaflet: Leaflet 1.0.0-beta.2 ("08d655f")
https://s3.amazonaws.com/fieldwire-dev/leaflet-master/leaflet-src.js
http://jsfiddle.net/xu72htcy/4/embedded/result/

@IvanSanchez
Copy link
Member

@amir-infsoft Does it work in the current master branch? (See https://playground-leaflet.rhcloud.com → "add library" → "Leaflet development version")

@amir-infsoft
Copy link

@IvanSanchez
Copy link
Member

git bisect says that the regression is at c6d8587 AKA «Don't use maxTouchPoints detection in IE11+ (or edge) either, matching our msPointer detection.»

Damn. :-(

@amir-infsoft
Copy link

I see. When I add && navigator.maxTouchPoints part in current dev version then it works.

Can you add this in new version(s)?
I presume this was removed because it was causing some other issue/incorrect behavior?

@IvanSanchez
Copy link
Member

Paging @danzel @yohanboniface - I have no idea how to fix this without breaking #3804/#3839.

I presume this was removed because it was causing some other issue/incorrect behavior?

@amir-infsoft Exactly. It's not the first time that fixing a very specific bug creates a tiny very specific bug somewhere else.

@danzel
Copy link
Member

danzel commented Jan 19, 2016

Adding && maxTouchPoints isn't a fix, that just means it'll only break on touch devices.
That change was sort of a follow up to afa0d1d87b4c522ca3cbeef785c829b886355982 too.

Gonna investigate this for a bit. What if we cancelled any existing drags when the draggable gets disabled?

@danzel
Copy link
Member

danzel commented Jan 19, 2016

Ok, so on mouse down if IE feels like it has a touch device, it will generate a pointerdown event first, which leaflet happily handles starting the drag.
Then IE makes a mousedown event, which you catch and disable the drag.

We could change Draggable.disable() to cancel any drags in progress which would make this work... I think that is probably better behaviour too?

@amir-infsoft
Copy link

@danzel Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants