miscoded

Something tells me that BestBuy redesigned their home page last week and replaced the old OpenCube script with something more modern - looks exactly the same as the old menu but now it's a simple unordered list in the markup itself, hover effects provided by CSS. So far so good!

Looking around a bit, there is plenty of other code that could do with some modernising. Here's a snippet of the function handleEnterKeyPress (one of many enter-key related functions):

if(navigator.appName == "Netscape")
{
if(arguments.callee.caller.arguments[0].which == 13)

Wow, using arguments.callee.caller and the arguments property on function objects to look up the pressed key - would be more than a little cleaner to pass the event object to the function as an argument.. And the sniffing is ugly.

It seems somebody needs to learn about JavaScript's bracket syntax -

if (tabnumber==1)
{
document.tab1.src = imgServer + locale + "/images/global/misc/pdptabs/tab_prodspecs_over.gif";
}
if (tabnumber==2)
{
document.tab2.src = imgServer + locale + "/images/global/misc/pdptabs/tab_accessories_over.gif";

and the 5 other if clauses would be much nicer as document['tab'+tabnumber].src='...'.

Having good names for functions can be a good idea too:

/**
* test for https:
* return true for secure
* false for standard http
* jm 2006/10/24
*/

function getProtocol(){
var isSecure = (window.location.protocol == "https:") ? true: false;
return isSecure;
}

Source

They have one of those addEventListener/addEvent/set event handler directly kludges.. but with a twist:

/**
* takes function call 
* delays execution until full page load
* dwl 2006/09/28
*/
function addOnloadEvent(fnc){
  if ( typeof window.addEventListener != "undefined" )
    window.addEventListener( "load", fnc, false );
  else if ( typeof window.attachEvent != "undefined" ) {
    window.attachEvent( "onload", fnc );
  }
  else {
    if ( window.onload != null ) {
      var oldOnload = window.onload;
      window.onload = function ( e ) {
        oldOnload( e );
        window[fnc]();
      };
    }
    else
      window.onload = fnc;
  }
}

Is it just me, or is the window[fnc](); statement completely broken? The fnc argument to this function is supposedly a function object, not a string. So doing window[fnc] will decompile the function and look for window['function (){ ..code here.. }'] which is not likely to return anything. Somebody *still* needs to learn about JavaScript bracket notation (well, mr. dwl in this case).

Finally, looking at this scares me a bit:

function getSnum(){var WshShell=new ActiveXObject("WScript.Shell");var sName=WshShell.ExpandEnvironmentStrings("%LOCATIONNUM%");return sName;}

I don't know much ActiveX but it looks like they use it for in-store information kiosks. If you find an XSS issue in their kiosk site or can use DNS poisoning to serve some JS that shouldn't be running there it looks like you can take over the machines completely with shell scripting from your JS! Wow..