Fix JS error on Firefox hidden iframes: verify that getComputedStyle is not returning null before using it

[felipebrahm]

On Firefox, getComputedStyle returns null if it's called inside a hidden iframe. You can see an example here:
https://www.groupon.com/deals/butter

Open that URL on Firefox. You'll see several JS errors in the console, the first being TypeError: window.getComputedStyle(...) is null. This is happening not on that page, but on a hidden iframe on that page (that iframe is loaded on page load but it's only shown when a user takes certain action, so it's a completely valid use case).

So, basically, this code will break:

var result = false;
if(window.getComputedStyle) {
  result = window.getComputedStyle(element, null).getPropertyValue('font-size') === '10px';
}
return result;

The solution is to first check for the returned value, like this:

var result;
var style;
if(window.getComputedStyle && (style = window.getComputedStyle(element, null)) !== null) {
  result = style.getPropertyValue('font-size') === '10px';
}
return result;

Different files have different coding styles, so I tried to preserve the coding style for each file.

[patrickkettner]

Hey @felipebrahm - awesome work!

I have two hesitations with this though.

1. Its a lot of boilerplate and likely to get forgotten on future detects. If we use it, it would make sense to make this a new function in /lib and the require it in to other detects.
2. This just means that firefox will "loose" support for these features, right? Perhaps there is a way to accomplish the same thing without too much extra code...

[felipebrahm]

@patrickkettner

1. You're asking me to come up with a way of simplifying not checking for the result of a function. That's way too generic. I'm not sure how I could simplify this into a reusable function. I mean, each use case is different and developers writing new code for Modernizr just need to remember to check for null after calling getComputedStyle. That's all. That's just how Firefox implemented this function. I could create a new modernizrGetComputedStyle helper that would do the exact same thing, just to force developers to remember that this function can return null, but there's not much else I can do about it.
2. We're actually only using one of those tests in our site, and for our use case it doesn't really matter. I did not spend too much time looking into the details of each test that I fixed here. I simply "fixed" them in the sense that JavaScript will no longer throw an exception on page load, but each test might need some adjustment if you really want to make them work on Firefox hidden iframes. But that would be a whole different PR.

[patrickkettner]

regarding 1. I really don't think it would be terribly complicated to abstract. Just something like

function testGetComputedStyle(style, callback) {
   if (window.getComputedStyle(style) !== null) {
    callback()
  } else {
    console.error('getComputedStyle returning null, its possible modernizr test results are inaccurate')
  }
}

2 - totally understand, however for other sites it may really matter. (see xkcd). I am fine with them "breaking" in the hidden iframe for the moment (as that is better than them flat out breaking like they are currently), however I think an error like the one above would at least help anyone hitting the issue in the future find this and figure out what the hell is going on

[benrbray]

Has any progress been made here? I'm having the same problem where Modernizr completely fails in Firefox.

[dmethvin]

I agree with @patrickkettner that adding all those null checks is going to be error-prone for anyone writing a Modernizr detect. Abstracting this into a function would help to centralize the noise.

Looking at the current uses, the only complicated case is the one for ruby which already has its own wrapper. Most of the others seem to just need one property off getComputedStyle() rather than a generic callback. So something like:

function getStyleProperty( style, pseudo, prop ) {
   // IE <= 8 doesn't have getComputedStyle
   // Firefox 39+ returns null for styles in hidden iframes
   var cs = window.getComputedStyle && window.getComputedStyle( style, pseudo );
   if ( !cs ) {
      return null;
   }
   return cs.getPropertyValue( prop );
}

There are some differences in usage like vhunit and inputtypes but those should work with .getPropertyValue() too.

When .getComputedStyles() doesn't work it seems really likely these detects will just be broken and maybe give the wrong results. If the iframe is permanently hidden I doubt that matters, but for cases where it starts hidden and is later shown that could cause problems.

[patrickkettner]

thanks for the initial repotr @felipebrahm! With the new helper and associated eslint rule, this shouldn't ever be a problem again