Updating CSS for "active" tab I'd like to get some feedback on this very basic JavaScript to update which tab is showing as "active" on my simple web page. This is all in a file titled adjustactive.js and included in my footer template for all pages like so:

&lt;script src="assets/js/adjustactive.js"&gt;&lt;/script&gt;


Here's the full content of that .js file:

//function courtesy of comments in http://befused.com/javascript/get-filename-url
function getFileName() {
    var url = document.location.href;
    url = url.substring(0, (url.indexOf("#") == -1) ? url.length : url.indexOf("#"));
    url = url.substring(0, (url.indexOf("?") == -1) ? url.length : url.indexOf("?"));
    url = url.substring(url.lastIndexOf("/") + 1, url.length);
    if(url.length == 0) url = "index";
    return url;
}


function updateNav() {
    var idName = getFileName();
    document.getElementById(idName).className = "active";
}

window.onload = updateNav();
First off, you have a bug. This instruction

window.onload = updateNav();


doesn't execute updateNav on load. It executes updateNav immediately and assigns its return value (which is undefined) to onload. Instead it should be:

window.onload = updateNav;


Also assigning event handlers directly to on... properties isn't something you should be doing. The main problem is you are potentially overwriting any other scripts that also addign to onload. You should instead use addEventListener.



You can simplify parsing the URL. document.location already is provides the parts of the URL. In your case document.location.pathname contains the path without domain, query, hash, etc:

function getFileName() {
    var url = document.location.pathname;
    url = url.substring(url.lastIndexOf("/") + 1, url.length);
    if (url.length == 0) url = "index";
    return url;
}




In my opinion you are misusing the id attribute here. This is not what it's made for. (However to be honest I have difficulties finding the right words to explain why not - it just feels wrong.)



Finally you should be aware that this is a bad usage of JavaScript. You are using JS here for something that should be done server-side (that is directly in the HTML).