jQuery code returns 'Object expected' error

Question

I have the following jQuery code. It is writing two event listeners like this ok? The following code throws an.error

$(document).ready(function () {
    $('tr').hover(
        function () {
            $(this).css('background-color', '#FFFF99');
            $(this).contents('td').css({
                'border': '1px solid red', 
                'border-left': 'none', 
                'border-right': 'none' 
            });
            $(this).contents('td:first').css('border-left', '1px solid red');
            $(this).contents('td:last').css('border-right', '1px solid red');
        });

        $('#radioButtonImageSourceContainerId input:radio').click(function () {
            if ($(this).val() === 'fromFile') {
                addVisibility("from-file");
                removeVisibility("from-url");
            } 
            else if ($(this).val() === 'fromUrl') {
                removeVisibility("from-file");
                addVisibility("from-url");
            }
        })
    });

The other 2 functions are

function addVisibility(elemId) {
    document.getElementById(elemId).style.display = "";
    if (document.getElementById(elemId).style.visibility == "hidden") {
        document.getElementById(elemId).style.visibility = "visible";
    }
}

function removeVisibility(elemId) {
    document.getElementById(elemId).style.display = "";
    if (document.getElementById(elemId).style.visibility == "visible") {
        document.getElementById(elemId).style.visibility = "hidden";
    }
}

Answer 1

Check to make sure you found an element first.

function addVisibility(elemId) {
    var el = document.getElementById(elemId);
    if( el )
        el.style.display = "";
        if (el.style.visibility == "hidden") {
            el.style.visibility = "visible";
        }
    } else {
        console.log( 'NO ELEMENT WAS FOUND' );
    }
}

...and cache your DOM selection instead of repeating it.

Another issue is these lines:

$(this).contents('td').css({...
$(this).contents('td:first')...
$(this).contents('td:last')...

The contents() method returns text nodes as well. You should use find or children instead.

$(this).find('td').css({...
$(this).children('td').css({...

---

A better way to rewrite this part of it (with jQuery) would IMO be this:

$(document).ready(function () {
    $('tr').hover(function () {
        $(this).addClass( 'hilite_row' );
        var tds = $(this.cells).addClass( 'hilite_cell' );
        tds.first().addClass( 'hilite_left_cell' );
        tds.last().addClass( 'hilite_right_cell' );
    });

    $('#radioButtonImageSourceContainerId input:radio').click(function () {
        var value = $(this).val(),
            visib = ['hidden','visible'],
            file = visib[ +(value === 'fromFile') ],
            url = visib[ +(value === 'fromUrl') ];
        if( file === url ) return;
        $("#from-file").css('visibility', file);
        $("#from-url").css('visibility', url);
    })
});

Then define the style for the classes in your CSS.

---

Or even better, use the :hover pseudo-selector and do it all in CSS, though IE6 won't work.

Answer 2

Just a tip for you: your table row hover is doing a tiny bit of extra work with contents(). contents() looks for text nodes and comments which you can safely ignore; find() would work as well but it travels recursively through the DOM instead of just through immediate children. I would change it to children() instead:

$('tr').hover(function () {
    $(this).css('background-color', '#FFFF99');
    $(this).children('td').css({ 
        'border': '1px solid red', 
        'border-left': 'none', 
        'border-right': 'none' 
    });
    $(this).children('td:first').css('border-left', '1px solid red');
    $(this).children('td:last').css('border-right', '1px solid red');
 },
 function() {
     // mouseout - you were missing this
 });