search

salcode / bootstrap-genesis

WordPress Genesis Child Theme setup to use Bootstrap, Sass, and Grunt
MIT License
184 stars 63 forks source link

Suggested Changes #127

Closed bryanwillis closed 8 years ago

bryanwillis commented 8 years ago

I'm still getting complaints from users being forced into compatibility mode and the site layouts breaking due to the conditional comment tags rendering first.

<!doctype html>
<!--[if lt IE 7 ]> <html class="ie ie6 no-js" dir="ltr" lang="en-US"> <![endif]-->
<!--[if IE 7 ]>    <html class="ie ie7 no-js" dir="ltr" lang="en-US"> <![endif]-->
<!--[if IE 8 ]>    <html class="ie ie8 no-js" dir="ltr" lang="en-US"> <![endif]-->
<!--[if IE 9 ]>    <html class="ie ie9 no-js" dir="ltr" lang="en-US"> <![endif]-->
<!--[if gt IE 9]><!--><html class="no-js" dir="ltr" lang="en-US"><!--<![endif]-->
<head profile="http://gmpg.org/xfn/11">
<meta charset="UTF-8" />
<title>Bootstrap Genesis Theme</title>
<meta name="viewport" content="width=device-width, initial-scale=1" />
<meta http-equiv="X-UA-Compatible" content="IE=edge">

To quickly bring people up to date on the issue, basically company's running IE9 automatically throw this theme into compatibility mode because of the ie comments intended to add browser specific css classes to different versions of IE. This occurs because if the browser does not see the x-ua-compatible meta tag in time than it immediately falls into quirks mode.

The purpose of the comments are to add support for older IE browsers. By forcing sites in IE9 into compatibility mode and destroying many layouts we are taking a step backwards as the conditional comments were meant to do the opposite.

Take a look at the percentages of users currently using IE 6,7,8, and 9: IE 6 - 0.05% IE 7 - 0.05% IE 8 - 1.20% IE 9 - 0.94%

Total = 2.24%

Given those numbers I can't see any convincing reason to offer support for IE6 or IE7. Neither HTML5 Boilerplate, Bootstrap, Wordpress, or Genesis use or reccomend using these conditional comments anymore for the above reasons along with a few others.

Bootstrap uses Respond and HTML5Shiv, and Genesis uses HTML5Shiv. Since this is Bootstrap Genesis should we follow suit?

Below is how I'm currently doing things and are my suggestions on what we should change / add.

One First, we can start by deleting conditional-ie-conditional-comments.php and adding a wiki on how to add them for those that plan on adding their own css for ie 6, 7, 8, 9.

Two :Add x-ua-compatible on server-side. This avoids risk all together as well as fixes errors with validators. Additionally, we can add support for chrome frame if we want although development has stopped for it.

// Lets do this on server side and completely avoid all together.
add_action( 'send_headers', 'bsg_add_header_xua' );
function bsg_add_header_xua() {
    header( 'X-UA-Compatible: IE=edge,chrome=1' );
}

Three : Add no-js class using wordpress filters. This makes it much easier to remove since all that's needed is remove_filter( 'language_attributes', 'bsg_js_detection_lang_atts' );. Plus now we no longer have to hardcode the head which makes language attributes available again and the ability to add schema markup to the head (<head itemscope itemtype="http://schema.org/WebSite">). Also, the current way we have it isn't doing any html5 check like the rest of the theme does and is also adding link rel='profile' to the head tag which is the wrong way to do it in html5.

add_filter( 'language_attributes', 'bsg_js_detection_lang_atts' );
add_action( 'genesis_meta', 'bsg_js_detection_script', 10 );

function bsg_js_detection_lang_atts($output) {
    return $output . ' class="no-js"';
}
function bsg_js_detection_script() {
    if ( has_filter( 'language_attributes', 'bsg_js_detection_lang_atts' ) ) {
        echo "<script>(function(html){html.className = html.className.replace(/\bno-js\b/,'js')})(document.documentElement);</script>\n";
    }
}
// This could be added to `bsg_js_detection_script` or in a separate function.
// A separate function allows us to disable this feature and leave the `no-js` support.
function bsg_rel_profile() {
    if ( genesis_html5() ) {
    echo '<link rel="profile" href="http://gmpg.org/xfn/11" />\n';
    }
}
add_action( 'genesis_meta', 'bsg_rel_profile', 11);

Four : Add HTML5 Shim and Respond.js IE8 support of HTML5 elements and media queries. We remove genesis html5shiv and add our own to make sure they're properly loaded at the end of the opening head.

// http://getbootstrap.com/getting-started/#support-ie8-ie9
if ( genesis_html5() ) {
    remove_action( 'wp_head', 'genesis_html5_ie_fix' );
    add_action( 'wp_head', 'bsg_html5_shiv_respond_js_add_last', 999 );
}
function bsg_html5_shiv_respond_js_add_last() { 
?><!--[if lt IE 9]>
<script src="//oss.maxcdn.com/html5shiv/3.7.2/html5shiv.min.js"></script>
<script src="//oss.maxcdn.com/respond/1.4.2/respond.min.js"></script>
<![endif]-->
<?php
}

Finally the new head would start off something like this:

<!DOCTYPE html>
<html lang="en-US" class="no-js">
<head itemscope itemtype="http://schema.org/WebSite">
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<title>Bootstrap Genesis Theme</title>
<meta name="description" content="Descriptoin Goes Here" />
<script>(function(html){html.className = html.className.replace(/\bno-js\b/,'js')})(document.documentElement);</script>
<link rel='profile' href='http://gmpg.org/xfn/11' />
salcode commented 8 years ago

I think there are a number of other items in here we should consider making separate issues and refer back to these notes.

In generally, I'd like to make them separate issues with details about how to recreate the problem. I'm guessing in most cases it would be a code snippet that works in Genesis but not in Bootstrap Genesis

bryanwillis commented 8 years ago

Just saw that older viewsions of IE will no longer be supported as of the 12th of this month... Not sure how that will affect this issue as I haven't read too much about it.Maybe this intranet issue will be irrelevant? After, checking out the the IE Lifecycle it looks like companies that need support for older systems and IE are suppose to upgrade to Enterprise Mode for Internet Explorer 11 and as for desktop and server systems there is not support for IE7 or 8 on any systems. Anyway, I'll do some more reading, but we might as well wait and see how this all plays out for a few weeks before we waste time making changes. I could be wrong though... Thoughts?

salcode commented 8 years ago