Saturday, August 13, 2005

Beware of false falses, or something like that

This is a really, really common gotcha that trips up even expert webdevs.

When you're checking for the existence of an object or property, make sure what you're testing is not going to return an unexpected value. For example.

if (document.documentElement.scrollTop) {...}

Right away, we're in trouble. Why? Because the default value of scrollTop is zero (for a page that hasn't been scrolled yet). In JavaScript (and many other "real" programming languages), zero is the same as false. So in the above statement, the conditional in parenthesis will fail, and the code will make the wrong choice — and we haven't even tested for document.body yet.

What about this?

if (document.documentElement && document.documentElement.scrollTop) {...}

Also no good if scrollTop returns zero. Here's a better way:

if (document.documentElement) {...}

This test will return undefined if documentElement is missing, which is correct behavior. In a pseudo-Boolean check like this one, undefined is as good as false.

3 Comments:

At 3:00 AM, Anonymous Anonymous said...

What you really want there is

if( document.documentElement.scrollTop != undefined ) { /* ... */ }

and likewise

if(
    document.documentElement != undefined
    && document.documentElement.scrollTop != undefined
) { /* ... */ }

 
At 10:25 AM, Blogger scottandrew said...

aristotle: I'm not sure that first approach will work. If document.documentElement doesn't exist, you'll get a fatal error (at least in IE) that says something like "documentElement is not an object."

rory: I'm not picking on your code, so don't take it personally. :) My point is that it's not always accurate to test for the existence of properties that return values that could be interpreted as false.

 
At 12:16 AM, Anonymous Anonymous said...

Scott: yes, I realize it won’t work. I wasn’t getting into a comprehensive treatise about checking for certain methods. :-) I was just talking about a more robust choice of boolean condition.

It just occured to me that there is, in fact, a better one yet:

if( "scrollTop" in document.documentElement ) { /* ... */ }

If checking the whole shabang really thoroughly is what you’re after, you probably need something like this:

if(
    'documentElement' in document
    && typeof document.documentElement == 'object'
    && 'scrollTop' in document.documentElement
) { /* ... */ }

which obviously is so cumbersome that just wrapping the access into a series of try {} blocks is better for the purpose:

try {
    /* use W3C DOM without checking */
}
catch(e) {
    /* no, that's not it */
    try {
        /* use IE DOM without checking */
    }
    catch(e) {
        /* ??? */
    }
}

 

Post a Comment

<< Home