For whatever reason, some browsers were having problems in jCarosel.js (line 24):
for (var objectParent in callbackParents) {
callbackFunction = callbackFunction[callbackParents[objectParent]];
}

I've not experienced this before, having used the component multiple times. On checking the code, it seems to be wanting to run through the callbackParents with the "filter" and "indexOf" properties of the object. They didn't seemed to fit. So code was wrapped around the actioning code to prevent "filter" and "indexOf" being used in the index lookup.

for (var objectParent in callbackParents) {
if (!isNaN( parseInt( objectParent ) )) {
callbackFunction = callbackFunction[callbackParents[objectParent]];
}
}

This allowed the code to work in the browsers() I was having problems with.

CommentFileSizeAuthor
#5 jcarousel_each_replacements.patch1.58 KBquicksketch

Comments

quicksketch’s picture

quicksketch
he/him
commented
Priority: Critical » Normal

Could you clarify how to reproduce the issue? I'm not clear on if this is a site-specific issue or if it can be reproduced with some specific configuration of the jCarousel module without any customizations/add-ons.

Psi-factor’s picture

Psi-factor commented

I have some problem. When i put 2 views with carousel style in IE8 ( ie7 mode) browser ger a freez. With only one views - works good.
gregnelson patch do nothing for my situation.

dscutaru’s picture

dscutaru commented

Had same issue, thanks gregnelson for the tip.
Drupal 7.7, jcarousel 7.x-2.4-alpha3, front page, one carousel.
In mytemplate_jcarousel_options_alter() I've modified options; next & prev buttons set to NULLs, initCallback & itemFirstInCallback set.
Had js errors in IE 7 and 6, no problems in IE 8 and in other browsers (didn't try safari).

quicksketch’s picture

quicksketch
he/him
commented

I've seen some similar errors in other issue queues due to a conflict with ExtJS. We apparently shouldn't be using the for (var n in y) construct because a lot of JS libraries and code feel the need to extend the normal JavaScript object or array types to provide "extra features", which breaks massive amounts of code across the internet but they don't care because our approach isn't "proper". In any case, we can avoid this problem by switching to jQuery's $.each() method instead, even if it's more verbose.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Fixed
StatusFileSize
new jcarousel_each_replacements.patch1.58 KB

This patch should avoid errors like this in the future by using jQuery's safer, $.each() method instead of the for/in approach, which may loop over things that are not properties in objects and arrays.

Committed to both 2.x branches.

quicksketch’s picture

quicksketch
he/him
commented
Title: IE8/7, Safari, plus issue. [with fix] » Loop through callbackParents fails in IE8/7 and Safari when combined with other JS libraries

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

ollyharkness’s picture

ollyharkness commented

What's happening is that a different module has changed the Object prototype to include indexOf (might be borealis.js) taking effect over the whole project - this means that when you iterate through an Object using a 'for in' loop and don't include the check

if(callbackParents.hasOwnProperty(objectParent)){}

then you're in trouble - also I don't know if callbackParents is an Array or Object. To use a 'for in' loop over an array is wrong.