miscoded

Seems the boss has been browsing web stats again - I came across a bug report from him saying a DHTML menu on onestat.com only works once. (Load page, click any "demo" link at the bottom and try the menu on the left.)

I'm not paying that much attention to stats myself, being too busy doing the work that will hopefully smoothen out compatibility problems and make it possible to grow our usage share. It's exactly the type of brokenness Jon found on onestat.com I fear the most - a bit hidden away, subtle, annoying when the user needs that page to work but perhaps not disastrous enough to notify us of. Too many of these issues, and we've lost a user! (Unless, of course, the user happens to be our CEO - luckily he's an active surfer who comes across more compat problems than any average user and eagerly reports them ).

Now, deep inside the Onestat scripts is some code to handle browser differences. The script assumes that a browser below a certain level of DOM support can't be expected to create menus after the document loaded. That's of course not a bad assumption to make - until you see the sniffing they use to determine whether you have sufficient DOM support. Here is the relevant part:

if(kh.indexOf("Opera/7")>-1||kh.indexOf("Opera 7")>-1)return "Op7"; 
. 
. 
else if(kh.indexOf("Opera")>-1)return "Default";

Check the calendar, sir: we're in late 2009. Opera 7 - the only Opera version this script thinks is capable of opening the menu a second time - was released in January 2003. That's a 6 year old browser sniffer! I hope the script that generates their statistics is a bit more up to date!

And it's more than a little ironic that a web statistics company contributes to Opera's low usage share by running such old and broken code on their website.

Even in the classy company of bad version detection scripts we've met since we started testing Opera 10, this Flash detection approach stands out. That script goes to great effort to require an update every.single.time Adobe releases another Flash version.

It starts with a bold claim to be the ESPN "Flash detection system", no less:

// Author: Danny Mavromatis
// Version: 2.07.0
// Created: 10/29/2001
// Updated: 3/6/2006
// ESPN.com FLASH detection system
var f2 = false;
var f3 = false;
var f4 = false;
var f5 = false;
var f6 = false;
var f7 = false;
var f8 = false;
var f9 = false;

Look at all those variables. What might they be used for? Read on:

var fD = navigator.plugins["Shockwave Flash" + isVersion2].description;
var fV = parseInt(fD.charAt(fD.indexOf(".") - 1));

So, first it extracts one single letter that precedes a dot in the plugin's description of itself. This is presumably the plugin's major version number, you know the 8 in "8.0", the 9 in "9.0", and, um, the 0 in "10.0". Then:

f2 = fV == 2;
f3 = fV == 3;
f4 = fV == 4;
f5 = fV == 5;
f6 = fV == 6;
f7 = fV == 7;
f8 = fV == 8;
f9 = fV == 9;

..it just sets the corresponding "f"-variable to true. If the major version was 8, f8 will be true and so on. A great way to make sure the code will require maintenance - new variables for each new version. And then comes the real gem:

for (var i = 2; i <= mV; i++) {
if (eval("f" + i) == true) aV = i;
}
// alert("version detected: " + aV);

Let's see, we just had a variable fV which contained Flash's major version number (or at least its least significant digit) - but this script has severe amnesia. What was that number again? Better use a loop and 8 eval() calls to check the value of that variable. You never know, do you?

Now, dear readers - Mr. Mavromatis clearly needs some help with this code. Obviously, complexity and maintenance requirements are among his design goals. The natural question is whether there are good, non-obvious ways this script could be improved to be more complex and require even more maintenance? Suggestions welcome in comments.

Update: Danny Mavromatis responded in comments and - though he no longer works on the ESPN site - has made sure that the page where we found this problem has been updated. Kudos to Mr. Mavromatis for his quick response and sense of responsibility. I wish more web developers would act this way!

No more personal attacks, please!

