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.
6 Comments:
What you really want there is
if( document.documentElement.scrollTop != undefined ) { /* ... */ }
and likewise
if(
document.documentElement != undefined
&& document.documentElement.scrollTop != undefined
) { /* ... */ }
You raise a good point ,but if you examine the trinary operator expressions in the comments of the previous post (which clearly inspired this one) you should see that the correct value is assigned even when scrollTop is 0. It may seem to follow the 'wrong' logical branch but it gives the right answer.
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.
try/catch and typeof are probably the most appropriate constructs to use for this type of detection:
try {
if (typeof document.documentElement.scrollTop != "undefined") {...}
} catch (e) {
...
}
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) {
/* ??? */
}
}
I didn't intend to come off defensively. I figured the result of the trinary operator expression I posted was worth mentioning because it *looked* like it would fall into this trap. You're absolutely right that this trap exists. I wonder is there a way around it by using the strict equality (===) operator?
Post a Comment
<< Home