robincornett / genesis-accessible

This plugin fixes some accessibility issues with the Genesis Framework
28 stars 9 forks source link

genwpacc-skiplinks.js breaks site below IE9 #18

Closed PurpleHippoDesign closed 9 years ago

PurpleHippoDesign commented 9 years ago

Plugin breaks site below IE9 due to: Object doesn't support property or method "addEventListener" in genwpacc-skiplinks.js

rianrietveld commented 9 years ago

I'm afraid if I make it conditional, people will turn it off be default because they don't know what the Javascript does, It's a pity that this is so hard to test, I havent got IE 8 either. I found on found some discussion on http://stackoverflow.com/questions/20160216/addeventlistener-doesnt-work-in-ie-tested-in-ie8 If we have someone with IE8 we could try out

if ( window.addEventListener ) {
window.addEventListener( 'hashchange', function() {
    'use strict';
    var element = document.getElementById( location.hash.substring( 1 ) );

    if ( element ) {
        if ( ! /^(?:a|select|input|button|textarea)$/i.test( element.tagName ) ) {
            element.tabIndex = -1;
        }
        element.focus();
    }
}, false);
}
GaryJones commented 9 years ago

The basic approach to support IE8 is something like:

if ( window.addEventListener ) { 
  window.addEventListener( 'hashchange', function() {
    ...
  }
} else { // IE8 and earlier - note the extra 'on' event prefix.
  window.attachEvent( 'onhashchange', function() {
    ...
  }
}
rianrietveld commented 9 years ago

Will change that, a lot of people are still on IE < 9 in the Netherlands too

rianrietveld commented 9 years ago

Changed it into:

if ( window.addEventListener ) {
    window.addEventListener( 'hashchange', function() {
        'use strict';
        var element = document.getElementById( location.hash.substring( 1 ) );

        if ( element ) {
            if ( ! /^(?:a|select|input|button|textarea)$/i.test( element.tagName ) ) {
                element.tabIndex = -1;
            }
            element.focus();
        }
    }, false);
} else { // IE8 and earlier
  window.attachEvent( 'onhashchange', function() {
    'use strict';
        var element = document.getElementById( location.hash.substring( 1 ) );

        if ( element ) {
            if ( ! /^(?:a|select|input|button|textarea)$/i.test( element.tagName ) ) {
                element.tabIndex = -1;
            }
            element.focus();
        }
    }, false);
}
GaryJones commented 9 years ago

If that works, you should be able to do:

function ga_skiplinks() {
    'use strict';
    var element = document.getElementById( location.hash.substring( 1 ) );

    if ( element ) {
        if ( ! /^(?:a|select|input|button|textarea)$/i.test( element.tagName ) ) {
            element.tabIndex = -1;
        }
        element.focus();
    }
}

if ( window.addEventListener ) {
    window.addEventListener( 'hashchange', ga_skiplinks, false );
} else { // IE8 and earlier
    window.attachEvent( 'onhashchange', ga_skiplinks, false );
}
rianrietveld commented 9 years ago

Ah, thanks Gary, your JS is aways way better. I have no idea if it works in IE, will aks @robincornett

rianrietveld commented 9 years ago

@garyj: @robincornett did a IE 8 test on your JS and it works, so it's in today's commit

GaryJones commented 9 years ago

Sweet. Thanks ladies.

GaryJones commented 9 years ago

Pinging @cdils so she can add it to UP too :-)

rianrietveld commented 9 years ago

Fixed in commit https://github.com/RRWD/genesis-accessible/commit/6639cf71a4ac87606fcc2374918c0e7540c75c93