search

instantpage / instant.page

Make your site’s pages instant in 1 minute and improve your conversion rate by 1%
https://instant.page
MIT License
6.04k stars 205 forks source link

n.target.closest is not a function #66

Closed kkmuffme closed 2 years ago

kkmuffme commented 4 years ago

Followup to https://github.com/instantpage/instant.page/issues/58 and https://github.com/instantpage/instant.page/issues/23

In 3.0.0 it's n.target.closest is not a function

The issue happens in Safari 13.0.5 on iOS 13.3.1 on iPhone Only with the "mouseover" event - perhaps as the mouseover event has some weird implementation on iOS (in that version?) in certain use cases (perhaps with external input device?)

How to fix this: in function mouseoverListener(event) { line 119 of instantpage.js add:

if ( ! 'closest' in event.target ) {
    return;
}
digitaljhelms commented 4 years ago

@dieulot any chance this will get fixed? I have 71k of these events logged in Sentry, impacting 59k users. 🤞

thenationofalex commented 3 years ago

Is there any update on this? I'm experiencing the same issue with Sentry reporting the same error.

dieulot commented 3 years ago

@digitaljhelms @thenationofalex Could you (or anyone else) tell me on which pages, and if any with which specific browsers, this happens? Here or in private. I’ve been reluctant to fix it with a check on event.target/event.target.closest as I’d like to understand why that happens (why event.target isn’t an element) in the first place.

kkmuffme commented 3 years ago

Used to be iOS only, getting it from Chrome (e.g. Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.131 Safari/537.36 ) now too. However this may be bots with fake user agents though.

dieulot commented 3 years ago

My best guess is that in those cases event.target is document. null/undefined would throw different.

I’m not familiar with error monitoring systems. Would such errors (constant prefix, variable text) be able to be searched for without too much trouble?

Uncaught TypeError: instant.page non-element event target: timeStamp=864, type=mouseover, typeof=object, nodeType=1, nodeName=HTML, viewport=1280x361, coords=604x316+0

throw new TypeError(`instant.page non-element event target: timeStamp=${~~event.timeStamp}, type=${event.type}, typeof=${typeof event.target}, nodeType=${event.target.nodeType}, nodeName=${event.target.nodeName}, viewport=${innerWidth}x${innerHeight}, coords=${event.clientX}x${event.clientY}+${scrollY}`)

I plan to push a version that includes this tomorrow if there’s no objection by then.

Hopefully the event’s coordinates would point to what kind of elements/interactions cause this. I couldn’t find anything on the web (nor in jQuery’s source code) about event.target sometimes not being an element, and I failed to grasp Chromium source code’s event logic.

Allain55 commented 2 years ago

Is there any update on this?

digitaljhelms commented 2 years ago

It's blowing my mind that, with an open PR, this hasn't been resolved in the last 2 years. Just merge the PR for crying out loud, there's no mouse events on mobile/table devices.

dieulot commented 2 years ago

This is fixed (with a check on event.target.closest) in v5.1.1. https://github.com/instantpage/instant.page/commit/00249af912beabe77ecd902ebcef190d698adc81