Added Support for the main structural element

[jbowyers]

Added a DOMContentLoaded event listener that tests css property values. Specifically the display property is tested on the 'main' structural element for the sake of adding support for this element. The current Master branch version fails to adequately test support for 'main' in IE11, Opera, and some versions of Safari. Support for the 'main' element was also added to the testing files.

[aFarkas]

Your code is a little bit too complicated and hard to read to get the task done you want. You use two nested try blocks without catch at least for the first try, you are iterating over an Array with a for in without using hasOwnProperty, you use eval...

Maybe the following code is simpler to read and will work too:

// test css properties - for newer browsers
if (supportsHtml5Styles) {
    try {
        document.addEventListener("DOMContentLoaded", function () {
            var el = document.createElement('main');
            document.body.appendChild(el);
            supportsHtml5Styles = !(window.getComputedStyle && window.getComputedStyle(el).getPropertyValue('display') != 'block');
            document.body.removeChild(el);
            if (!supportsHtml5Styles) {
                shivDocument(document);
            }
        }, false);
    } catch (e) { }
}

I will think about a better way to add element detection support.

[jbowyers]

@aFarkas - thanks for your comments and suggestions. I appreciate the need to make the code more readable. Below is a commented version with your suggested revisions.

The two try blocks are necessary. The outer try is needed because the DomContentLoaded event listener is not supported in IE8. The inner try is needed because failure of the callback function will not be caught by the outer try becuase it happens in a callback function.

I wanted to keep the test array so it would be easy to add other elements to this test in future.

// test css properties - for newer browsers
  if (supportsHtml5Styles) {  //  Document has not been shived
    // Need outer try because DOMContentLoaded event listener not supported in IE8-
    try {
      // Tests need to happen after DOM content loaded
      document.addEventListener("DOMContentLoaded", function (event) {
        // Need inner try because outer try will not catch a callback
        try {
          // Test array of specified elements
          for (var i = 0; i < shivTestElements.length; i++) { 
            // create the test element
            var el = document.createElement(shivTestElements[i][0]);
            // Append the test element to ensure browser default CSS Properties are applied
            document.body.appendChild(el);
            // Test property value
            if (window.getComputedStyle) {
              var cssProperty = window.getComputedStyle(el).getPropertyValue(shivTestElements[i][1]);
              supportsHtml5Styles = (cssProperty == shivTestElements[i][2]);
            }
            document.body.removeChild(el);
          }
          if (!supportsHtml5Styles) { //  Document needs to be shived
            shivDocument(document);
          }
        } catch (e) { }
      });
    } catch (e) { }
  }

[aFarkas]

If we would need something in the future, ee can still add it. If there comes a new block element with HTML we will only test against this new element. So i don see a reason to have this array only because we might need it, but currently don't.

We had something like this in the future and we don't need to go async. But needs some further testing:

// test css properties - for newer browsers
if (supportsHtml5Styles && window.getComputedStyle) {
    (function () {
        try {
            var el = document.createElement('main');
            document.documentElement.appendChild(el);
            supportsHtml5Styles = window.getComputedStyle(el).getPropertyValue('display') == 'block';
            document.documentElement.removeChild(el);
        } catch (e) { }
    })();
}

[jbowyers]

That's definately more elegant but am not clear how it would be implemented. Isn't the self-invoking function called too soon?

[aFarkas]

Yeah, this is the idea behind this. I looked and we had the following test before: https://github.com/aFarkas/html5shiv/blob/f3c25cc64d76e65dee1a54e7d825a22dc562c77a/src/html5shiv-printshiv.js#L4-16

Maybe we should switch to it again, while we still do a hidden in element test?

ping @jonathantneal

[aFarkas]

ping @paulirish @jonathantneal @jbowyers

Would like to change our feature detection script to something like this:

(function () {
  try {
    var fake, root, docEl, el;
    var a = document.createElement('a');
    a.innerHTML = '<xyz></xyz>';
    //if the hidden property is implemented we can assume, that the browser supports basic HTML5 Styles
    supportsHtml5Styles = ('hidden' in a) && !!window.getComputedStyle;

    supportsUnknownElements = a.childNodes.length == 1 || (function () {
      // assign a false positive if unable to shiv
      (document.createElement)('a');
      var frag = document.createDocumentFragment();
      return (
          typeof frag.cloneNode == 'undefined' ||
              typeof frag.createDocumentFragment == 'undefined' ||
              typeof frag.createElement == 'undefined'
          );
    }());

    //if hidden test was true, detect the newest "structural" html element style
    if(supportsHtml5Styles){
      el = document.createElement('main');
      docEl = document.documentElement;
      root = document.body || (fake = docEl.insertBefore(document.createElement('body'), docEl.firstChild));
      root.insertBefore(a, root.firstChild);
      supportsHtml5Styles = getComputedStyle(el, null).display !== 'inline';
      if(fake){
        docEl.removeChild(fake);
      } else {
        root.removeChild(nav);
      }
    }

  } catch (e) {
    // assign a false positive if detection fails => unable to shiv
    supportsHtml5Styles = true;
    supportsUnknownElements = true;
  }

}());

Basically the if(supportsHtml5Styles){ part is new and now tests the main element for not beeing inline. We have refused to add this test in the past, but I think it could be time to re-add the old display detection script. What do you think?

[jonathantneal]

What a difference a few years makes. This PR was pretty cool, but it also may have gotten a bit of ahead of itself. I think we can close it, @aFarkas. Are you okay with that?

[aFarkas]

@jonathantneal Of course!