This Javascript looks like butt.



  • More spelunking through old code. This time, a bit ripped from being hard coded in a VB6 COM DLL into a separate .js file. Also keep in mind that each line was a string concatenation operation in VB6:

    var navToCreate = 'switch(app){\n';
    navToCreate += '  case \'Resident Care\':\n';
    navToCreate += '    if(window.frames(\'IResidentCare\')!=void window.frames(\'IResidentCare\')){\n';
    navToCreate += '      switch(area){\n';
    navToCreate += '        case \'Residents\':\n';
    navToCreate += '         document.all(\'BtnResidentCare\').click();\n'
    navToCreate += '          if(window.frames(\'IResidentCare\').document.all(\'Residentsbutt\')!=void window.frames(\'IResidentCare\').document.all(\'Residentsbutt\') && (window.frames(\'IResidentCare\').OKToNavigate()==\'\')){\n';
    navToCreate += '            if(window.frames(\'IResidentCare\').document.all(\'Residentsbutt\').className!=\'Selected\'){\n';
    navToCreate += '              window.frames(\'IResidentCare\').document.all(\'Residentsbutt\').click();\n';
    navToCreate += '            }else{\n';
    navToCreate += '              if(window.frames(\'IResidentCare\').window.frames(\'mainiframe\').window_onload!= void window.frames(\'IResidentCare\').window.frames(\'mainiframe\').window_onload){;\n';
    navToCreate += '                window.frames(\'IResidentCare\').window.frames(\'mainiframe\').window_onload();\n';
    navToCreate += '              }\n';
    navToCreate += '            }\n';
    navToCreate += '          }\n';
    navToCreate += '          break;\n';
    navToCreate += '        default:\n';
    navToCreate += '          break;\n';
    navToCreate += '      }\n';
    navToCreate += '    }\n';
    navToCreate += '    break;\n';
    navToCreate += '  case \'Units\':\n';
    navToCreate += '    if(window.frames(\'IisUnits\')!=void window.frames(\'IisUnits\')){\n';
    navToCreate += '      switch(area){\n';
    navToCreate += '        case \'Units\':\n';
    navToCreate += '          if(window.frames(\'IisUnits\').document.all(\'UnitListbutt\')!=void window.frames(\'IisUnits\').document.all(\'UnitListbutt\')){\n';
    navToCreate += '            document.all(\'BtnisUnits\').click();\n';
    navToCreate += '            if(window.frames(\'IisUnits\').document.all(\'UnitListbutt\').className!=\'Selected\'){\n';
    navToCreate += '              window.frames(\'IisUnits\').document.all(\'UnitListbutt\').click();\n';
    navToCreate += '            }\n';
    navToCreate += '          }\n';
    navToCreate += '          break;\n';
    navToCreate += '        default:\n';
    navToCreate += '          break;\n';
    navToCreate += '      }\n';
    navToCreate += '    }\n';
    navToCreate += '    break;\n';
    navToCreate += '  case \'Community Admin\':\n';
    navToCreate += '    if(window.frames(\'IisAdmin\')!=void window.frames(\'IisAdmin\')){\n';
    navToCreate += '      switch(area){\n';
    navToCreate += '      }\n';
    navToCreate += '    }\n';
    navToCreate += '      break;\n';
    navToCreate += '  default:\n';
    navToCreate += '      break;\n';
    navToCreate += '}\n';
    window.top.navTo = new Function('app', 'area', navToCreate);
    

    Because window.top.navTo = function(app, area){ ... }; would have been too hard to read, I guess. Besides the gratuitous use of window.all everywhere, and the bad choice of shortening button to butt, I think my two favorite bits are the semicolon right after a { on line 11, and the block towards the end with the random void, followed by an empty switch. I'm not even entirely sure what this code is trying to accomplish, other than causing syntax errors. And it's only Monday morning.



  • Nevermind window.frames isn't a function, so he shouldn't be able to call window.frames('IResidentCare').. that should return "property is not a function" (It's an object, so can be accessed either window.frames.IResidentCare or with window.frames['IResidentCare']).

    So much facepalm



  • VB6-era is about the last time that JavaScript would work in a browser.



  • And the "void".. looking at it now that I'm at work.. He's simply using it to be able to test against undefined. Which is ridiculous and stupid.



  • @blakeyrat said:

    VB6-era is about the last time that JavaScript would work in a browser.

    My current task is making this work in IE10+, which a lot of this code is in HTC files, using document.all, a bunch of nasty Javascript to handle hovers, using the modalPopup method and requires the MSXML ActiveX object. Added bonus: The new user agent in IE11 does not contain the magic string 'MSIE', so the site redirects you to the Microsoft Internet Explorer page. That and there's XSS attacks abound in this site, and the ASP (not .Net, classic ASP) session cookie is not marked HTTP Only.

    And of course, we have this style of comments:

    // This functions loops through the option list of the destination box and looks in ShooterData
    // for any nodes that are still in the destination box - meaning they were not removed. If a 
    // node is found, then remove it from the node list because there is no processing that needs to 
    // be done to this node. If a node is not found then add a node to the node list and set its Action
    // to Insert. After that, it loops through all of the z:rows in ShooterData that do not have an action
    // and set there actions to delete, this means that it has been removed from the destination box and
    // was not dealt with in the loop through the destination box option list.
    

    And I also see this pattern everywhere:

    if (oBody.onpagesave) {
    	eval(oBody.onpagesave);
    }
    

    Maybe I should go drink for lunch. There is a place called Trainwreck Saloon near here, which always seems appropriate when dealing with this code.



  • And I just found this

    <input type="submit" name="btnOK" value="OK" onclick="return ReturnResidentIDY();WebForm_DoPostBackWithOptions(new WebForm_PostBackOptions("btnOK", "", true, "", "", false, false))" id="btnOK" style="width:60px;">
    

    Why do I continue to be amazed by the lack of understanding of Javascript in this app.



  • @bardofspoons42 said:

    My current task is making this work in IE10+, which a lot of this code is in HTC files, using document.all, a bunch of nasty Javascript to handle hovers, using the modalPopup method and requires the MSXML ActiveX object.

    I suppose you don't have the option of presenting an analysis of the long term costs of supporting this ghoul vs. the cost of a new implementation to your manager or your manager's manager?



  • @Ragnax said:

    I suppose you don't have the option of presenting an analysis of the long term costs of supporting this ghoul vs. the cost of a new implementation to your manager or your manager's manager?

    Oh, that option has been presented, and is acknowledged it needs to be done, as the VB6 code is getting more and more problematic. But it's been deemed that getting it working in IE10 is a shorter term project than re-architecting the whole thing, and I tend to agree as we'd either have to completely throw away the VB6 code and restart from basic requirements, or spend months reverse engineering the existing code.

    The conversion is going decently so far. I've been cleaning up messy table based layouts with a few divs and a few lines of css, hovers are now pure CSS. I've mainly figured out how to get jQuery's XML support to do most of what they were doing (Pretty much selecting an element from the XML document, and updating it's value. The biggest thorn in my side currently is the whole VB6 code works on posting an XML document back to the server, so I can't do simple redirects for most things, or directly put a URL into an iframe. Which also breaks refreshing the page. And proper post-redirect-get will take way too long to implement, as every single VB6 DLL, of which there are 32, re-implement the same basic bits of request handling.


Log in to reply