iframe issues when adding banner/footer to DOM

A couple of weeks ago when I decided to push the NYGOV banner/footer to production for Tax and Finance's Online Services, external users immediately started reporting issues. All pages using Ajax didn't work anymore so I had to remove the banner/footer immediately. Since then, I haven't much time to analyze the root of the issue until today (yes Saturday :-1:)

At first I thought there was a conflict with jQuery (being used both by OLS and loaded with Google Tag Manager (gtm.js).) I know there have been some reports of jQuery conflicts so I focused on that first. Also, based on the error, it looked like a possible cause.

After quite some research, I found out the problem had to do with the way IE handles <iframe>s. Based on how OLS pages are architected, it is preferable to using JavaScript to add both the banner and footer to the pages (similar to iFlow applications but not quite the same.) This should be a valid alternative for adding these two pieces. Initially I was adding both the banner and footer using purely DOM (the right way to doing this) but I faced some issues too (#148). To overcome this issue (that no one has commented about), I started using innerHTML to append the <iframe>s. Everything looked ok until this issue showed up on IE9-IE11 (see screen shot with error below.)

Anyway, to make the long story short, the issue happens when appending both the banner and the footer to the {same} (in this case the <body>) element. When adding the banner first, the <body> gets a wrapper <div> appended to it. This <div> contains the <iframe> for the banner and the assets for the banner are loaded. Seconds later the footer gets appended to the <body> also inside a <div>. Here, the <body> element realizes there is a change in its DOM and decides to reload the assets for the banner once again (after the footer assets begin to load.) This double load (see second screen shot - initiator = frame navigate) causes Object, String, Array and other global objects to be undefined for all but the last assets loaded (now the banner's) and hence producing the JavaScript error since the code for the footer no longer finds a valid global Object. This error caused other issues down the road with OLS JavaScript code. You can read more about the way IE handles <iframe> changes here.

To fix this, I could make a change to the footer so it gets appended to a different element rather than the <body> element and this will solve the issue for now.

At this point though, I can not trust the NYGOV banner/footer code and for this reason I have decided to build my own simplified version of the static banner/footer. OLS has more than 2 million users and having issues like the one experienced a couple of weeks ago made me waste many days, brought me many headaches and sidetracked me from other priorities. The risk for other potential bugs is high considering the lack of proper versioning methodologies and the implementation as a whole. You call me negative if you want (I think myself more as constructively honest), but both the banner/footer rely on a poor architecture on every angle, general implementation, efficiency, the lack of unobtrusive JavaScript, versioning approach, etc. If the banner/footer were implemented the right way (I have already proposed and documented how to do this) I would be ok... but I don't see this code changing or at least I am not aware of a complete rewrite to make it completely Object-oriented, encapsulated and unobtrusive (especially separation of presentation/behavior.) Last but not least and just in case, this is not IE's fault.

Thanks.

Note 1: I am also adding my code below. Note 2: Please give me access to categorize the issue with Labels.

PLUGINS.banner = (function banner() {
    // private properties
    var divElem = document.createElement("DIV"),

    // private API
    _priv = {}, 

    _init = function _init() {
        var bodyElem = document.body,
            skipNav = _priv._getSkipNav(),
            iframe = _priv._getIframe();

        divElem.setAttribute("id", "nygovBanner");
        divElem.setAttribute("class", "nygov-banner");

        // append elements to wrapper div
        divElem.appendChild(skipNav);
        divElem.innerHTML = iframe;

        // append div to body
        bodyElem.insertBefore(divElem, bodyElem.firstChild);
    };

    // _priv API
    /* _priv._getSkipNav */
    _priv._getSkipNav = function _priv_getSkipNav() {
        ...
        return aElem;
    },

    /* _priv._getIframe */
    _priv._getIframe = function _priv_getIframe() {
        return '<iframe id="nygov-universal-navigation-frame" class="nygov-universal-container" width="100%" height="86px" src="' + (document.location.protocol === "file:" ? "http:" : "") + '//static-assets.ny.gov/load_global_menu/ajax?iframe=true&target=blank" data-updated="2014-11-07 08:30" frameborder="0" style="border:none; overflow:hidden; width:100%; height:86px;" scrolling="no">Your browser does not support iFrames</iframe>';
    };

    // revealing public API
    return {
        init: _init
    };  
}());

PLUGINS.footer = (function footer() {
    // private properties
    var divElem = document.createElement("DIV"),

    // private API
    _priv = {}, 

    _init = function _init() {
        var bodyElem = document.body,
            iframe = _priv._getIframe();

        // make sure last child is a text node
        bodyElem.innerHTML += " ";

        divElem.setAttribute("id", "nygovFooter");
        divElem.setAttribute("class", "nygov-footer");

        // append elements to wrapper div
        divElem.innerHTML = iframe;

        // append div to body ... >> here is when the issue occurs by appending the footer to the <body> ... <<
        bodyElem.insertBefore(divElem, bodyElem.lastChild);
    };

    // _priv API
    /* _priv._getIframe */
    _priv._getIframe = function _priv_getIframe() {
        return '<iframe id="nygov-universal-footer-frame" class="nygov-universal-container" width="100%" height="200px" src="' + (document.location.protocol === "file:" ? "http:" : "") + '//static-assets.ny.gov/load_global_footer/ajax?iframe=true" data-updated="2014-11-07 08:30" frameborder="0" style="border:none; overflow:hidden; width:100%; height:200px;" scrolling="no">Your browser does not support iFrames</iframe>';
    };

    // revealing public API
    return {
        init: _init
    };  
}());

nygov_iframe_issue

nygov_iframe_issue_2

That's a solid strategy... and an acceptable alternative. We'll leave this issue open so that it can be referenced for improving the general implementation.

Just to verify, @mfarfanr this is an issue stemming from the Static Implementation of the uNav, correct?

Based on how OLS pages are architected, it is preferable to using JavaScript to add both the banner and footer to the pages

I suspect that the uNav is only designed to be included as part of the HTML page, not added with JavaScript, so this was probably never in scope or tested at all. This might mean this is less of a bug, and could become an enhancement issue to "Provide Implementation code for including uNav via JavaScript".

This double load (see second screen shot - initiator = frame navigate) causes Object, String, Array and other global objects to be undefined for all but the last assets loaded (now the banner's) and hence producing the JavaScript error since the code for the footer no longer finds a valid global Object.

@mfarfanr Do you have - or can you make - a sample page where the error occurs? Could this error also happen if the footer loads before the header? Is https://github.com/nys-its/universal-navigation/issues/147 possibly related?

This error caused other issues down the road with OLS JavaScript code.

Do the JavaScript errors just prevent further code from executing or is there anything else happening? One enhancement for the future may be to make sure that the uNav catches it's own errors so it will not prevent other code on the page from running. Worth splitting into a separate issue?

this issue showed up on IE9-IE11 ... this is not IE's fault

Is it possible to replicate the error in another browser? I tried to replicate the issue but I only have IE8 (and Chrome and Firefox).

@esteinborn - I have only used the static version, but if the Interactive version also uses an iframe - which I believe it does -, then using JavaScript to append the Interactive banner/footer code to the body element should produce the same results.

@Lukenickerson - I wouldn't consider the ability to adding any external component using JavaScript something "out of scope" but more an involuntary "miss" in the general implementation strategy which should've never been done using an iframe in the first place. This is a bug to me because I was told to add the banner/footer to my pages, and based on what the current codebase offers I would expect using JavaScript instead of inline HTML be a doable alternative. By doing this, I avoid deploying packages via migrations and I can deploy the js files myself rapidly (or rollback.) The bug is unfortunately an architectural bug, particularly stemming from the fact that an iframe is being used (which as we know brings quite a few more bugs, i.e. the ability to properly (automatically) resize the parent container.)

I can make a very simple page if you like to see the issue. Not sure about #147 which says the issue is also showing up on IE8. IE8 doesn't manage the iframe as IE9-IE11 does. The issue is specific to IE9-IE11 for now (not sure about future versions of IE.)

Yes, the caused JavaScript error fails to parse other code. I wouldn't do silent try { } catch() { } to suppress this (and other) errors; you need a way to troubleshoot issues like this one. Base classes (objects) shouldn't have any try-catch statements, instead the caller (business layer) should have a try-catch to deal with any leaks on the core code like this one. Also, I wouldn't work on any other futures to the uNav until it gets rewritten. The more you add the harder it will be to rewrite it and the riskier it will become for the reasons explained on the initial comment.

I don't have any info on the uNav architecture, so I'm really just guessing what was in/out of scope. But it's not unthinkable to me that one of the design requirements was "Works (somewhat) without JavaScript", which might lead to an iframe solution.

If you'd like help finding an instance-level work-around for this issue, having a simple page up would be a good way to start. I think the best outcome for now could be a new Implementation code for including uNav via JavaScript, which anyone in your situation could use.

@Lukenickerson Not using JavaScript even for the simplest solution of this banner/footer is not possible. The static version I have done is very minimalist, but I still need delegated JavaScript (body.onclick) to open/close the menu on the smaller device breakpoint(s).

I know already how to work around these issues and I could provide instructions on how to deal with them. Unfortunately, even with new uNav code changes for allowing the inclusion of the current b/f via JavaScript, the foundation is still inconsistent with a solid, flexible and reliable (i.e. no versioning) implementation for a system with 2+ million users - many of them trying to file their taxes on the last day permitted. The day I pushed the b/f the call center had many calls from angry taxpayers complaining about OLS page issues. Thanks for trying to help though.

itswebny / universal-navigation

iframe issues when adding banner/footer to DOM #186