0

I have a problem with the following javascript code. Basically I want to run a background check (check.php) for a process on an Unix server. The execution time of the php and response takes about 1.2-1.5 seconds (did a test in Mozilla) so I've come up with the script below to refresh and check what's going on with that process and change the background of a DIV accordingly. Right now, after a few refreshes it just hangs, because I think it goes into a loop and it doesn't wait for the first function to finish and it just piles up. Any ideas what am I doing wrong? A more simpler way to do it?

<script language="javascript" type="text/javascript">

    function getVal(param)
    {
        var strURL = param;
        var req = new XMLHttpRequest();
        req.open("GET", strURL, false); //third parameter is set to false here
        req.send(null);
        return req.responseText;
    }

    function changeDivImage()
    {   
        var pid = getVal("check.php")
        var imgPath = new String();
        imgPath = document.getElementById("status_body").style.backgroundImage;

            if(pid == 2)
            {
                document.getElementById("status_body").style.backgroundImage = "url(active.png)";
            }
            else
            {
                document.getElementById("status_body").style.backgroundImage = "url(down.png)";
            }
        changetimer();
    }

    function changetimer()
    {
        setInterval( "changeDivImage()", 5000 );
    }
    </script>
Chris19
  • 210
  • 7
  • 17
  • 1
    Don't use synchronous XHR. It's almost never what you want and you'll almost certainly hang the user's browser under certain conditions. – Brian Donovan Feb 04 '11 at 16:56
  • 1
    In addition, don't use setInterval. Use setTimeout, and the next time that the request comes back, do another setTimeout. That way you don't stack up a bunch of requests (and get LIFO type issues). – Sean Feb 04 '11 at 17:00

2 Answers2

2

setInterval will keep calling the funciton over and over, so calling the changetimer funciton again after the changeDivImage returns will stack up more and more unwanted functions. try this:

<script language="javascript" type="text/javascript">

    function getVal(param)
    {
        var strURL = param;
        var req = new XMLHttpRequest();
        req.open("GET", strURL, false); //third parameter is set to false here
        req.send(null);
        return req.responseText;
    }
    //this function calls it self automatically and again everytime 
    //but only when it finishes its operations.

(function changeDivImage()
{   
    var pid = getVal("check.php")
    var imgPath = new String();
    imgPath = document.getElementById("status_body").style.backgroundImage;

        if(pid == 2)
        {
            document.getElementById("status_body").style.backgroundImage = "url(active.png)";
        }
        else
        {
            document.getElementById("status_body").style.backgroundImage = "url(down.png)";
        }
      setTimeout(changeDivImage, 5000);
})();

Amjad Masad
  • 4,035
  • 1
  • 21
  • 20
  • 1
    [You really shouldn't use arguments.callee.](http://stackoverflow.com/questions/103598/why-was-the-arguments-callee-caller-property-deprecated-in-javascript) Why not call `setTimeout(changeDivImage, 5000);` instead? – Chris Shouts Feb 04 '11 at 17:33
  • thanks for your reply, I did try your code yesterday, but it didn't work. Firebug said that changeDivImage() is undefined, so I had to remove the brackets at the end to make it work. Right now I don't get any errors with the javascript code but I'll need to cut down on the execution time for the check.php. I'll post somethign as soon as I get it right. – Chris19 Feb 06 '11 at 07:01
1

According to MDC, the setInterval function

Calls a function repeatedly, with a fixed time delay between each call to that function.

Since you are invoking the changetimer function at the end of the changeDivImage function, it will create a new timer without removing the old timer every time changetimer is invoked. I would suggest removing the call to the changetimer function at the end of the changeDivImage function.

Chris Shouts
  • 5,377
  • 2
  • 29
  • 40