var flashStep = 1;
function flash() {
 if(flashStep==1) {
 document.bgColor="FFFF00";
 flashStep=2;
 }
 else {
 document.bgColor="FF0000";
 flashStep=1;
 }
}
var task = window.setInterval("flash()",1000);

This code is supposed to make the screen flash, but does not work at all. /. Why does this not work, it looks flawless to me

var flashStep = 1;
function flash() {
 if(flashStep==1) {
 document.bgColor="FFFF00";
 flashStep=2;
 }
 else {
 document.bgColor="FF0000";
 flashStep=1;
 }
}
var task = window.setInterval("flash()",1000);

This code is supposed to make the screen flash, but does not work at all. http://jsfiddle/phjnM/2/. Why does this not work, it looks flawless to me

• javascript

7 Answers 7

Several issues. First of all, bgColor is a property of elements - it doesn't apply to document. Perhaps you meant document.body? Second, you're using bgColor. Please don't. You should instead use style.backgroundColor. Third, you'll probably want to use a boolean value for two states. Fourth, never pass a string to setInterval or setTimeout. So, your code, rewritten, would be something along the lines of:

var flash = false;
var task = setInterval(function() {
    if(flash = !flash) {
        document.body.style.backgroundColor = '#ff0';
    } else {
        document.body.style.backgroundColor = '#f00';
    }
}, 1000);

The problem is the (global) scope being applied when you pass a string to setInterval. If you pass a function reference instead it works:

var task = window.setInterval(flash,1000);

See http://jsfiddle/phjnM/7/

The other option is in jsfiddle choose the "nowrap" option so that your code isn't defined in an onload/onready handler's scope...

Because your code in the JSFiddle is wrapped in an onload function.

Passing a reference to the function will fix it.

var task = window.setInterval(flash,1000);

Or an alternative would be to choose a (no wrap) option from the menu on the left.

By the way, do this to color the document background.

var i = 0;
var task = setInterval(flash, 1000);

function flash() {
    document.documentElement.style.backgroundColor=(i = ~i) ? '#ff0' : '#f00';
}

JSFIDDLE DEMO

Use window.setInterval(flash, 1000).

This particular problem was probably a scope problem when running the code on jsfiddle. In general, don't pass code as string when you actually want to pass a callback, which in Javascript you can do by passing the function itself.

setInterval(flash,1000);

http://jsfiddle/phjnM/5/

See now its working, http://jsfiddle/phjnM/8/

Instead of:

document.bgColor="FFFF00"

you need this form:

document.body.style.backgroundColor="#FFFF00";

You were missing the .body.style., you had the wrong name for the style attribute backgroundColor and you need to have a # on the color value.

Further, your setTimeout() should be like this:

var task = setInterval(flash,1000);

In your fiddle, you can get the flash function to be global, by changing it's definition to:

window.flash = function() {

You can see this all work in a revised version of your jsFiddle: http://jsfiddle/jfriend00/h4VZm/