3

I have the following jQuery code:

var id_atual
var temp_id
var tempo_flash = 50
var $slide_atual = $('#slider .atual')
var $slide_prox = $('#slider .ocultar')
setInterval(function(){
    id_atual = $slide_atual.attr('alt')
    $.post('get_banner.php', {atual: id_atual}, function(proximo){
        temp_id = proximo.split(';;')
        $slide_prox.attr('src', temp_id[0]).load(function(){
            $slide_atual.hide('fade', tempo_flash, function(){
                $slide_atual.attr('alt', temp_id[1]).attr('src', temp_id[0]).load(function(){
                    $slide_atual.show('fade', tempo_flash)
                })
            })
        })
    })
}, 4000)

And the following HTML code:

<div id="slider">
    <img src="imagens/slider/imagen-slider.jpg" alt="1" class="atual"/>
    <img src="" alt="" class="ocultar" />
</div>

Where the class .ocultar have a

display: none;

The vars tempo_flash is only the animation time, and the file get_banner.php is only for getting the next banner from the mysql database. It is tested and working fine. The problem I have is that after a little (4 or 5 banner changing) the browser stops answering (for Firefox Chrome and Opera) and on IE I get an alert Stack overflow at line: 3 and the javascript of the whole page stops working.

Any help appreciated!

Bali C
  • 30,582
  • 35
  • 123
  • 152
  • Have you tried stepping through your code with a debugger? – Matt Ball Apr 10 '12 at 13:52
  • 15
    You should spend the money to get that semicolon key fixed :-) – Pointy Apr 10 '12 at 13:53
  • 1
    Try changing your `$.post()` to `$.ajax()` and including a `timeout` parameter. I'd imagine that the AJAX requests are taking some time to complete and stacking up and therefore eating the browsers resources. – Rory McCrossan Apr 10 '12 at 13:54
  • 3
    I think part of the problem might be that you declare all those variables *outside* the function. There's no reason for that; they should be declared *inside* the function passed to `setInterval()`. As it is, it's very likely that the separate functions are stepping on eachother's toes. – Pointy Apr 10 '12 at 13:56
  • i tried to put the var declarations inside, and got the same error... i'm trying to changing to $.ajax right now, but the firebug says it's getting less than 200ms every post... – Caio Felipe Giasson Apr 10 '12 at 14:09

2 Answers2

3

Inside each iteration of the setInterval()ed function, you assign a .load() event to an image place holder. Assigning an event to an object does not remove existing ones!

So on second iteration, the image place holder will have two .load() event handlers, then three and so on; and every time the image is loaded, it will fire all event handlers attached to .load() event. You probably need to re-factor your code, perhaps by assigning the .load() event handler only once (and use semicolons).

Salman A
  • 262,204
  • 82
  • 430
  • 521
  • Took off all theh .load() events, and it stops giving the "stack overflow" error, so its solved!!!!! thank you Salman, is there a better way to trigger an event only when the image finnish loading, after changing the "src"? – Caio Felipe Giasson Apr 10 '12 at 15:02
  • 1
    Attach `.load()` event to the element so it will fire whenever the image finishes loading. Inside the callback function you can check `$(this).attr("src")` to check which image was loaded. – Salman A Apr 10 '12 at 16:18
0

you shouldn't use setInterval, you should use a setTimeout inside a function, and execute it on the callback of the $.post, something like:

var id_atual
var temp_id
var tempo_flash = 50
var $slide_atual = $('#slider .atual')
var $slide_prox = $('#slider .ocultar')
function tictac(){
    setTimeout(function(){
        id_atual = $slide_atual.attr('alt')
        $.post('get_banner.php', {atual: id_atual}, function(proximo){
            temp_id = proximo.split(';;')
            $slide_prox.attr('src', temp_id[0]).load(function(){
                $slide_atual.hide('fade', tempo_flash, function(){
                    $slide_atual.attr('alt', temp_id[1]).attr('src', temp_id[0]).load(function(){
                        $slide_atual.show('fade', tempo_flash)
                    })
                })
            })
        })
        ticktac();
    }, 4000);
}

this way, the 4 seconds only start counting if and when the response from the server is complete, you will not have your overflow problems

André Alçada Padez
  • 10,987
  • 24
  • 67
  • 120