Load getacooljob.com in Opera, see error page (http://aac.taleo.net/servlets/CareerSection?art_ip_action=FlowDispatcher&flowTypeNo=13&pageSeq=1&art_servlet_language=en&csNo=2'%3E%3C/head%3E%3Cbody%20bgcolor=). At first sight it might look like a standard browser sniffer saying "go away and don't dare becoming our customer until you've found a browser we like"... until you look a bit closer at that address. See the junk at the end?

&csNo=2'%3E%3C/head%3E%3Cbody%20bgcolor= decodes to

&csNo=2'></head><body bgcolor=

and it's caused by the lack of a closing quote for the META tag's content attribute on the previous page:

<META HTTP-EQUIV="refresh" content="01; url='http://aac.recruitsoft.com/servlets/CareerSection?art_ip_action=FlowDispatcher&amp;flowTypeNo=13&amp;pageSeq=1&amp;art_servlet_language=en&amp;csNo=2'>

The site apparently belongs to Taleo which is the "Leader in On Demand Talent Management". As they aren't talented enough to balance their quotes or use a validator to spot markup mistakes, the level of hype obviously exceeds their technical competence..

See also statements like "this Career Section does not allow the use of the BACK or FORWARD browser buttons during the application process" (source), the hideous sniffing here

function stopEvent() {
var userAgent = window.navigator.userAgent;
var appName = window.navigator.appName;
if ((appName.indexOf("Explorer") >= 0) && (userAgent.indexOf("Mozilla/3") >= 0) && (userAgent.indexOf("Mac") >= 0)) {
return (true);
} else {
return (false);
}
}

and their "anti-multiple-tabs" JavaScript

//---------------------------------------------------------------------
// This method increments the count of browser using the career section
// on the current session in the document's cookie.
// This code is inserted only when IE browser is used
//---------------------------------------------------------------------
function setMultiBrowserDetection()
{
if (window.navigator.cookieEnabled)
{
// The multibrowser problem doesn't occur where cookies are not available
var cs_cnt = -1;
var index = document.cookie.indexOf("cs_cnt");
var countbegin = -1;
var countend = -1;
if (index == -1)
{
cs_cnt = 1;
}
else
{
countbegin = (document.cookie.indexOf("=", index) + 1);
countend = document.cookie.indexOf(";", index);
if (countend == -1)
{
countend = document.cookie.length;
cs_cnt = eval(document.cookie.substring(countbegin, countend));
}
else
{
if (countend < countbegin)
{
cs_cnt = 0;
}
else
{
cs_cnt = eval(document.cookie.substring(countbegin, countend));
}
}
if (cs_cnt < 1)
{
cs_cnt = 1;
}
else
{
cs_cnt = cs_cnt + 1;
}
}
document.cookie = "cs_cnt="+cs_cnt;
if (cs_cnt > 1)
{
// More than one browser is using the current session
window.document.location = "http://aac.taleo.net/servlets/CareerSection?art_ip_action=MultipleBrowserConflict";
}
}
}

complete with eval() instead of parseInt() (meaning that anyone who can get a cookie set on this site can XSS them).. When you stop shaking your head, I'd like you to opine on how much less talented a web site could possibly get..

Since I spend most of my day investigating problems with Opera, I'm sometimes too quick to take for granted that a problem is Opera's problem. So when I was exploring the JIRA site with an internal build of Opera, clicked a link saying JIRA video overview and saw nothing but an empty page and a very weird JS console error, I immediately started reducing the page to a small test case to file a bug report.

The error was completely incomprehensible, except that it seemed like Opera tried to execute some HTML markup as JavaScript:

Inline script compilation
Syntax error while loading: line 6 of inline script at http://www.atlassian.com/software/jira/videos/jira_video/jira_video.jsp :
Expected expression
<script language="JavaScript"></script><noscript></noscript><!--/DO NOT REMOVE/-->
--------------------------------------^

This sometimes happens if a file has a SCRIPT element with a SRC that links to a HTML file. But the page had only two external .js files, and neither was broken.

After a bit of trial and error I had a minimal file. The error occurs because a starting HTML comment inside a SCRIPT tag does not have a matching closing comment inside that SCRIPT tag. So Opera looks ahead for another closing comment, and it seems this confusion means some of the markup is seen as being "inside" the script.

Now I was in for a great surprise: both IE7 and Firefox 2 did exactly what Opera did when displaying my attempted test case! Have a look:
jira.htm
My first thought was that while minimising I had removed something that made the site work in IE and Firefox. Then I tried watching the video in either browser.. No joy. JIRA's video page is broken (by an Omniture SiteCatalyst script that carelessly omits a //--> from a script element) and - wow - they have broken it in a cross-browser compatible way!

Well, I just added that missing closing comment and re-loaded the page from cache in Opera - but I must be the only person who has seen JIRA's demo video recently