search

nicolas-cusan / breakpoint-helper

Small helper library to work with layout breakpoints in Javascript.
https://nicolas-cusan.github.io/breakpoint-helper/
MIT License
18 stars 2 forks source link

[Bug] On Safari 13 (ios 13) and earlier #7

Closed merobal closed 2 years ago

merobal commented 2 years ago

We found a bug which occurs on old safari (unfortunately few of our users use old version and don't want to upgrade...)

Tested via https://www.lambdatest.com/

The error in production: TypeError: n.addEventListener is not a function. (In 'n.addEventListener("change",t)', 'n.addEventListener' is undefined)

And the code context indicated that the source of the issue is this line: https://github.com/nicolas-cusan/breakpoint-helper/blob/master/src/index.js#L192 (mq can be undefined.)

I put our listener = listenAll(callback); line inside a try-catch and our prod issue disappeared.

Can you please fix this issue (add an undefined check for mq would be enough for us) and release?

nicolas-cusan commented 2 years ago

Hey @merobal, thank you for filing this issue.

I am pretty busy at the moment, but if you file a PR I will consider merging it.

Just out of curiosity: what code are you using to invoke the instance? What version iOS version has this error as window.matchMedia is supported since iOS/safari 5.

merobal commented 2 years ago

We use the code I showed at #4

Not sure why the bug occurred, I was able to reproduce at online tools (which are slow and limited in free version) and unfortunately I don't have an old ios device nearby for further investigation.

I created a PR.

merobal commented 2 years ago

Hi @nicolas-cusan did you have time to take a look at the PR?

nicolas-cusan commented 2 years ago

Hi @merobal I added some feedback to the PR. In any case I tested the library on iOS 13 via Browserstack and did not get any errors. Any chance you can provide more information about the iOS version?

My guess is, if there is an error in old browsers, that these versions do not support addEventListener on MediaQueryList and the old and now deprecated addListener needs to be used. Maybe you could try that test it and update the PR accordingly (will add this comment there too).

nicolas-cusan commented 2 years ago

Hi @merobal I just released a new version that catches the error. Please test it and report back. Thank you!

merobal commented 2 years ago

Thank you! I tested the code that crashed with 1.0.3 and works good with 1.0.4 without try-catch at our side. 🎉

Also it didn't happen on first page-load, it happened when the user navigated to a new screen inside react scope (like after a "start screen" before a survey, then it crashed for some reason). Tested with virtual iphone x device and ios 13.0.