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.
Comment | File | Size | Author |
---|---|---|---|
#12 | jcarousel-dom-id-unique-1493958-12.patch | 682 bytes | jts86 |
#7 | jcarousel-dom-id-unique-1493958-3.patch | 676 bytes | afmangum |
#3 | jcarousel-dom-id-unique-1493958-2.patch | 889 bytes | ralf.strobel |
#2 | jcarousel-dom-id-hash-1493958-1.patch | 1.21 KB | ralf.strobel |
Comments
Comment #1
ralf.strobel CreditAttribution: ralf.strobel commentedMaybe 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.]
Comment #2
ralf.strobel CreditAttribution: ralf.strobel commentedOk, 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.
Comment #3
ralf.strobel CreditAttribution: ralf.strobel commentedThought 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:
Comment #4
Rob_Feature CreditAttribution: Rob_Feature commentedNote 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....
Comment #5
bdimaggioRalf, thanks so much for posting this. Your bug-tracking and patch saved my butt on a high-profile client's site.
Comment #6
afmangum CreditAttribution: afmangum commentedralf.strobel rocks!
Comment #7
afmangum CreditAttribution: afmangum commentedActually 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:Comment #7.0
afmangum CreditAttribution: afmangum commentedRemoved statement about HTML body output, since this gets rendered according to js settings in browser.
Comment #8
John Franklin CreditAttribution: John Franklin commentedPatch in #7 -- hopefully -- fixed it for me.
Comment #10
markpavlitski CreditAttribution: markpavlitski commentedFixed in 7.x-2.x-dev branch.
Comment #12
jts86 CreditAttribution: jts86 commented#7 failed for me. Fixing tabs.