Incorrect JS syntax is bothering IE7

Thanks for the report! You're correct, the trailing comma makes older browsers choke.

We can't just remove the comma from any single line, because someone may choose to add a callback or complete function, and in that case those are the lines which need the comma removed.

The correct approach (other than re-writing this to fix the overall architecture) is to remove that last comma after the entire command has been built.

I thought this was handled properly (line 368 in modernizr.module at the time of comment), but your report indicates that it doesn't.

As a sidenote, I would advise you to load respond.js directly using the following syntax that avoids an asynchronous download:

if ( ! Modernizr.mq('only screen') ) {
  document.write('<script src="/path/to/respond.min.js"></s'+'cript>');
}

This advice is straight from the Modernizr devs.

Patch excludes some required commas. See snippet below. I'm working on my own fix and will post if I find something flexible enough :S

<script>Modernizr.load([{
  test: Modernizr.mq,
  nope: '/sites/all/themes/mytheme/javascripts/respond.min.js'
},{
  test: window.matchMedia,
  nope: '/sites/all/themes/mytheme/javascripts/matchMedia.js'
  load: '/sites/all/themes/mytheme/javascripts/enquire.min.js'
}]);</script>

Here's what I've ended up changing that same block to (starts at line 333 in 7.x-3.1):

// Build the Modernizr.load() commands.
  if (count($testObjects)) {
    $num_tests = 1;
    $output .= 'Modernizr.load([';
    foreach ($testObjects as $load) {
      $output .= ($num_tests > 1) ? ',' : '';
      $output .= '{' . "\n";
      $output .= '  test: ' . $load['test'];

      // Print each action and its resources
      $actions = array('yep', 'nope', 'both', 'load');
      $prefix = "\n";
      foreach ($actions as $action) {
        if (isset($load[$action])) {

          // Begin output for this action
          $output .= $prefix.'  ' . sprintf('%-4s', $action) . ': ';

          // How many resources for this action?
          if (count($load[$action]) == 1) {
            // Single resource
            $output .= "'" . $load[$action][0] ."' ";
            $output = substr($output, 0, -1);            
          }
          else {
            // Multiple resources
            $output .= '[';
            foreach ($load[$action] as $resource) {
              $output .= "'" . $resource . "'";
            }
            // Truncate last comma
            $output = substr($output, 0, -1);
            $output .= "],\n";
          }
        }
        $prefix = ",\n";
      }

Hello, thanks for tracking down the problem. Do you know how to make a patch? I can't really spot the difference in the code you supplied, but if you created a patch I would be able to review/commit the fix much faster.

Was planing on working on a patch this week anyway. I'll get it up asap.

Here's the patch. It basically adds the commas as a prefix rather than appending them. I had to move a few linebreaks as well to keep the formatting.

Rerolled.

k I looked into this, pretty simple fix based on a silly error of mine. I'm adding some newlines into the output just for readability's sake, and the substr() mentioned in comment #1 needs to remove two characters instead of one to account for both the comma and newline.

Would you mind trying this patch?

Did a quick test. Seems to be working for me.

That's close enough to an RTBC for me ;)

fixed!

Needs work because a single resource has the same bug...

Should look like this:

            // Single resource
            $output .= "'" . $load[$action][0] . "'\n";

Can I get a patch to review and commit?

It appears that the latest dev did indeed solve the issue. Sorry about that. I was using 7.x-3.1 + a patch I found here in the issue queue.