First let's clean this up so we can see what's going on more clearly.
if (! empty($facebook) and ! preg_match('/^\s+$/',$facebook) and is_file($_SERVER['DOCUMENT_ROOT'] . "/$smw_dir./facebook.png")) {
$fade = ($animation == 'fade' or $animation == 'combo')?" style=\"opacity:$icon_opacity; -moz-opacity:$icon_opacity;\"":null;
echo "<a href=\"$facebook\" $nofollow $newtab><img src=\"$smw_path/facebook.png\" alt=\"$imgcaption Facebook\"
title=\"$imgcaption Facebook\" $fade class=\"$animation;\"/></a>";
}
A few explanations:
- Looks like you're expecting maybe spaces for $facebook? Your programming should make sure that doesn't happen so you could just use if ! empty(), but if you expect there might be a space, there could be two. Or three. Or a tab. Or a newline. The preg captures all those possibilities.
- Your file_exists is OK, but I prefer is_file and a full path. It makes it more specific (just a preference)
- $var = (condition)?'yes':'no'; is a ternary or short circuit evaluation, it does in one line what usually takes 4: if (condition) { $var='yes'; } else { $var = 'no'; }
- I chose to escape " 's so we can easily interpolate scalars as opposed to concatenation or dropping in and out of PHP, which is one of the attributes of spaghetti code. :-)
- No need to echo '', if the condition exists we "do this" ortherwise we do nothing.
With that out of the way, the good thing you're doing is returning false on the link for your javascript - however, I would move that return false to the function itself:
function "recordOutboundLink(link_obj, title, url) {
//
return false; }
onclick="
return recordOutboundLink(this, 'Outbound Links', 'example.com');">
This kind of handling means you can leave everything "as is" and it will still work with JS disabled. So overall, if you don't do either of those, with the above changes, all you'd add is
if (! empty($facebook) and ! preg_match('/^\s+$/',$facebook) and is_file($_SERVER['DOCUMENT_ROOT'] . "/$smw_dir./facebook.png")) {
$fade = ($animation == 'fade' or $animation == 'combo')?" style=\"opacity:$icon_opacity; -moz-opacity:$icon_opacity;\"":null;
echo "<a href=\"$facebook\" onclick=\"return recordOutboundLink(this, 'Outbound Links', 'example.com');\"
$nofollow $newtab><img src=\"$smw_path/facebook.png\" alt=\"$imgcaption Facebook\" title=\"$imgcaption Facebook\"
$fade class=\"$animation;\"/></a>";
}
Note that, since you're using XHTML (as identified by the /> on img), the proper syntax is onclick, not onClick (which will still work, but . . . yeah. :-) )
I'd see if you can avoid adding inline javascript though. an easy way, if you're using jQuery, is to just attach your behavior to everything with the class fade or combo:
$(function(){
// Explicitly using "a.face" in case you have other elements with the class fade or combo
if ($('a.fade').length || $('a.combo').length) {
$('a.fade','a.combo').click(function() { return recordOutboundLink(this, 'Outbound Links', 'example.com'); });
}
});
If you're not using jQuery, you can do the same thing with regular JS wrapped in window.onload, which I don't have time to code up at the moment. :-)