New to jQuery, simple popup window script, any problems?

Question

Still a little "gunshy" about jQuery. Does this simple popup window script look okay? Is my jQuery logic right?

I have a few questions in the code comments that I'm curious about.

$( document ).ready( function()
{
    $( "a[target='popup']" ).click( function( event )
    {
     var $href = $( this ).attr( 'href' ); // should I use "this.href"?
     var $name = $( this ).attr( 'target' ); // does the window name matter?
     var $config = 'width=590, height=590, top=20, left=60, scrollbars=1';
     var $popup = window.open( $href, $name, $config );

     if ( window.focus ) // is this even necessary?
                            // any other conditions I should check instead?
     {
      $popup.focus();
     }
     event.preventDefault(); // should this be above $popup.focus()?
    });
});

It seems to work, but since this script will be important for acquiring RSS subscribers on my site, I thought I'd make sure.

Answer 1

window.focus

Makes a request to bring the window to the front. It may fail due to user settings and the window isn't guaranteed to be frontmost before this method returns.

event.preventDefault()

Cancels the event if it is cancelable, without stopping further propagation of the event.

I don't think there will be any difference if you give this above the focus method.

Answer 2

// should I use "this.href"? No, use the jquery selector - if you're going to use $(this) a lot, put it into a variable at the start so you don't have the overhead of creating the jquery object each time (you do it twice, so you're creating a jquery object twice).

// does the window name matter? If you want to do anything with the window later, like close it or change it's location, you'll need the name. It's just a handle to the window.

// is this even necessary? This just makes sure you can do what you're about to try - it's a feature test to ensure you don't generate an error where the focus() method isn't available.

// any other conditions I should check instead? Nope - test for the function you will call (you call it when you focus the popup).

// should this be above $popup.focus()? No. It's better to leave this until last as this is where another developer would look for it. Do all the stuff you want to do first, then pop this in to stop the event from bubbling up.

Finally, what's with the $ prefix on your variable names? You might want to save that practice for PHP as the $ is now a handle on jquery.

$(document).ready( function() {
    $("a[target='popup']").click( function(event) {
        var myObject = $(this);
        var href = myObject.attr("href");
        var name = myObject.attr("target");
        var config = "width=590, height=590, top=20, left=60, scrollbars=1";
        var popup = window.open(href, name, config);

        if ( window.focus ) {
                popup.focus();
        }
        event.preventDefault();
    });
});

Answer 3

Your javascript function is correct. Here is an explanation for each question:

1) Should I use this.href?
No you shouldn't because thats not the jquery way to do things. Javascript implementations can vary from browser to browser, and this jQuery function will make sure the call returns the correct value in every browser it is intended to support. While this.href might work, there is no guarantee it will, but the jQuery will work (in the browsers it is intended to support).

2) Does the window name matter?
Yes. The point of using this jQuery script is to control the window that comes up, however, the link should work (targeting the named window) even if the user has javascript disabled. This javascript is intended to let you control what the window looks like.

3) Is this necessary?
Yes, this goes back to you not being guaranteed to support certain javascript features. The call to window.focus is just checking if the focus function exists for this element in this browser. If it does exist, it will try to set the focus to that element, and if it does not exist, it will not show as a script error (bad user experience) in the browser.

4) should this be above $popup.focus()?
This is letting the browser know that you successfully created and popped up the window yourself and that the event should stop working (thus canceling the browsers default event of opening the new window).