jsor / jcarousel

Riding carousels with jQuery.
https://sorgalla.com/jcarousel/
MIT License
1.99k stars 734 forks source link

jcarousel stops working on IE with Jquery version 3.2.0 and above #832

Closed psun888 closed 6 years ago

psun888 commented 6 years ago

jcarousel stops working on IE with JQuery version 3.2.0 and above, only showing the first slide, neither next-button nor pre-button working.

Tested: JQuery 3.0.0, working JQuery 3.1.1, working JQuery 3.2.0, not working JQuery 3.2.1, not working JQuery 3.3.1, not working

jsor commented 6 years ago

Could you setup a reproducible test case and tell the IE version you're using?

psun888 commented 6 years ago

Jan, I tried both IE11 on Windows 10 and Windows Server 2016. To reproduce the problem, you can simply change the line of JQuery to version JQuery 3.2.0 or above. Or using the existing CDN links: https://code.jquery.com/jquery-3.2.0.js https://code.jquery.com/jquery-3.2.1.js https://code.jquery.com/jquery-3.3.1.js

Either one of them, when testing with IE11, only the first image showing, and you cannot go to the rest of slides. Testings under both FireFox and Chrome are working.

petercooperjr-spls commented 6 years ago

I can confirm I'm having this issue as well. I tried upgrading my version of jQuery to 3.3.1 and carousels stopped working on IE 11 (Windows 7) and work in other major browsers. Downgrading jQuery to 3.1.1 seems to fix the problem.

graymatter00 commented 6 years ago

I am also seeing this issue after upgrading an ASP.Net MVC app to jQuery 3.3.1. Running Windows 10 latest updates. Works fine on Edge and Chrome. However, IE11 has the issue.

eat-sleep-code commented 6 years ago

I can confirm this issue on IE 11 with 3.3.1. It appears to work okay in Edge and Chrome and Safari.

jsor commented 6 years ago

I think i was able to track this down. Looks like the reason is a change in how jQuery handles width() and height() since 3.0.0 (https://jquery.com/upgrade-guide/3.0/#breaking-change-width-height-css-quot-width-quot-and-css-quot-height-quot-can-return-non-integer-values) in combination with "bug fixes" for the 3.2.0 release (i was not able to track down whta changes exactly).

That resulted in width() returning a negative value here and jCarousel running in vertical mode: https://github.com/jsor/jcarousel/blob/8712f87e7a3c7436f4f63ebe60d966d69a9d8ef1/src/core_plugin.js#L90

I've fixed that in 72c9c15734c12791a368b5f00e708cc22301bba5. Can anyone confirm that this fixes the bug by using a patched dist file from here: https://github.com/jsor/jcarousel/tree/8712f87e7a3c7436f4f63ebe60d966d69a9d8ef1/dist?

graymatter00 commented 6 years ago

I have downloaded and tested the new version as requested. I can confirm that the jcarousel in my web app now works as expected in IE11 and continues to work as expected in both Edge and Chrome. :)

I'm running Windows 10 Pro (64 bit) latest production version and updates. Chrome is also the latest 64 bit version.

Thanks for the quick fix.

psun888 commented 6 years ago

Thanks Jan, it's now perfectly working with version 3.3.1. I tested it with IE11 on Windows 10 and Windows Server 2016, and also all other major browsers. I leave it open for 2 days, so others can test it thoroughly.

petercooperjr-spls commented 6 years ago

Confirmed for me as well. I'm using jquery.jcarousel.min.js from that 8712f8 tree dist, with jQuery 3.3.1, and carousels on our site now work in IE11 (Windows 7).

Are files in that 8712f8 tree dist "production-ready", or is there some difference between them and a "real" release that means I should wait before deploying?

Thank you.

jsor commented 6 years ago

I realized that this is a breaking change for square vertical carousels where jCarousel now detects that carousel as horizontal.

Looks like the default 20000em set as width for list can't be handled by IE and reports a negative value (both via getBoundingClientRect() and in the console)

ie

So, a better fix is to replace all occurences like

https://github.com/jsor/jcarousel/blob/5cf7575fd9f4cd283c9dca8287cef36bb64c5035/examples/basic/jcarousel.basic.css#L38

with a lower width value, eg. 10000em which seems to be handled fine by IE.

It would be great if you could test again with the latest released version from https://github.com/jsor/jcarousel/tree/0.3.5/dist (or whatever version you used) and changing the relevant CSS in your stylesheets. Thanks!

psun888 commented 6 years ago

Jan, this way works much better:) I tested it with IE 11 under Windows 10 and Windows Server 2016, both are working without any issue, also Firefox and Chrome are still working well. Really appreciate your hard work, cheers!

graymatter00 commented 6 years ago

I can also confirm that using the release version of jCarousel, with the latest jQuery, and changing the width to 10000em works with IE11, Edge and Chrome. That is, jCarousel works as expected in all the browsers. Thanks!

I don't know if this is meaningful or helpful, but I did some testing and found that jCarousel in IE11 stops working when the 'width' is set greater than 13421em - at least that's what happens in my web app. That is, if width<=13421em, jCarousel works in IE11; if width>=13422em, jCarousel does not work in IE11.

Thanks again for your work in providing and maintaining jCarousel.

jsor commented 6 years ago

Thanks for the confirmation. I've released 0.3.7 which also includes some hardening against width() / height() potentially returning non-numeric values. The main change regarding this issue is changing the list width from 20000em to 10000em in the examples and documentation. That means, upgrading is optional if you changed that in your stylesheets.

FYI: i've released 0.3.6 before but unfortunately introduced a regression, so you should skip that version.

eat-sleep-code commented 6 years ago

@jsor When will 0.3.7 be pushed to NPM?

jsor commented 6 years ago

Sorry, it's published now.

eat-sleep-code commented 6 years ago

@jsor tried changing the value of the em, doesn't seem to have fixed the IE11 issue for me.

jsor commented 6 years ago

@eat-sleep-code The carousels work perfectly for me in IE 11. What's exactly the error you're experiencing?

eat-sleep-code commented 6 years ago

Darn it. It must have been some nasty caching issue or some issue with my work computer. Works fine here at home on IE11 now. Weird.

Thanks. Issue resolved.

jsor commented 6 years ago

Great, closing for now. Feel free to reopen if needed.