1

I am seeing a major memory leak within Firefox and IE on my below code. To be fair, it could very well be my poor implementation and it needs changing, to allow Firefox and other browsers to garbage collect.

Does anyone have any suggestions on how to tweak the code to allow for a more efficient way of refreshing the page?

<input type="checkbox" onclick="sel1()" id="AutoRefresh">

<script type="text/javascript">
    function sel1(){
        var ref = document.getElementById('AutoRefresh').checked;
        if(ref == true) {
            setInterval(function(){
                document.getElementById('dataRefreshButton').click(); }, 2000);
            }
            window.alert("Auto refresh on");
        }
</script>
Jørgen R
  • 10,568
  • 7
  • 42
  • 59
user1479931
  • 244
  • 1
  • 3
  • 11
  • Don't check for `ref == true`, `ref` should already be a boolean, and as such, `true` or `false` (So, use: `if(ref)`). Also, `.click()` doesn't appear to be a function on the element, or so my console says. – Cerbrus Dec 13 '12 at 09:19
  • Not sure, but maybe this could be of assistance: http://stackoverflow.com/questions/2990484/javascript-memory-leak-on-page-refresh-remedy?rq=1 – acjay Dec 13 '12 at 09:23

2 Answers2

0

I think this will be better.

<script type="text/javascript">
    function getdata(){
        // get your data    
    }
    var intGetData;
    function sel1(){
        var ref = document.getElementById('AutoRefresh').checked;
        if(ref == true) {
            intGetData = setInterval(getdata, 2000);
            window.alert("Auto refresh on");
        }
        else{
            clearInterval(intGetData);  
            window.alert("Auto refresh off");           
        }
    }
</script>
Junnan Wang
  • 627
  • 4
  • 12
  • Hi, thanks for this. This doesn't seem to refresh the page. Not sure if I have missed something on this example. – user1479931 Dec 13 '12 at 11:45
  • This depend on what you write in the getdata method. You can refresh the data fields using ajax or simply refresh the page. I suppose that you want to use ajax so I write this code. – Junnan Wang Dec 14 '12 at 02:16
0

Change setInterval to setTimeout. Your current code will set up a new interval to click the element every 2s when clicked. If you click once, you trigger an avalanche.

Or even better, you stop the interval:

var handle = null;
function sel1(){
    var ref = document.getElementById('AutoRefresh').checked;
    clearInterval(handle);
    if (ref)
        handle = setInterval(function(){
            document.getElementById('dataRefreshButton').click();
        }, 2000);
    window.alert("Auto refresh on");
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Hi, thanks for this. This pushes up the Firefox memory, a little at a time. This may be an issue with Firefox itself. But I had a good impression, that the code is not efficient before this. – user1479931 Dec 13 '12 at 11:48