What is wrong with this functions recursion? (setTimeout issue)

Question

Hi guys,

I am writing a marquee script because I do not like the fact that with most marquee scripts when the marquee reaches the last item (thinking using a ul, so last item is the last li), it waits till that item is off screen, and then resets the position of the ul, and starts the scrolling all over agian.

My approach is to create a clone of the ul, append it after the current ul, and start it scrolling, then delete the original ul once it is completely hidden.

This works almost perfect except for one thing. When you call the function it generates the time required for a setTimout. This setTimout is used to create the new UL and set it moving.

This works fine for the first loop, but then glitches up.

It is a bit hard to explain, but it seems to on the second time the timeout should be called, instead of waiting for the time, it just calls itself instantly. Please see http://webspirited.com/marquee.html for an example

For a working example. The javascript code is as follows:

var count = 0;
$('document').ready(function () {
    //generate some random rows
    for (var i = 0; i <= 20; i++)
    $('.ulscroll').append('<li>Content ' + i + '</li>');
    //add one row so we can see its the last
    $('.ulscroll').append('<li>Last Item</li>');

    //set the ul's width
    var width = 0;
    $('.ulscroll').children('li').each(function () {
        width += $(this).outerWidth();
    });
    log('ul width: ' + width);
    $('.ulscroll').width(width);

    //activate the marquee              
    marquee('.ulscroll', 1, false);
});

function marquee(id, speed, sub) {
    //next two lines debugging purposes only
    count += 1;
    log('Marquee ran ' + count + ' times');

    //store copy of speed sent it(to pass for recursion)
    var s1 = speed;
    //set speed to 10*
    speed = speed * 10;

    //store parent width, and own width (if sub then add on width of parent div)
    var pwidth = $(id).parent('div').outerWidth();
    var width = (sub ? $(id).width() + pwidth : $(id).width());

    //set timeout
    var t = (width - pwidth) * speed;
    setTimeout(function () {
        var clone = $(id).clone().css('left', pwidth);
        $(id).addClass('oldul');
        $(id).after(clone);
        marquee(id + ':not(.oldul)', s1, true);
    }, t);

    $(id).animate({
        left: '-=' + width
    }, width * speed, 'linear', function () {
        $(this).remove();
    });
}

function log(text) {
    $('#log').append('<div>' + text + '</div>');
}

---

Solution
The issue was caused by passing the selector in with :not(.ulold); Here is the revised setTimeout

setTimeout(function(){
  var clone = $(id).clone().css('left', pwidth);
  $(id).addClass('oldul');
  var idx = id.split(':');
  idx = idx[0];
  log('idx: '+idx);
  $(idx).after(clone);
  marquee(idx+':not(.oldul)', s1, true);
},t);

Answer 1

The problem, most likely, lies with this code..

    var clone = $(id).clone().css('left', pwidth);
    $(id).addClass('oldul');

The fact that you are re-using the selector messes things up because the original selector matches a lot of things as time progresses..

In the initial call use marquee('.ulscroll:not(.oldul)', 1, false);

Answer 2

Learn to use a good debugger, such as Firebug. You'll see that eventually the animation end callback (the one that removes the elements matching the selector) gets called before the anonymous timeout function (the one that calls marquee).

An issue with the code on the sample page (but not in the posted code): tout is local to each invocation of marquee. The clearTimeout(tout) call does nothing.

Lastly, please don't use a marquee. It has usability problems: moving text is harder to read, causes distraction, and will always move either too quickly for some readers or too slowly for some readers.