views:

412

answers:

2

Hi folks,

While I have the problem itself seemingly resolved, I'm hoping someone can shed some light on the why of this...

Below are two snapshots of the same function whose job is to remove a div that contains a user feedback message. It's setup to use an optional timeout, if a timeout is specified it makes a call to itself using setTimeout() which then removes the div.

The only difference between the two versions of the function is where this.remove() is called - in the problem version I send a message to the log using blackbirdjs first and then call this.remove() - after this executes the log is flooded with unending log messages of "Removing feedback div..." as fast as the browser can pump them in.

In the working version, however, I simply reverse the order and everything executes normally and all is well...

I'm boggled, I would think that the order in this case would be trivial but apparently not. Can anyone shed some light on why this would be happening? Is this a jQuery bug or a problem with blackbird or some kind of weird quirk of JavaScript in general?

NOTE:
I had some mixed success using a call to confirm() - if it came back false I told it to return and this stopped it - however, just adding return after the remove call had no effect.

Interestingly enough, either version seems to work fine in IE8 - so this may be a firefox/gecko problem?

Problem Code:

function clear_feedback(target_container, timeout){
    log.debug("timeout: " + timeout);
    log.debug("target_container: " + target_container);

    if(timeout == undefined){
        log.info("removing target...");

        $(target_container).children(".update_feedback").slideUp("slow",
            function() {
                log.info("Removing feedback div...");
                this.remove();
            }
        );
    }
    else{
        log.info("Setting timeout, THEN removing target...");

        setTimeout("clear_feedback('" + target_container + "')", timeout);
    }
}

Working Code:

function clear_feedback(target_container, timeout){
    log.debug("timeout: " + timeout);
    log.debug("target_container: " + target_container);

    if(timeout == undefined){
        log.info("removing target...");

        $(target_container).children(".update_feedback").slideUp("slow",
            function() {
                this.remove();
                log.info("Removing feedback div...");
            }
        );
    }
    else{
        log.info("Setting timeout, THEN removing target...");

        setTimeout("clear_feedback('" + target_container + "')", timeout);
    }
}
A: 

I have seen an issue just like this but in a different context; however, I suspect the root cause is the same.

If you take a look at log.info, you'll see that it inserts a node into the DOM. If one of the jquery functions happens to be traversing the DOM in just the right location, in particular, right at the spot where log.info is inserting the node, and then if that causes your callback to be invoked, your callback will insert another node, and you end up in an infinite loop.

The question of why this doesn't happen in IE8 is likely to be one of two reasons: either the DOM structure isn't exactly the same across browsers, or IE8 uses a different strategy for handling DOM node insertion while javascript code is traversing the tree.

You might try using Firebug, placing a breakpoints around the problematic lines and then viewing the DOM tree to see if you can spot behavior like this.

jdigital
Cool - thanks for the info - I'll investigate and let you know what, if anything, I find.
Crazy Joe Malloy
+1  A: 

You should have checked your browsers error console instead of just relying on the blackbirdjs console.

Then you would have noticed that the browsers error console is flooded with error messages too (with either of your code versions)

The actual problem in your code is

this.remove();

this is a HTML DOM element in the callback-function and doesn't have the function remove() thus the children only get hidden but not really deleted. And on this.remove() you get an exception. As the callback-function throws an exception jQuery ends up in an endless loop trying to do its job

What you need to do is wrapping the element in a jQuery object.

$(this).remove();


Now it's also clear why the second version seems to have fixed the error

log.info("Removing feedback div..."); //error logged
this.remove();  //exception

this.remove();  //exception
//log line not executed as previous line threw exception
log.info("Removing feedback div...");


The fact that jQuery even ends up in and endless loop and if this is correct behavior is debatable and needs more investigation deeper in the inner-workings of jQuery. But this isn't of interest to you

For those interested there is realted bug ticket

http://dev.jquery.com/ticket/2846

jitter
gah..../head-slam-on-desk I haven't had the chance to test this yet and probably won't for the next while (took some annual leave Christmas holidays started for me as of Friday), but after reading this the error seems to be pretty clear...rookie mistake.
Crazy Joe Malloy
Sorry for the long delay - first day back at work. Many thanks!
Crazy Joe Malloy