views:

77

answers:

3

I've got a plug-in I'm working on which fetches a portion of a remote page, and saves that portion to be used in a jQuery UI dialog widget; originally I was attaching the HTML to the DOM and hiding it with display:none until it was called with $.dialog but I realized I could just leave the DOM node in a variable.

Is there any reason not to do this? Here's an example:

function makeDialogLink() {

    var HTML = $("<div />").load('file.html #container', function() {

        $('a#test').bind('click', function(e) {
            e.preventDefault();
            showDialog();
        }); 
    });

    function showDialog() {
        $(HTML).dialog({
            autoOpen : true,
            width  : opts.width,
            modal  : opts.modal,
            resizable : opts.resizeable,
            title  : opts.title
        });

        // some other stuff happens in here, and a setTimout
        closeDialog();
    }
    function closeDialog() {
        $(HTML).dialog('close');
    }
}

I've simplified this quite a bit from what I'm actually doing, but this encapsulates what my question is about pretty well.

As you can see I've loaded a portion of a remote document, I pop it up in a dialog, and close that dialog later, but I never directly attach the fetched HTML to the DOM ($.dialog does it at some point, of course).

Is there any reason to NOT do it this way? Seems a lot nicer then having to shove the HTML into a hidden DIV and then fetch it later. I'm just wondering if there's some pitfall I'm unaware of to using closures in javascript in this fashion.

+1  A: 

Yes, you have to be careful about nodes which are not attached to the DOM. IE is notoriously bad at leaking memory. (Scroll down to the section on cross-page leaks.)

Since you're creating an unattached DOM tree and then attaching it to the document (in showDialog()), I'd imagine your code leaks. Of course, the only way to be sure is to get some profiling tools and find out.

A safer way to do this is to use a hidden div.

var HTML = $("#hiddenDiv").load('file.html #container');
josh3736
I'm not sure if a memory leak problem is applicable. $.load is only getting called once for each element the plug-in operates on, but I'm going to read the article you linked :)
Will
@Will: The DOM insertion order leak is a problem for any number of calls. So, if the plugin operates on ten elements, you'll leak those ten elements. Worse, you're actually leaking *across pages*: the leaked memory won't be freed until the browser is closed.
josh3736
+2  A: 

I can't think of any serious drawbacks to that. It may be a bit slower, since it's reattaching it to the DOM every time you need a dialog but it doesn't really matter since 1) the difference would not even be noticeable unless you're popping up hundreds of dialogs and 2) it may not be that much slower, if at all, than having a div that you toggle.

It seems like the best and cleanest solution. I would keep it if I were in your position.

CD Sanchez
A: 

You can also just store the data from your ajax call as a string (use $.get or $.ajax instead). Only create the dom elements after the user clicks on the dialog button.

morgancodes