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

Access of undefined property when dragging the map in IE11 #2102

Closed
fnicollet opened this issue Oct 17, 2013 · 16 comments
Closed

Access of undefined property when dragging the map in IE11 #2102

fnicollet opened this issue Oct 17, 2013 · 16 comments
Assignees
Labels
blocker Critical issue or PR that must be resolved before the next release
Milestone

Comments

@fnicollet
Copy link
Collaborator

Hi,

I have just been trying leaflet-master (18 Oct 2013) in my project, which is a Windows 8 application, that is running into IE11.

Microsoft unprefixed the "MSPointerXXX" events t more standard pointerxxx events, see "MS vendor prefix removal":
http://msdn.microsoft.com/en-US/library/windows/apps/dn263112.aspx

I saw that there was a commit 14 days ago about this issue:
6e3e0d9

But when i init a simple map and click the map, it crashes. The problem is in
L.DomEvent, at this line:

if (L.Browser.pointer && type.indexOf('touch') === 0) {

here, type is undefined.

Here is the call stack:
addListener [leaflet-src.js] Line 6332
_onDown [leaflet-src.js] Line 6647
handler [leaflet-src.js] Line 6329
cb [leaflet-src.js] Line 7152

Basically, before that, it goes in the "_onDown" function from Draggable.js:
https://github.com/Leaflet/Leaflet/blob/master/src/dom/Draggable.js
Especially at this line:

L.DomEvent
            .on(document, L.Draggable.MOVE[e.type], this._onMove, this)
            .on(document, L.Draggable.END[e.type], this._onUp, this);

The "e.type" is "pointerdown" at this point, so it cannot find it in the static map defined like this:

statics: {
                START: L.Browser.touch ? ['touchstart', 'mousedown'] : ['mousedown'],
                END: {
                        mousedown: 'mouseup',
                        touchstart: 'touchend',
                        MSPointerDown: 'touchend'
                },
                MOVE: {
                        mousedown: 'mousemove',
                        touchstart: 'touchmove',
                        MSPointerDown: 'touchmove'
                }
        },

Adding "pointerdown" to both lists fixes it, from what i tested (dragging the map, dragging markers)

END: {
            mousedown: 'mouseup',
            touchstart: 'touchend',
            pointerdown: 'touchend',
            MSPointerDown: 'touchend'
        },
        MOVE: {
            mousedown: 'mousemove',
            touchstart: 'touchmove',
            pointerdown: 'touchmove',
            MSPointerDown: 'touchmove'
        }

Fabien

@danzel
Copy link
Member

danzel commented Oct 18, 2013

will look at this one too.

@mourner
Copy link
Member

mourner commented Oct 18, 2013

@fnicollet wow, thanks for the report! I hope that only affects tablets. Or is it broken on desktop IE11 too?

@fnicollet
Copy link
Collaborator Author

@mourner Note sure, i tried the map on the leaflet website in IE11 Desktop and i could pan the map but it is not using master so i don't know if that's like a false positive. I'm at work atm on Windows 7 so i can't test it yet but will tell you later on

@mourner
Copy link
Member

mourner commented Oct 18, 2013

@fnicollet added your fix to master, could you please retest both this issue and #2103, preferably on both desktop IE11 and tablet with touch?

@fnicollet
Copy link
Collaborator Author

Cool, thanks
I will try in desktop IE11, see how it behaves. Also, this fix does not fix #2103 in my testing so it should be reopened. I couldn't get to the root of the #2103 issue but it looks like it is something totally different

@mourner
Copy link
Member

mourner commented Oct 18, 2013

@fnicollet yeah, I didn't close that one yet :)

@fnicollet
Copy link
Collaborator Author

ah yes, it was a reference to the closed one, read too quick :)

@fnicollet
Copy link
Collaborator Author

Also broken in IE11 Desktop with master. The fix do work for dragging but multi-touch is still broken. I am going to try and investigate that one further

@fnicollet
Copy link
Collaborator Author

Strange thing, the map on http://leafletjs.com/ that uses stable version works ok when dragging but multitouch is also broken

@mourner
Copy link
Member

mourner commented Oct 18, 2013

You mean the master is broken in IE11 when interacting with touch, and works OK with mouse, right?

@fnicollet
Copy link
Collaborator Author

master: broken "simple" touch (dragging) and broken multitouch. The fix only fix the dragging behaviour, multitouch still broken
stable: ok "simple" touch (dragging) and broken multitouch. The fix doesn't change anything

@fnicollet
Copy link
Collaborator Author

Ok, first thing I just discovered, this test

var ie = !!window.ActiveXObject

return false in IE11. Even tho, using Visual Studio, if get as a value for it:

function ActiveXObject() {
    [native code]
}

Not sure why "!!" doesn't return true when it is a function? I tried it in IE9 and IE10, this test returns true.

Then the next test:

ie3d = ie && ('transition' in doc.style)

return false because "ie" is false and that's what is breaking multitouch on IE

@mourner
Copy link
Member

mourner commented Oct 18, 2013

Crazy Microsoft, lol...

@fnicollet
Copy link
Collaborator Author

Here is a forum post about this change:
http://social.technet.microsoft.com/Forums/ie/en-US/531252d3-cae6-4b7c-8747-6f3a77be77e4/internet-explorer-11-preview-activex-not-loading
The test needs to be changed to

"ActiveXObject" in window

returns true in any version of IE

@fnicollet
Copy link
Collaborator Author

ok, this totally fix the multitouch issues as the browser is then considered as "any3d", so transitions and so on work

mourner added a commit that referenced this issue Oct 18, 2013
@mourner
Copy link
Member

mourner commented Oct 18, 2013

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Critical issue or PR that must be resolved before the next release
Projects
None yet
Development

No branches or pull requests

3 participants