I have a page with six carousels on them (four vertical, two horizontal). At random intervals, the first carousel on the page suddenly gets rendered all messed up (vertical instead of horizontal), forcing me to clear the render cache manually.

I found that in the JS settings (Drupal.settings.jcarousel.carousels), there should normally be six entries (jcarousel-dom-1 ... jcarousel-dom-6). But when the error occured, jcarousel-dom-5 was missing and its content had instead overwritten jcarousel-dom-1.

My suspicion is that under some circumstances, jCarousel fails to recognize how many carousels already exist and so it begins at 1 again, overwriting the first set of settings. Maybe this happens when the cache entries of the carousel blocks don't expire at the same time, so that the carousels are not re-rendered at the same time.

If there is no way to prevent this automatically, maybe there could be an option to assign a number value to each carousel manually in the config, in order to resolve such conflicts.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ralf.strobel’s picture

Maybe the best way would be to no longer use incremental numbers for the JS entries but some kind of unique identifier, based on the view name and display.

[PS: I removed the statement about wrong HTML body output from the issue opener, since I realized the css classes are assigned by javascript in the browser, according to the JS settings.]

ralf.strobel’s picture

Title: Secondary carousels on page sometimes override js settings of first » Collisions in js settings when using multiple cached carousels on same page
Status: Active » Needs review
FileSize
1.21 KB

Ok, since nobody wanted to look into this for a while, I have now written the fix myself. So far, it seems that my suspicion was correct and this happened when the cache of one carousel expires earlier than the others.

Instead of using the static variable counter, I now generate the carousel DOM IDs as a hash string based on the view/display name and arguments. That way they should be unique to each output.

I've made a patch file for you. However, I didn't clone the original repository, so you might have to enter the changes manually if there are conflicts. They are actually quite small.

ralf.strobel’s picture

Thought about it again. We don't actually need the arguments to identify each carousel but only view and display name. The hashing I did was also not really necessary. We can just use the plain string.

So here's an even simpler patch that basically makes this a one line change:

function jcarousel_views_add($view, $display_id = NULL) {
  //static $dom_counter = 0;
/*...*/
    'jcarousel_dom_id' => isset($view->jcarousel_dom_id) ? $view->jcarousel_dom_id : $view->name . '_' . $display_id,
/*...*/
}
Rob_Feature’s picture

Note that this doesn't just happen when caching...it was happening to me when I simply had all 3 of my carousels (which all appear on the same page) in the same View (each as a block). They seem to inherit the same ID even without caching on.

This solution seems to be a good idea in general. Worked for me on a D6 install....

bdimaggio’s picture

Ralf, thanks so much for posting this. Your bug-tracking and patch saved my butt on a high-profile client's site.

afmangum’s picture

afmangum’s picture

Actually ran into a problem there with multiple carousels from the same view (using parameters passed to the view to get different results), which the $dom_counter variable solves. So, leaving that var alone and changing the other line to the following works:

'jcarousel_dom_id' => isset($view->jcarousel_dom_id) ? $view->jcarousel_dom_id : $view->name . '_' . $display_id . '_' . ++$dom_counter,
afmangum’s picture

Issue summary: View changes

Removed statement about HTML body output, since this gets rendered according to js settings in browser.

John Franklin’s picture

Patch in #7 -- hopefully -- fixed it for me.

markpavlitski’s picture

Issue summary: View changes
Status: Needs review » Fixed

Fixed in 7.x-2.x-dev branch.

Status: Fixed » Closed (fixed)

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

jts86’s picture

#7 failed for me. Fixing tabs.