variable scope in javascript behavior

I have the following javascript function which is responsible of togglling some elements in my html page based on the passed parameter:

function toggleDeliveryOption(source) {
    if(source.toLowerCase() === "maildeliveryoption") 
    {
        var fs = document.getElementById("mailDeliveryOptionFS");
    }
    else if(source.toLowerCase() === "faxdeliveryoption")
    {
        var fs = document.getElementById("faxdeliveryoptionFS");
    }
    else if(source.toLowerCase() === "emaildeliveryoption")
    {
        var fs = document.getElementById("emaildeliveryoptionFS");
    }
    if(fs)
    {
        if(fs.style.display == "none")
            fs.style.display = "block";
        else
            fs.style.display = "none"
    }
} 

now the开发者_开发百科 weired behavior is that if source was the first one (maildeliveryoption) it works and fs will holds the element with id mailDeliveryOptionFS, but for other two elements in the 2 other else branches, fs evaluates to null so it doesn't get into the if condition! I think this problem has something to do with variable scope in javascript, but I can't figure out what is the problem

Perhaps you are not looking for the right ID. IDs are case sensitive, even if you compare the condition with a lower case ID, the real ID will still be camel case (like for the first element, since it's the only one which is being found).

function toggleDeliveryOption( source )
{ 
    var source = source.toLowerCase(),
        id,
        fs;

    switch ( source ) 
    {
        case "maildeliveryoption":
            id = "mailDeliveryOptionFS";
            break;

        case "faxdeliveryoption":
            id = "faxDeliveryOptionFS";
            break;

        case "emaildeliveryoption":
            id = "emailDeliveryOptionFS";
            break;
    }

    fs = document.getElementById( id );

    if ( fs )
    {
        if ( fs.style.display == "none" )
        {
            fs.style.display = "block";
        }
    }
} 

Looks good (though using a switch would look nicer). I don't know what these calls are benig used for but you do have a capitalization difference between the first one and latter two. You have "mailDeliveryOptionFS" in beautiful camel case but "faxdeliveryoptionFS" and "emaildeliveryoptionFS" are not. That might be your problem? Change them to "faxDeliveryOptionFS" and "emailDeliveryOptionFS" respectively?

Change your code to this to properly declare the local variable fs once. Then, you can reliably test at the end of your code to see if it's been set or not. Your previous code might work due to variable hoisting, but it isn't the recommended way of coding. A variable's scope is the function it's declared in.

function toggleDeliveryOption(source) {
    var fs = null;
    if(source.toLowerCase() === "maildeliveryoption") 
    {
        fs = document.getElementById("mailDeliveryOptionFS");
    }
    else if(source.toLowerCase() === "faxdeliveryoption")
    {
        fs = document.getElementById("faxdeliveryoptionFS");
    }
    else if(source.toLowerCase() === "emaildeliveryoption")
    {
        fs = document.getElementById("emaildeliveryoptionFS");
    }
    if (fs)
    {
        if (fs.style.display == "none") {
            fs.style.display = "block";
        } else {
            fs.style.display = "none";
        }
    }
} 

The other potential gotcha here is that fs.style.display may not be previously set as reading the style variable this way only returns explicit inline styles in the HTML (it does not return things set via CSS). If you want to know the actual state of the display setting, you have to use computed style which will include CSS settings. Getting the computed style is different in windows so it takes a few more lines of code to do that. This is one of the reasons I use a framework like YUI or jQuery because it takes care of all this cross-browser mess for me.

Incidentally, you could simplify the code a lot like this:

function toggleDeliveryOption(source) {
    var fs = document.getElementById(source);
    if (fs) {
        if (fs.style.display == "none") {
            fs.style.display = "block";
        } else {
            fs.style.display = "none";
        }
    }
}

To make this simpler version work, just pass the id of the option to toggle:

toggleDeliveryOption("mailDeliveryOptionFS");

If you want the actual display state including CSS settings, then you would need to use this:

// add IE compatibility function for getComputedStyle
if (!window.getComputedStyle) {
    window.getComputedStyle = function(el, pseudo) {
        this.el = el;
        this.getPropertyValue = function(prop) {
            var re = /(\-([a-z]){1})/g;
            if (prop == 'float') prop = 'styleFloat';
            if (re.test(prop)) {
                prop = prop.replace(re, function () {
                    return arguments[2].toUpperCase();
                });
            }
            return el.currentStyle[prop] ? el.currentStyle[prop] : null;
        }
        return this;
    }
}


function toggleDeliveryOption(source) {
    var fs = document.getElementById(source);
    if (fs) {
        var style = getComputedStyle(fs, null);
        if (style.display == "none") {
            fs.style.display = "block";
        } else {
            fs.style.display = "none";
        }
    }
}